adoroszlai commented on code in PR #7181:
URL: https://github.com/apache/ozone/pull/7181#discussion_r1760799678
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListSubcommand.java:
##########
@@ -105,12 +119,39 @@ public void execute(ScmClient scmClient) throws
IOException {
ReplicationType.fromProto(type),
replication, new OzoneConfiguration());
}
- List<ContainerInfo> containerList =
- scmClient.listContainer(startId, count, state, type, repConfig);
+
+ int maxCountAllowed = parent.getParent().getOzoneConf()
+ .getInt(ScmConfigKeys.HDDS_CONTAINER_LIST_MAX_COUNT,
+ ScmConfigKeys.HDDS_CONTAINER_LIST_MAX_COUNT_DEFAULT);
+ if (all) {
+ System.out.printf("Attempting to list all containers." +
+ " The total number of container might exceed" +
+ " the cluster's current limit of %s. The results will be capped at
the" +
+ " maximum allowed count.%n",
ScmConfigKeys.HDDS_CONTAINER_LIST_MAX_COUNT_DEFAULT);
Review Comment:
Should this and similar informational messages go to `System.err`?
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java:
##########
@@ -509,6 +543,104 @@ public List<ContainerInfo> listContainer(long
startContainerID,
}
}
+ /**
+ * Lists a range of containers and get their info.
+ *
+ * @param startContainerID start containerID.
+ * @param count count must be {@literal >} 0.
+ * @param state Container with this state will be returned.
+ * @param factor Container factor.
+ * @return a list of containers capped by max count allowed
+ * in "hdds.container.list.max.count" and total number of containers.
+ * @throws IOException
+ */
+ @Override
+ @Deprecated
Review Comment:
Instead of adding a new method that's already deprecated, we can call the
one that accepts `ReplicationConfig`:
https://github.com/apache/ozone/blob/6b2f0f2e8e0541d7175093f0acd73fe7fdff7bab/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java#L726-L731
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/client/ScmClient.java:
##########
@@ -128,6 +128,19 @@ void deleteContainer(long containerId, Pipeline pipeline,
boolean force)
List<ContainerInfo> listContainer(long startContainerID,
int count) throws IOException;
+ /**
+ * Lists a range of containers and get their info.
+ *
+ * @param startContainerID start containerID.
+ * @param count count must be {@literal >} 0.
+ *
+ * @return a list of containers capped by max count allowed
+ * in "hdds.container.list.max.count" and total number of containers.
+ * @throws IOException
+ */
+ Pair<List<ContainerInfo>, Long> listContainerWithCount(long startContainerID,
+ int count) throws IOException;
Review Comment:
Please consider creating a wrapper object for the return type, instead of
using `Pair`. Having a custom type allows it to evolve, if necessary. It also
makes the code more readable, by using more specific method names instead of
`getLeft`, `getRight`.
--
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]
