kerneltime commented on code in PR #7944:
URL: https://github.com/apache/ozone/pull/7944#discussion_r1968786190
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ReconcileSubcommand.java:
##########
@@ -34,15 +48,188 @@
versionProvider = HddsVersionProvider.class)
public class ReconcileSubcommand extends ScmSubcommand {
- @CommandLine.Parameters(description = "ID of the container to reconcile")
- private long containerId;
+ @CommandLine.Parameters(description = "One or more container IDs separated
by spaces. " +
+ "To read from stdin, specify '-' and supply the container IDs " +
+ "separated by newlines.",
+ arity = "1..*",
+ paramLabel = "<container ID>")
+ private List<String> containerList;
+
+ @CommandLine.Option(names = { "--status" },
+ defaultValue = "false",
+ fallbackValue = "true",
+ description = "Display the reconciliation status of this container's
replicas")
+ private boolean status;
@Override
public void execute(ScmClient scmClient) throws IOException {
- scmClient.reconcileContainer(containerId);
- System.out.println("Reconciliation has been triggered for container " +
containerId);
- // TODO a better option to check status may be added later.
- System.out.println("Use \"ozone admin container info --json " +
containerId + "\" to see the checksums of each " +
- "container replica");
+ Iterator<String> idIterator;
+ // PicoCLI arity check guarantees at least one element will be present.
+ if (containerList.get(0).equals("-")) {
+ // Read from stdin.
+ idIterator = new Scanner(System.in, StandardCharsets.UTF_8.name());
+ } else {
+ // A list of containers was provided.
+ idIterator = containerList.iterator();
+ }
+
+ if (status) {
+ // Automatically creates one array for the output, while allowing us to
flush each object individually.
+ try (SequenceWriter arrayWriter =
JsonUtils.getSequenceWriter(System.out)) {
+ // Since status is retrieved using container info, do client side
validation that it is only used for Ratis
+ // containers. If EC containers are given, print a message to stderr
and eventually exit non-zero, but continue
+ // processing the remaining containers.
+ int invalidCount = 0;
+ while (idIterator.hasNext()) {
+ long containerID = Long.parseLong(idIterator.next());
+ if (!printReconciliationStatus(scmClient, containerID, arrayWriter))
{
+ invalidCount++;
+ }
+ }
+ if (invalidCount > 0) {
+ throw new RuntimeException("Failed to process reconciliation status
for " + invalidCount + " containers");
+ }
+ }
+ } else {
+ int invalidCount = 0;
+ while (idIterator.hasNext()) {
+ long containerID = Long.parseLong(idIterator.next());
+ try {
+ executeReconciliation(scmClient, containerID);
+ } catch (Exception ex) {
+ System.err.println("Failed to send reconciliation to container " +
containerID + ": " + ex.getMessage());
+ invalidCount++;
+ }
+ }
+ if (invalidCount > 0) {
+ throw new RuntimeException("Failed trigger reconciliation for " +
invalidCount + " containers");
+ }
+ }
+ }
+
+ private boolean printReconciliationStatus(ScmClient scmClient, long
containerID, SequenceWriter arrayWriter)
+ throws IOException {
+ ContainerInfo containerInfo = scmClient.getContainer(containerID);
+ if (containerInfo.getReplicationType() !=
HddsProtos.ReplicationType.RATIS) {
+ System.err.println("Cannot get status of container " + containerID +
+ ". Reconciliation is only supported for Ratis replicated
containers");
+ return false;
+ }
+ List<ContainerReplicaInfo> replicas =
scmClient.getContainerReplicas(containerID);
+ arrayWriter.write(new ContainerWrapper(containerInfo, replicas));
+ arrayWriter.flush();
+ return true;
+ }
+
+ private void executeReconciliation(ScmClient scmClient, long containerID)
throws IOException {
+ scmClient.reconcileContainer(containerID);
+ System.out.println("Reconciliation has been triggered for container " +
containerID);
+ System.out.println("Use \"ozone admin container reconcile --status " +
containerID + "\" to see the checksums of " +
+ "each container replica");
Review Comment:
We don't have a good way to indicate when reconciliation is completed...
Maybe we add a reconciliation count as a metric to the container info?
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ReconcileSubcommand.java:
##########
@@ -34,15 +48,188 @@
versionProvider = HddsVersionProvider.class)
public class ReconcileSubcommand extends ScmSubcommand {
- @CommandLine.Parameters(description = "ID of the container to reconcile")
- private long containerId;
+ @CommandLine.Parameters(description = "One or more container IDs separated
by spaces. " +
+ "To read from stdin, specify '-' and supply the container IDs " +
+ "separated by newlines.",
+ arity = "1..*",
+ paramLabel = "<container ID>")
+ private List<String> containerList;
+
+ @CommandLine.Option(names = { "--status" },
+ defaultValue = "false",
+ fallbackValue = "true",
+ description = "Display the reconciliation status of this container's
replicas")
+ private boolean status;
@Override
public void execute(ScmClient scmClient) throws IOException {
- scmClient.reconcileContainer(containerId);
- System.out.println("Reconciliation has been triggered for container " +
containerId);
- // TODO a better option to check status may be added later.
- System.out.println("Use \"ozone admin container info --json " +
containerId + "\" to see the checksums of each " +
- "container replica");
+ Iterator<String> idIterator;
+ // PicoCLI arity check guarantees at least one element will be present.
+ if (containerList.get(0).equals("-")) {
+ // Read from stdin.
+ idIterator = new Scanner(System.in, StandardCharsets.UTF_8.name());
+ } else {
+ // A list of containers was provided.
+ idIterator = containerList.iterator();
+ }
+
+ if (status) {
+ // Automatically creates one array for the output, while allowing us to
flush each object individually.
+ try (SequenceWriter arrayWriter =
JsonUtils.getSequenceWriter(System.out)) {
+ // Since status is retrieved using container info, do client side
validation that it is only used for Ratis
+ // containers. If EC containers are given, print a message to stderr
and eventually exit non-zero, but continue
+ // processing the remaining containers.
+ int invalidCount = 0;
+ while (idIterator.hasNext()) {
+ long containerID = Long.parseLong(idIterator.next());
Review Comment:
Might be useful to catch `NumberFormatException` and print a more friendly
error message.
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ReconcileSubcommand.java:
##########
@@ -34,15 +48,188 @@
versionProvider = HddsVersionProvider.class)
public class ReconcileSubcommand extends ScmSubcommand {
- @CommandLine.Parameters(description = "ID of the container to reconcile")
- private long containerId;
+ @CommandLine.Parameters(description = "One or more container IDs separated
by spaces. " +
+ "To read from stdin, specify '-' and supply the container IDs " +
+ "separated by newlines.",
Review Comment:
Not for this PR, but we can look into brace expansion so that the CLI can
specify a range {1...1000}. This can ease the scanning for status via CLI
across many containers.
--
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]
