errose28 commented on code in PR #8523:
URL: https://github.com/apache/ozone/pull/8523#discussion_r2246548259
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/BasicDatanodeInfo.java:
##########
@@ -19,84 +19,70 @@
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import java.util.List;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
/**
* Represents filtered Datanode information for json use.
*/
-
-public class BasicDatanodeInfoJson {
- private final String id;
- private final String hostName;
- private final String ipAddress;
- private final List<DatanodeDetails.Port> ports;
- private final long setupTime;
- private final int currentVersion;
- private final HddsProtos.NodeOperationalState persistedOpState;
- private final HddsProtos.NodeOperationalState opState;
- private final long persistedOpStateExpiryEpochSec;
- private final HddsProtos.NodeState healthState;
- private final boolean decommissioned;
- private final boolean maintenance;
- private final int level;
- private final int cost;
- private final int numOfLeaves;
- private final String networkFullPath;
- private final String networkLocation;
- private final String networkName;
+@JsonPropertyOrder({
Review Comment:
Can we use `@JsonProperty(index)` on each method instead of declaring an
order up front? Labelling each method will be much easier to maintain. Note
that the indices can have gaps between them so entries can be inserted in the
future without updating all the following indices. For example we may want to
leave a gap in the indices between `ipAddress` and `ports` in case other node
identifying info is added later.
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/ListInfoSubcommand.java:
##########
@@ -155,7 +149,7 @@ private List<DatanodeWithAttributes> getAllNodes(ScmClient
scmClient)
long capacity = p.getCapacity();
long used = capacity - p.getRemaining();
double percentUsed = (capacity > 0) ? (used * 100.0) / capacity
: 0.0;
- return new DatanodeWithAttributes(
+ return new BasicDatanodeInfoJson(
Review Comment:
When sorting options are given, we can do limiting on the server side. When
they are not, we can move the limit to the beginning of the client side
processing to reduce the number of transformations done. Note that when a limit
is not defined `getLimit` returns `Integer.MAX_VALUE` so we still get the same
behavior.
```
diff --git
a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/ListInfoSubcommand.java
b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/ListInfoSubcommand.java
index 9271fcca0d..6b38191ad8 100644
---
a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/ListInfoSubcommand.java
+++
b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/ListInfoSubcommand.java
@@ -20,6 +20,7 @@
import com.google.common.base.Strings;
import java.io.IOException;
import java.util.Collections;
+import java.util.Comparator;
import java.util.List;
import java.util.Objects;
import java.util.UUID;
@@ -115,10 +116,6 @@ public void execute(ScmClient scmClient) throws
IOException {
.compareToIgnoreCase(nodeState) == 0);
}
- if (!listLimitOptions.isAll()) {
- allNodes = allNodes.limit(listLimitOptions.getLimit());
- }
-
if (json) {
List<BasicDatanodeInfo> datanodeList =
allNodes.collect(Collectors.toList());
System.out.println(JsonUtils.toJsonStringWithDefaultPrettyPrinter(datanodeList));
@@ -130,11 +127,12 @@ public void execute(ScmClient scmClient) throws
IOException {
private List<BasicDatanodeInfo> getAllNodes(ScmClient scmClient)
throws IOException {
+ int limit = listLimitOptions.getLimit();
+
// If sorting is requested
if (exclusiveNodeOptions != null && (exclusiveNodeOptions.mostUsed ||
exclusiveNodeOptions.leastUsed)) {
boolean sortByMostUsed = exclusiveNodeOptions.mostUsed;
- List<HddsProtos.DatanodeUsageInfoProto> usageInfos =
scmClient.getDatanodeUsageInfo(sortByMostUsed,
- Integer.MAX_VALUE);
+ List<HddsProtos.DatanodeUsageInfoProto> usageInfos =
scmClient.getDatanodeUsageInfo(sortByMostUsed, limit);
return usageInfos.stream()
.map(p -> {
@@ -165,10 +163,11 @@ private List<BasicDatanodeInfo> getAllNodes(ScmClient
scmClient)
null, HddsProtos.QueryScope.CLUSTER, "");
return nodes.stream()
+ .limit(limit)
.map(p -> new BasicDatanodeInfo(
DatanodeDetails.getFromProtoBuf(p.getNodeID()),
p.getNodeOperationalStates(0), p.getNodeStates(0)))
- .sorted((o1, o2) ->
o1.getHealthState().compareTo(o2.getHealthState()))
+ .sorted(Comparator.comparing(BasicDatanodeInfo::getHealthState))
.collect(Collectors.toList());
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
