You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "dombizita (via GitHub)" <gi...@apache.org> on 2026/07/06 19:37:31 UTC
[PR] HDDS-15715. Add helpful CLI flags to finalization CLIs [ozone]
dombizita opened a new pull request, #10678:
URL: https://github.com/apache/ozone/pull/10678
## What changes were proposed in this pull request?
Added new CLI flags to the upgrade finalization CLI:
- `finalize --wait`: the client will periodically poll the cluster for status from OM until the whole cluster is finalized. The finalize command is idempotent so waiting can be resumed at any time
- `status --json`: json version of the status output for scripting
- `status --verbose`: includes internal information like OM and SCM software and apparent versions, the min and max DN software and apparent versions in the cluster
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15715
## How was this patch tested?
Added unit tests, green run on my fork: https://github.com/dombizita/ozone/actions/runs/28660447709
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15715. Add helpful CLI flags to finalization CLIs [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on PR #10678:
URL: https://github.com/apache/ozone/pull/10678#issuecomment-5059892804
> Thanks for the updates. Left some more comments.
>
> Also here is an AI generated test we can add to cover the case of RPC failure during the wait loop. This assumes the desired behavior in this case is to exit the loop though, which I'm not sure is the case. It might be better to adjust the wait loop to print to stderr when this happens and retry automatically on the next interval, and update this test accordingly.
>
Thanks for the review, addressed the review comments in latest commit. I added the retry and print to stderr if there is a failed status query and adjusted the test case for this way.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15715. Add helpful CLI flags to finalization CLIs [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #10678:
URL: https://github.com/apache/ozone/pull/10678#discussion_r3605918626
##########
hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/ozone/admin/upgrade/TestFinalizeSubCommand.java:
##########
@@ -119,6 +132,165 @@ public void testNonZduServerPrintsErrorAndReturnsNonZero() throws Exception {
verify(omClient, never()).finalizeUpgrade();
}
+ @Test
+ public void testWithoutWaitFlagDoesNotPollStatus() throws Exception {
+ new CommandLine(cmd).parseArgs();
+ assertEquals(0, cmd.call());
+
+ verify(omClient, never()).queryUpgradeStatus();
+ }
+
+ @Test
+ public void testStatusCalledOnceWhenAlreadyFinalized() throws Exception {
+ when(omClient.queryUpgradeStatus()).thenReturn(finalizedStatus(3, 3));
+
+ new CommandLine(cmd).parseArgs("--wait");
+ assertEquals(0, cmd.call());
+
+ String output = outContent.toString(DEFAULT_ENCODING);
+ assertTrue(output.contains("Finalization complete."));
+ verify(omClient).finalizeUpgrade();
+ verify(omClient, times(1)).queryUpgradeStatus();
+ }
+
+ @Test
+ public void testWaitFlagPollsUntilFinalized() throws Exception {
+ when(omClient.queryUpgradeStatus())
+ .thenReturn(inProgressStatus(0, 3))
+ .thenReturn(inProgressStatus(2, 3))
+ .thenReturn(finalizedStatus(3, 3));
+
+ new CommandLine(cmd).parseArgs("--wait");
+ assertEquals(0, cmd.call());
+
+ String output = outContent.toString(DEFAULT_ENCODING);
+ assertTrue(output.contains("Waiting for finalization"));
+ assertTrue(output.contains("Finalization complete."));
+ verify(omClient, times(3)).queryUpgradeStatus();
+ }
+
+ @Test
+ public void testWaitFlagInterruptIsHandledCleanly() throws Exception {
+ // Make the poll interval long enough that the interrupt lands during the sleep (after the first poll).
+ cmd.setPollIntervalMillis(60_000);
+ when(omClient.queryUpgradeStatus()).thenReturn(inProgressStatus(0, 3));
+
+ new CommandLine(cmd).parseArgs("--wait");
+
+ AtomicInteger result = new AtomicInteger(-1);
+ Thread runner = new Thread(() -> {
+ try {
+ result.set(cmd.call());
+ } catch (Exception ignored) {
+ // The command handles interruption internally and should not throw.
Review Comment:
If no exception is expected we should fail the test if one is encountered.
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/upgrade/FinalizeSubCommand.java:
##########
@@ -51,11 +65,70 @@ public Integer call() throws Exception {
}
client.finalizeUpgrade();
out().println("Cluster finalization has been started. Monitor progress with `ozone admin upgrade status`");
+
+ if (wait) {
Review Comment:
We should probably alter this message if `--wait` is provided to indicate that we are waiting for finalization to complete.
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/upgrade/FinalizeSubCommand.java:
##########
Review Comment:
Existing issue, but we can fix it here.
```suggestion
description = "Initiates the process to finalize a cluster upgrade. This command is idempotent.",
```
##########
hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/ozone/admin/upgrade/TestStatusSubCommand.java:
##########
@@ -122,6 +134,96 @@ public void testNonZduServerPrintsErrorAndReturnsNonZero() throws Exception {
verify(omClient, never()).queryUpgradeStatus();
}
+ @Test
+ public void testJsonFlagBasicOutput() throws Exception {
+ when(omClient.queryUpgradeStatus()).thenReturn(basicResponse(false, false, 1, 3));
+
+ new CommandLine(cmd).parseArgs("--json");
+ assertEquals(0, cmd.call());
+
+ JsonNode root = JSON.readTree(outContent.toString(DEFAULT_ENCODING));
+ assertFalse(root.path("omFinalized").asBoolean());
+ assertFalse(root.path("scmFinalized").asBoolean());
+ assertEquals(1, root.path("datanodesFinalized").asInt());
+ assertEquals(3, root.path("datanodesTotal").asInt());
+ }
+
+ @Test
+ public void testJsonFlagWithVerboseIncludesVersions() throws Exception {
Review Comment:
The names of this and the previous test make it seem like the verbose flag affects json output, which is doesn't. If this is intentional, then there should be one test for all json output and it should also assert that content is identical when verbose is passed.
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/upgrade/FinalizeSubCommand.java:
##########
@@ -51,11 +65,70 @@ public Integer call() throws Exception {
}
client.finalizeUpgrade();
out().println("Cluster finalization has been started. Monitor progress with `ozone admin upgrade status`");
+
+ if (wait) {
+ return waitForFinalization(client);
+ }
}
return 0;
}
+ /**
+ * Polls the cluster status until OM, SCM and all healthy datanodes report finalized, the operator
+ * interrupts the command, or an RPC fails. {@code --wait} is safe to re-run: if the cluster is already
+ * finalized, the first poll returns done and the command exits 0.
+ */
+ private int waitForFinalization(OzoneManagerProtocol client) {
+ while (true) {
+ QueryUpgradeStatusResponse status;
+ try {
+ status = client.queryUpgradeStatus();
+ } catch (Exception e) {
+ err().println("Failed to query upgrade status: " + e.getMessage()
+ + ". Use `ozone admin upgrade status` to monitor progress.");
+ return 1;
+ }
+
+ // Check if cluster is already finalized
+ if (isClusterFinalized(status)) {
+ out().println("Finalization complete.");
+ return 0;
+ }
+
+ if (isVerbose()) {
+ StatusSubCommand.printVerbose(status, out());
+ } else {
+ HddsProtos.UpgradeStatus hdds = status.getHddsStatus();
+ out().printf("Waiting for finalization: OM=%s, SCM=%s, datanodes=%d/%d%n",
+ status.getOmFinalized(), hdds.getScmFinalized(),
+ hdds.getNumDatanodesFinalized(), hdds.getNumDatanodesTotal());
+ }
+ out().flush();
+
+ // Finalization checks before sleeping, so an already-finalized cluster returns without waiting a poll interval.
+ try {
+ Thread.sleep(pollIntervalMillis);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ out().println("Waiting interrupted. Use `ozone admin upgrade status` to monitor progress.");
+ return 0;
+ }
+ }
+ }
+
+ static boolean isClusterFinalized(QueryUpgradeStatusResponse status) {
+ HddsProtos.UpgradeStatus hdds = status.getHddsStatus();
+ return status.getOmFinalized()
+ && hdds.getScmFinalized()
+ && hdds.getNumDatanodesFinalized() == hdds.getNumDatanodesTotal();
+ }
Review Comment:
Looking at this again, I think we should add a new field to the proto to represent this, with this computation happening on the server side, i.e. OM returns `clusterFinalized = hdds.isFinalized() && omFinalized` and the client just consumes `clusterFinalized` to make its decision. It can still print the counts, but this allows us to change the definition of finalized on the server side without impacting old clients.
This is similar to what we did with the `shouldFinalize` field that OM gets from SCM instead of checking the SCM and datanode counts directly.
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/upgrade/FinalizeSubCommand.java:
##########
@@ -51,11 +65,70 @@ public Integer call() throws Exception {
}
client.finalizeUpgrade();
out().println("Cluster finalization has been started. Monitor progress with `ozone admin upgrade status`");
+
+ if (wait) {
+ return waitForFinalization(client);
+ }
}
return 0;
}
+ /**
+ * Polls the cluster status until OM, SCM and all healthy datanodes report finalized, the operator
+ * interrupts the command, or an RPC fails. {@code --wait} is safe to re-run: if the cluster is already
+ * finalized, the first poll returns done and the command exits 0.
+ */
+ private int waitForFinalization(OzoneManagerProtocol client) {
+ while (true) {
+ QueryUpgradeStatusResponse status;
+ try {
+ status = client.queryUpgradeStatus();
+ } catch (Exception e) {
+ err().println("Failed to query upgrade status: " + e.getMessage()
+ + ". Use `ozone admin upgrade status` to monitor progress.");
+ return 1;
+ }
+
+ // Check if cluster is already finalized
+ if (isClusterFinalized(status)) {
+ out().println("Finalization complete.");
+ return 0;
+ }
+
+ if (isVerbose()) {
+ StatusSubCommand.printVerbose(status, out());
+ } else {
+ HddsProtos.UpgradeStatus hdds = status.getHddsStatus();
+ out().printf("Waiting for finalization: OM=%s, SCM=%s, datanodes=%d/%d%n",
+ status.getOmFinalized(), hdds.getScmFinalized(),
+ hdds.getNumDatanodesFinalized(), hdds.getNumDatanodesTotal());
Review Comment:
Shouldn't we use the regular status output here instead of making a new one?
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java:
##########
@@ -1214,18 +1214,26 @@ public HddsProtos.UpgradeStatus queryUpgradeStatus() throws IOException {
try {
getScm().checkAdminAccess(getRemoteUser(), true);
+ if (scm.getScmContext().isInSafeMode()) {
+ throw new SCMException("Cannot query upgrade status while SCM is in safe mode. Wait until SCM exits "
+ + "safe mode and try again.", ResultCodes.SAFE_MODE_EXCEPTION);
Review Comment:
We should update OM in the PR as well to handle the SCM exception with this result code and return a propagate a coherent `OMException` back to the user, probably with result code `UNSUPPORTED_OPERATION` like #10777 does when the SCM versions don't match.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15715. Add helpful CLI flags to finalization CLIs [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #10678:
URL: https://github.com/apache/ozone/pull/10678#discussion_r3639059212
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/upgrade/FinalizeSubCommand.java:
##########
@@ -51,11 +65,70 @@ public Integer call() throws Exception {
}
client.finalizeUpgrade();
out().println("Cluster finalization has been started. Monitor progress with `ozone admin upgrade status`");
+
+ if (wait) {
Review Comment:
I did that, also adjusted to retry if there is an issue
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15715. Add helpful CLI flags to finalization CLIs [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on PR #10678:
URL: https://github.com/apache/ozone/pull/10678#issuecomment-5072546254
Test failure is not related to this change. I documented it in https://issues.apache.org/jira/browse/HDDS-15966 and amrunning the fix 100 times on my fork.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15715. Add helpful CLI flags to finalization CLIs [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #10678:
URL: https://github.com/apache/ozone/pull/10678#discussion_r3578850254
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java:
##########
@@ -154,6 +154,8 @@ default int getAllNodeCount() {
default DatanodeFinalizationCounts getDatanodeFinalizationCounts() {
int finalizedNodes = 0;
int totalHealthyNodes = 0;
+ int minApparentVersion = 0;
Review Comment:
Nice catch, changed it and added unit test for it. If there is no healthy DN reporting apparent versions it is changed to 0 in the end.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15715. Add helpful CLI flags to finalization CLIs [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #10678:
URL: https://github.com/apache/ozone/pull/10678#discussion_r3578826614
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java:
##########
@@ -507,5 +524,13 @@ public int getTotalHealthyDatanodes() {
public boolean allNodesFinalized() {
return numFinalizedDatanodes == totalHealthyDatanodes;
}
+
+ public Integer getMinApparentVersion() {
Review Comment:
It was a leftover, forgot to change it here.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15715. Add helpful CLI flags to finalization CLIs [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #10678:
URL: https://github.com/apache/ozone/pull/10678#discussion_r3546282455
##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -433,6 +433,10 @@ message UpgradeStatus {
optional int32 numDatanodesFinalized = 2;
optional int32 numDatanodesTotal = 3;
optional bool shouldFinalize = 4;
+ optional uint32 scmSoftwareVersion = 5;
Review Comment:
This will only be the leader SCM's software version. Unlike apparent version, these may not always match between SCMs. Maybe we should only return apparent versions of all components with the verbose option, not software version?
Note that software version can also be misleading if there was no component version change across an upgrade. In that case all versions can match even if the cluster is in mixed versions.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java:
##########
@@ -180,6 +182,14 @@ default DatanodeFinalizationCounts getDatanodeFinalizationCounts() {
ComponentVersion dnApparentVersion = datanodeInfo.getLastKnownApparentVersion();
ComponentVersion dnSoftwareVersion = datanodeInfo.getLastKnownSoftwareVersion();
+ int dnApparentVersionInt = dnApparentVersion.serialize();
+ if (dnApparentVersionInt < minApparentVersion) {
+ minApparentVersion = dnApparentVersionInt;
+ }
+ if (dnApparentVersionInt > maxApparentVersion) {
+ maxApparentVersion = dnApparentVersionInt;
+ }
Review Comment:
nit.
```suggestion
minApparentVersion = Math.min(minApparentVersion, dnApparentVersionInt);
maxApparentVersion = Math.max(maxApparentVersion, dnApparentVersionInt);
```
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/upgrade/StatusSubCommand.java:
##########
@@ -51,20 +60,150 @@ public Integer call() throws Exception {
"`ozone admin scm finalizationstatus` and `ozone admin om finalizationstatus`");
return 1;
}
- OzoneManagerProtocolProtos.QueryUpgradeStatusResponse status = client.queryUpgradeStatus();
-
+ QueryUpgradeStatusResponse status = client.queryUpgradeStatus();
- out().println("Upgrade status:");
- out().println(" OM Finalized? " + status.getOmFinalized());
- out().println(" SCM Finalized? " + status.getHddsStatus().getScmFinalized());
- out().println(" Datanodes finalized: " + status.getHddsStatus().getNumDatanodesFinalized()
- + "/" + status.getHddsStatus().getNumDatanodesTotal());
+ if (json) {
+ out().println(JsonUtils.toJsonStringWithDefaultPrettyPrinter(UpgradeStatusDto.from(status)));
+ } else if (isVerbose()) {
+ printVerbose(status, out());
+ } else {
+ printBasic(status, out());
+ }
}
return 0;
}
+ /** Basic, non-verbose human-readable status. */
+ static void printBasic(QueryUpgradeStatusResponse status, PrintWriter out) {
+ out.println("Upgrade status:");
+ out.println(" OM Finalized? " + status.getOmFinalized());
+ out.println(" SCM Finalized? " + status.getHddsStatus().getScmFinalized());
+ out.println(" Datanodes finalized: " + status.getHddsStatus().getNumDatanodesFinalized()
+ + "/" + status.getHddsStatus().getNumDatanodesTotal());
+ }
+
+ /**
+ * Verbose human-readable status that includes the software/apparent versions reported by OM
+ * and SCM, and the range of apparent versions across healthy datanodes (last known from heartbeat).
+ * Version fields that are not populated by the server (e.g. when running against an older
+ * cluster that does not yet ship these fields) are reported as "(unavailable)".
+ */
+ static void printVerbose(QueryUpgradeStatusResponse status, PrintWriter out) {
+ HddsProtos.UpgradeStatus hdds = status.getHddsStatus();
+ out.println("Upgrade status:");
+ out.println(" OM Finalized? " + status.getOmFinalized());
+ out.println(" OM Software Version: " + formatOmVersion(status.hasOmSoftwareVersion(),
+ status.getOmSoftwareVersion()));
+ out.println(" OM Apparent Version: " + formatOmVersion(status.hasOmApparentVersion(),
+ status.getOmApparentVersion()));
+ out.println(" SCM Finalized? " + hdds.getScmFinalized());
+ out.println(" SCM Software Version: " + formatHddsVersion(hdds.hasScmSoftwareVersion(),
+ hdds.getScmSoftwareVersion()));
+ out.println(" SCM Apparent Version: " + formatHddsVersion(hdds.hasScmApparentVersion(),
+ hdds.getScmApparentVersion()));
+ out.println(" Datanodes finalized: " + hdds.getNumDatanodesFinalized() + "/" + hdds.getNumDatanodesTotal());
+ out.println(" Min DN Apparent Version: " + formatHddsVersion(hdds.hasMinDatanodeApparentVersion(),
Review Comment:
```suggestion
out.println(" Min Datanode Apparent Version: " + formatHddsVersion(hdds.hasMinDatanodeApparentVersion(),
```
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java:
##########
@@ -507,5 +524,13 @@ public int getTotalHealthyDatanodes() {
public boolean allNodesFinalized() {
return numFinalizedDatanodes == totalHealthyDatanodes;
}
+
+ public Integer getMinApparentVersion() {
Review Comment:
Why Integer object instead of int primitive for these getters?
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java:
##########
@@ -194,7 +204,8 @@ default DatanodeFinalizationCounts getDatanodeFinalizationCounts() {
}
}
- return new DatanodeFinalizationCounts(finalizedNodes, totalHealthyNodes);
+ return new DatanodeFinalizationCounts(finalizedNodes, totalHealthyNodes,
+ minApparentVersion, maxApparentVersion);
Review Comment:
Optional, but might be better to make this a builder so all the int parameters don't accidentally get swapped.
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/upgrade/StatusSubCommand.java:
##########
@@ -51,20 +60,150 @@ public Integer call() throws Exception {
"`ozone admin scm finalizationstatus` and `ozone admin om finalizationstatus`");
return 1;
}
- OzoneManagerProtocolProtos.QueryUpgradeStatusResponse status = client.queryUpgradeStatus();
-
+ QueryUpgradeStatusResponse status = client.queryUpgradeStatus();
- out().println("Upgrade status:");
- out().println(" OM Finalized? " + status.getOmFinalized());
- out().println(" SCM Finalized? " + status.getHddsStatus().getScmFinalized());
- out().println(" Datanodes finalized: " + status.getHddsStatus().getNumDatanodesFinalized()
- + "/" + status.getHddsStatus().getNumDatanodesTotal());
+ if (json) {
+ out().println(JsonUtils.toJsonStringWithDefaultPrettyPrinter(UpgradeStatusDto.from(status)));
+ } else if (isVerbose()) {
+ printVerbose(status, out());
+ } else {
+ printBasic(status, out());
+ }
}
return 0;
}
+ /** Basic, non-verbose human-readable status. */
+ static void printBasic(QueryUpgradeStatusResponse status, PrintWriter out) {
+ out.println("Upgrade status:");
+ out.println(" OM Finalized? " + status.getOmFinalized());
+ out.println(" SCM Finalized? " + status.getHddsStatus().getScmFinalized());
+ out.println(" Datanodes finalized: " + status.getHddsStatus().getNumDatanodesFinalized()
+ + "/" + status.getHddsStatus().getNumDatanodesTotal());
+ }
+
+ /**
+ * Verbose human-readable status that includes the software/apparent versions reported by OM
+ * and SCM, and the range of apparent versions across healthy datanodes (last known from heartbeat).
+ * Version fields that are not populated by the server (e.g. when running against an older
+ * cluster that does not yet ship these fields) are reported as "(unavailable)".
+ */
+ static void printVerbose(QueryUpgradeStatusResponse status, PrintWriter out) {
+ HddsProtos.UpgradeStatus hdds = status.getHddsStatus();
+ out.println("Upgrade status:");
+ out.println(" OM Finalized? " + status.getOmFinalized());
+ out.println(" OM Software Version: " + formatOmVersion(status.hasOmSoftwareVersion(),
+ status.getOmSoftwareVersion()));
+ out.println(" OM Apparent Version: " + formatOmVersion(status.hasOmApparentVersion(),
+ status.getOmApparentVersion()));
+ out.println(" SCM Finalized? " + hdds.getScmFinalized());
+ out.println(" SCM Software Version: " + formatHddsVersion(hdds.hasScmSoftwareVersion(),
+ hdds.getScmSoftwareVersion()));
+ out.println(" SCM Apparent Version: " + formatHddsVersion(hdds.hasScmApparentVersion(),
+ hdds.getScmApparentVersion()));
+ out.println(" Datanodes finalized: " + hdds.getNumDatanodesFinalized() + "/" + hdds.getNumDatanodesTotal());
+ out.println(" Min DN Apparent Version: " + formatHddsVersion(hdds.hasMinDatanodeApparentVersion(),
+ hdds.getMinDatanodeApparentVersion()));
+ out.println(" Max DN Apparent Version: " + formatHddsVersion(hdds.hasMaxDatanodeApparentVersion(),
+ hdds.getMaxDatanodeApparentVersion()));
+ }
+
+ private static String formatOmVersion(boolean present, int serialized) {
Review Comment:
We already have a server version check at the top of this command, so by the time we get here we can assume that all fields are present according to the ZDU supported server's contract.
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/upgrade/StatusSubCommand.java:
##########
@@ -51,20 +60,150 @@ public Integer call() throws Exception {
"`ozone admin scm finalizationstatus` and `ozone admin om finalizationstatus`");
return 1;
}
- OzoneManagerProtocolProtos.QueryUpgradeStatusResponse status = client.queryUpgradeStatus();
-
+ QueryUpgradeStatusResponse status = client.queryUpgradeStatus();
- out().println("Upgrade status:");
- out().println(" OM Finalized? " + status.getOmFinalized());
- out().println(" SCM Finalized? " + status.getHddsStatus().getScmFinalized());
- out().println(" Datanodes finalized: " + status.getHddsStatus().getNumDatanodesFinalized()
- + "/" + status.getHddsStatus().getNumDatanodesTotal());
+ if (json) {
+ out().println(JsonUtils.toJsonStringWithDefaultPrettyPrinter(UpgradeStatusDto.from(status)));
+ } else if (isVerbose()) {
+ printVerbose(status, out());
+ } else {
+ printBasic(status, out());
+ }
}
return 0;
}
+ /** Basic, non-verbose human-readable status. */
+ static void printBasic(QueryUpgradeStatusResponse status, PrintWriter out) {
+ out.println("Upgrade status:");
+ out.println(" OM Finalized? " + status.getOmFinalized());
+ out.println(" SCM Finalized? " + status.getHddsStatus().getScmFinalized());
+ out.println(" Datanodes finalized: " + status.getHddsStatus().getNumDatanodesFinalized()
+ + "/" + status.getHddsStatus().getNumDatanodesTotal());
+ }
+
+ /**
+ * Verbose human-readable status that includes the software/apparent versions reported by OM
+ * and SCM, and the range of apparent versions across healthy datanodes (last known from heartbeat).
+ * Version fields that are not populated by the server (e.g. when running against an older
+ * cluster that does not yet ship these fields) are reported as "(unavailable)".
+ */
+ static void printVerbose(QueryUpgradeStatusResponse status, PrintWriter out) {
+ HddsProtos.UpgradeStatus hdds = status.getHddsStatus();
+ out.println("Upgrade status:");
+ out.println(" OM Finalized? " + status.getOmFinalized());
+ out.println(" OM Software Version: " + formatOmVersion(status.hasOmSoftwareVersion(),
+ status.getOmSoftwareVersion()));
+ out.println(" OM Apparent Version: " + formatOmVersion(status.hasOmApparentVersion(),
+ status.getOmApparentVersion()));
+ out.println(" SCM Finalized? " + hdds.getScmFinalized());
+ out.println(" SCM Software Version: " + formatHddsVersion(hdds.hasScmSoftwareVersion(),
+ hdds.getScmSoftwareVersion()));
+ out.println(" SCM Apparent Version: " + formatHddsVersion(hdds.hasScmApparentVersion(),
+ hdds.getScmApparentVersion()));
+ out.println(" Datanodes finalized: " + hdds.getNumDatanodesFinalized() + "/" + hdds.getNumDatanodesTotal());
+ out.println(" Min DN Apparent Version: " + formatHddsVersion(hdds.hasMinDatanodeApparentVersion(),
+ hdds.getMinDatanodeApparentVersion()));
+ out.println(" Max DN Apparent Version: " + formatHddsVersion(hdds.hasMaxDatanodeApparentVersion(),
Review Comment:
```suggestion
out.println(" Max Datanode Apparent Version: " + formatHddsVersion(hdds.hasMaxDatanodeApparentVersion(),
```
##########
hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/ozone/admin/upgrade/TestFinalizeSubCommand.java:
##########
@@ -119,6 +130,152 @@ public void testNonZduServerPrintsErrorAndReturnsNonZero() throws Exception {
verify(omClient, never()).finalizeUpgrade();
}
+ @Test
+ public void testWithoutWaitFlagDoesNotPollStatus() throws Exception {
+ new CommandLine(cmd).parseArgs();
+ assertEquals(0, cmd.call());
+
+ verify(omClient, never()).queryUpgradeStatus();
+ }
+
+ @Test
+ public void testWaitFlagCompletesImmediatelyWhenAlreadyFinalized() throws Exception {
+ when(omClient.queryUpgradeStatus()).thenReturn(finalizedStatus(3, 3));
+
+ new CommandLine(cmd).parseArgs("--wait");
+ assertEquals(0, cmd.call());
+
+ String output = outContent.toString(DEFAULT_ENCODING);
+ assertTrue(output.contains("Finalization complete."));
+ verify(omClient).finalizeUpgrade();
+ verify(omClient, times(1)).queryUpgradeStatus();
+ }
+
+ @Test
+ public void testWaitFlagPollsUntilFinalized() throws Exception {
+ when(omClient.queryUpgradeStatus())
+ .thenReturn(inProgressStatus(0, 3))
+ .thenReturn(inProgressStatus(2, 3))
+ .thenReturn(finalizedStatus(3, 3));
+
+ new CommandLine(cmd).parseArgs("--wait");
+ assertEquals(0, cmd.call());
+
+ String output = outContent.toString(DEFAULT_ENCODING);
+ assertTrue(output.contains("Waiting for finalization"));
+ assertTrue(output.contains("Finalization complete."));
+ verify(omClient, times(3)).queryUpgradeStatus();
+ }
+
+ @Test
+ public void testWaitFlagInterruptIsHandledCleanly() throws Exception {
+ // Make the poll interval long enough that the interrupt lands during sleep.
+ cmd.setPollIntervalMillis(60_000);
+ when(omClient.queryUpgradeStatus()).thenReturn(inProgressStatus(0, 3));
+
+ new CommandLine(cmd).parseArgs("--wait");
+
+ Thread runner = new Thread(() -> {
+ try {
+ cmd.call();
+ } catch (Exception ignored) {
+ // assertion in main thread
+ }
+ });
+ runner.start();
+ // Give the runner a moment to start sleeping.
+ Thread.sleep(50);
+ runner.interrupt();
+ runner.join(5_000);
+
+ String output = outContent.toString(DEFAULT_ENCODING);
+ assertTrue(output.contains("Waiting interrupted"));
+ verify(omClient, never()).queryUpgradeStatus();
Review Comment:
Once we remove the initial delay in the polling this should call the query command once.
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/upgrade/StatusSubCommand.java:
##########
@@ -51,20 +60,150 @@ public Integer call() throws Exception {
"`ozone admin scm finalizationstatus` and `ozone admin om finalizationstatus`");
return 1;
}
- OzoneManagerProtocolProtos.QueryUpgradeStatusResponse status = client.queryUpgradeStatus();
-
+ QueryUpgradeStatusResponse status = client.queryUpgradeStatus();
- out().println("Upgrade status:");
- out().println(" OM Finalized? " + status.getOmFinalized());
- out().println(" SCM Finalized? " + status.getHddsStatus().getScmFinalized());
- out().println(" Datanodes finalized: " + status.getHddsStatus().getNumDatanodesFinalized()
- + "/" + status.getHddsStatus().getNumDatanodesTotal());
+ if (json) {
+ out().println(JsonUtils.toJsonStringWithDefaultPrettyPrinter(UpgradeStatusDto.from(status)));
+ } else if (isVerbose()) {
+ printVerbose(status, out());
+ } else {
+ printBasic(status, out());
+ }
}
return 0;
}
+ /** Basic, non-verbose human-readable status. */
+ static void printBasic(QueryUpgradeStatusResponse status, PrintWriter out) {
+ out.println("Upgrade status:");
+ out.println(" OM Finalized? " + status.getOmFinalized());
+ out.println(" SCM Finalized? " + status.getHddsStatus().getScmFinalized());
+ out.println(" Datanodes finalized: " + status.getHddsStatus().getNumDatanodesFinalized()
+ + "/" + status.getHddsStatus().getNumDatanodesTotal());
+ }
+
+ /**
+ * Verbose human-readable status that includes the software/apparent versions reported by OM
+ * and SCM, and the range of apparent versions across healthy datanodes (last known from heartbeat).
+ * Version fields that are not populated by the server (e.g. when running against an older
+ * cluster that does not yet ship these fields) are reported as "(unavailable)".
+ */
+ static void printVerbose(QueryUpgradeStatusResponse status, PrintWriter out) {
+ HddsProtos.UpgradeStatus hdds = status.getHddsStatus();
+ out.println("Upgrade status:");
+ out.println(" OM Finalized? " + status.getOmFinalized());
+ out.println(" OM Software Version: " + formatOmVersion(status.hasOmSoftwareVersion(),
+ status.getOmSoftwareVersion()));
+ out.println(" OM Apparent Version: " + formatOmVersion(status.hasOmApparentVersion(),
+ status.getOmApparentVersion()));
+ out.println(" SCM Finalized? " + hdds.getScmFinalized());
+ out.println(" SCM Software Version: " + formatHddsVersion(hdds.hasScmSoftwareVersion(),
+ hdds.getScmSoftwareVersion()));
+ out.println(" SCM Apparent Version: " + formatHddsVersion(hdds.hasScmApparentVersion(),
+ hdds.getScmApparentVersion()));
+ out.println(" Datanodes finalized: " + hdds.getNumDatanodesFinalized() + "/" + hdds.getNumDatanodesTotal());
+ out.println(" Min DN Apparent Version: " + formatHddsVersion(hdds.hasMinDatanodeApparentVersion(),
+ hdds.getMinDatanodeApparentVersion()));
+ out.println(" Max DN Apparent Version: " + formatHddsVersion(hdds.hasMaxDatanodeApparentVersion(),
+ hdds.getMaxDatanodeApparentVersion()));
+ }
+
+ private static String formatOmVersion(boolean present, int serialized) {
Review Comment:
I think we should still use the DTO below to decouple the json output from the proto format, but that can be simplified along these lines as well.
##########
hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/ozone/admin/upgrade/TestFinalizeSubCommand.java:
##########
@@ -119,6 +130,152 @@ public void testNonZduServerPrintsErrorAndReturnsNonZero() throws Exception {
verify(omClient, never()).finalizeUpgrade();
}
+ @Test
+ public void testWithoutWaitFlagDoesNotPollStatus() throws Exception {
+ new CommandLine(cmd).parseArgs();
+ assertEquals(0, cmd.call());
+
+ verify(omClient, never()).queryUpgradeStatus();
+ }
+
+ @Test
+ public void testWaitFlagCompletesImmediatelyWhenAlreadyFinalized() throws Exception {
+ when(omClient.queryUpgradeStatus()).thenReturn(finalizedStatus(3, 3));
+
+ new CommandLine(cmd).parseArgs("--wait");
+ assertEquals(0, cmd.call());
+
+ String output = outContent.toString(DEFAULT_ENCODING);
+ assertTrue(output.contains("Finalization complete."));
+ verify(omClient).finalizeUpgrade();
+ verify(omClient, times(1)).queryUpgradeStatus();
+ }
+
+ @Test
+ public void testWaitFlagPollsUntilFinalized() throws Exception {
+ when(omClient.queryUpgradeStatus())
+ .thenReturn(inProgressStatus(0, 3))
+ .thenReturn(inProgressStatus(2, 3))
+ .thenReturn(finalizedStatus(3, 3));
+
+ new CommandLine(cmd).parseArgs("--wait");
+ assertEquals(0, cmd.call());
+
+ String output = outContent.toString(DEFAULT_ENCODING);
+ assertTrue(output.contains("Waiting for finalization"));
+ assertTrue(output.contains("Finalization complete."));
+ verify(omClient, times(3)).queryUpgradeStatus();
+ }
+
+ @Test
+ public void testWaitFlagInterruptIsHandledCleanly() throws Exception {
+ // Make the poll interval long enough that the interrupt lands during sleep.
+ cmd.setPollIntervalMillis(60_000);
+ when(omClient.queryUpgradeStatus()).thenReturn(inProgressStatus(0, 3));
+
+ new CommandLine(cmd).parseArgs("--wait");
+
+ Thread runner = new Thread(() -> {
+ try {
+ cmd.call();
+ } catch (Exception ignored) {
+ // assertion in main thread
Review Comment:
We can still make sure this failed with the correct result, either by checking the exception type or the thread's interrupt flag.
##########
hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/ozone/admin/upgrade/TestFinalizeSubCommand.java:
##########
@@ -119,6 +130,152 @@ public void testNonZduServerPrintsErrorAndReturnsNonZero() throws Exception {
verify(omClient, never()).finalizeUpgrade();
}
+ @Test
+ public void testWithoutWaitFlagDoesNotPollStatus() throws Exception {
+ new CommandLine(cmd).parseArgs();
+ assertEquals(0, cmd.call());
+
+ verify(omClient, never()).queryUpgradeStatus();
+ }
+
+ @Test
+ public void testWaitFlagCompletesImmediatelyWhenAlreadyFinalized() throws Exception {
+ when(omClient.queryUpgradeStatus()).thenReturn(finalizedStatus(3, 3));
+
+ new CommandLine(cmd).parseArgs("--wait");
+ assertEquals(0, cmd.call());
+
+ String output = outContent.toString(DEFAULT_ENCODING);
+ assertTrue(output.contains("Finalization complete."));
+ verify(omClient).finalizeUpgrade();
+ verify(omClient, times(1)).queryUpgradeStatus();
+ }
+
+ @Test
+ public void testWaitFlagPollsUntilFinalized() throws Exception {
+ when(omClient.queryUpgradeStatus())
+ .thenReturn(inProgressStatus(0, 3))
+ .thenReturn(inProgressStatus(2, 3))
+ .thenReturn(finalizedStatus(3, 3));
+
+ new CommandLine(cmd).parseArgs("--wait");
+ assertEquals(0, cmd.call());
+
+ String output = outContent.toString(DEFAULT_ENCODING);
+ assertTrue(output.contains("Waiting for finalization"));
+ assertTrue(output.contains("Finalization complete."));
+ verify(omClient, times(3)).queryUpgradeStatus();
+ }
+
+ @Test
+ public void testWaitFlagInterruptIsHandledCleanly() throws Exception {
+ // Make the poll interval long enough that the interrupt lands during sleep.
+ cmd.setPollIntervalMillis(60_000);
+ when(omClient.queryUpgradeStatus()).thenReturn(inProgressStatus(0, 3));
+
+ new CommandLine(cmd).parseArgs("--wait");
+
+ Thread runner = new Thread(() -> {
+ try {
+ cmd.call();
+ } catch (Exception ignored) {
+ // assertion in main thread
+ }
+ });
+ runner.start();
+ // Give the runner a moment to start sleeping.
+ Thread.sleep(50);
+ runner.interrupt();
+ runner.join(5_000);
+
+ String output = outContent.toString(DEFAULT_ENCODING);
+ assertTrue(output.contains("Waiting interrupted"));
+ verify(omClient, never()).queryUpgradeStatus();
+ }
+
+ @Test
+ public void testWaitFlagIsResumableAfterCancel() throws Exception {
+ // First invocation: block forever in Thread.sleep so we can interrupt.
+ cmd.setPollIntervalMillis(60_000);
+ // Second invocation is expected to see a finalized cluster on the first poll.
+ when(omClient.queryUpgradeStatus()).thenReturn(finalizedStatus(3, 3));
+
+ new CommandLine(cmd).parseArgs("--wait");
+
+ Thread runner = new Thread(() -> {
+ try {
+ cmd.call();
+ } catch (Exception ignored) {
+ // First call swallows interrupt cleanly; nothing to assert from the side thread.
+ }
+ });
+ runner.start();
+ Thread.sleep(50);
+ runner.interrupt();
+ runner.join(5_000);
+
+ String firstOutput = outContent.toString(DEFAULT_ENCODING);
+ assertTrue(firstOutput.contains("Cluster finalization has been started"));
+ assertTrue(firstOutput.contains("Waiting interrupted"));
+ // Side thread was interrupted during the initial sleep, so the first poll never happened.
+ verify(omClient, never()).queryUpgradeStatus();
+
+ // Second invocation — reset output capture and shrink the poll interval so it exits promptly.
+ outContent.reset();
+ cmd.setPollIntervalMillis(1);
+
+ new CommandLine(cmd).parseArgs("--wait");
+ assertEquals(0, cmd.call());
+
+ String secondOutput = outContent.toString(DEFAULT_ENCODING);
+ assertTrue(secondOutput.contains("Cluster finalization has been started"));
+ assertTrue(secondOutput.contains("Finalization complete."));
+ // finalizeUpgrade must have been issued on both runs (idempotent server-side).
+ verify(omClient, times(2)).finalizeUpgrade();
+ // queryUpgradeStatus only ran during the second invocation.
+ verify(omClient, times(1)).queryUpgradeStatus();
+ // The client is closed after each try-with-resources block.
+ verify(omClient, times(2)).close();
+ }
+
+ @Test
+ public void testWaitFlagWithVerbosePrintsFullStatus() throws Exception {
+ verbose = true;
+ when(omClient.queryUpgradeStatus())
+ .thenReturn(finalizedStatus(2, 2));
+
+ new CommandLine(cmd).parseArgs("--wait");
+ assertEquals(0, cmd.call());
+
+ String output = outContent.toString(DEFAULT_ENCODING);
+ // Verbose output uses the StatusSubCommand.printVerbose layout.
Review Comment:
I don't see any checks on verbose specific output here.
##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -433,6 +433,10 @@ message UpgradeStatus {
optional int32 numDatanodesFinalized = 2;
optional int32 numDatanodesTotal = 3;
optional bool shouldFinalize = 4;
+ optional uint32 scmSoftwareVersion = 5;
+ optional uint32 scmApparentVersion = 6;
+ optional uint32 minDatanodeApparentVersion = 7;
+ optional uint32 maxDatanodeApparentVersion = 8;
Review Comment:
In the OM proto we are using int32 for the version type. I don't have a strong preference but should keep them consistent.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/OMUpgradeTestUtils.java:
##########
@@ -71,12 +71,14 @@ public static void waitForFinalization(OzoneManagerProtocol omClient)
throws TimeoutException, InterruptedException {
waitFor(() -> {
try {
- UpgradeFinalization.StatusAndMessages statusAndMessages =
- omClient.queryUpgradeFinalizationProgress("finalize-test", false,
- false);
- System.out.println("Finalization Messages : " +
- statusAndMessages.msgs());
- return statusAndMessages.status().equals(FINALIZATION_DONE);
+ QueryUpgradeStatusResponse status = omClient.queryUpgradeStatus();
+ HddsProtos.UpgradeStatus hdds = status.getHddsStatus();
+ System.out.println("Finalization status: omFinalized=" + status.getOmFinalized()
Review Comment:
Since this is called during tests we should use a logger here.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java:
##########
@@ -154,6 +154,8 @@ default int getAllNodeCount() {
default DatanodeFinalizationCounts getDatanodeFinalizationCounts() {
int finalizedNodes = 0;
int totalHealthyNodes = 0;
+ int minApparentVersion = 0;
Review Comment:
This should either be initialized to `Integer.MAX_VALUE` or the first concrete DN version we get. Currently the min will always be reported as zero. Looks like there's a unit test gap for these SCM changes.
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/upgrade/FinalizeSubCommand.java:
##########
@@ -51,11 +65,65 @@ public Integer call() throws Exception {
}
client.finalizeUpgrade();
out().println("Cluster finalization has been started. Monitor progress with `ozone admin upgrade status`");
+
+ if (wait) {
+ return waitForFinalization(client);
+ }
}
return 0;
}
+ /**
+ * Polls the cluster status until OM, SCM and all healthy datanodes report finalized, the operator
+ * interrupts the command, or an RPC fails. {@code --wait} is safe to re-run: if the cluster is already
+ * finalized, the first poll returns done and the command exits 0.
+ */
+ private int waitForFinalization(OzoneManagerProtocol client) {
+ while (true) {
+ try {
+ Thread.sleep(pollIntervalMillis);
Review Comment:
We should check first then sleep to avoid waiting 5 seconds even when the cluster is already finalized.
##########
hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/ozone/admin/upgrade/TestFinalizeSubCommand.java:
##########
@@ -119,6 +130,152 @@ public void testNonZduServerPrintsErrorAndReturnsNonZero() throws Exception {
verify(omClient, never()).finalizeUpgrade();
}
+ @Test
+ public void testWithoutWaitFlagDoesNotPollStatus() throws Exception {
+ new CommandLine(cmd).parseArgs();
+ assertEquals(0, cmd.call());
+
+ verify(omClient, never()).queryUpgradeStatus();
+ }
+
+ @Test
+ public void testWaitFlagCompletesImmediatelyWhenAlreadyFinalized() throws Exception {
Review Comment:
The test does not actually check that the poll interval is not used when the cluster is already finalized. In the current version the wait flag actually doesn't complete immediately when the cluster is already finalized: it still waits 5 seconds.
```suggestion
public void testStatusCalledOnceWhenAlreadyFinalized() throws Exception {
```
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java:
##########
Review Comment:
We should block this query when SCM is in safemode just like the finalize command. This way we don't have to handle the zero datanode case.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15715. Add helpful CLI flags to finalization CLIs [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #10678:
URL: https://github.com/apache/ozone/pull/10678#discussion_r3639048051
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/upgrade/FinalizeSubCommand.java:
##########
@@ -51,11 +65,70 @@ public Integer call() throws Exception {
}
client.finalizeUpgrade();
out().println("Cluster finalization has been started. Monitor progress with `ozone admin upgrade status`");
+
+ if (wait) {
+ return waitForFinalization(client);
+ }
}
return 0;
}
+ /**
+ * Polls the cluster status until OM, SCM and all healthy datanodes report finalized, the operator
+ * interrupts the command, or an RPC fails. {@code --wait} is safe to re-run: if the cluster is already
+ * finalized, the first poll returns done and the command exits 0.
+ */
+ private int waitForFinalization(OzoneManagerProtocol client) {
+ while (true) {
+ QueryUpgradeStatusResponse status;
+ try {
+ status = client.queryUpgradeStatus();
+ } catch (Exception e) {
+ err().println("Failed to query upgrade status: " + e.getMessage()
+ + ". Use `ozone admin upgrade status` to monitor progress.");
+ return 1;
+ }
+
+ // Check if cluster is already finalized
+ if (isClusterFinalized(status)) {
+ out().println("Finalization complete.");
+ return 0;
+ }
+
+ if (isVerbose()) {
+ StatusSubCommand.printVerbose(status, out());
+ } else {
+ HddsProtos.UpgradeStatus hdds = status.getHddsStatus();
+ out().printf("Waiting for finalization: OM=%s, SCM=%s, datanodes=%d/%d%n",
+ status.getOmFinalized(), hdds.getScmFinalized(),
+ hdds.getNumDatanodesFinalized(), hdds.getNumDatanodesTotal());
+ }
+ out().flush();
+
+ // Finalization checks before sleeping, so an already-finalized cluster returns without waiting a poll interval.
+ try {
+ Thread.sleep(pollIntervalMillis);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ out().println("Waiting interrupted. Use `ozone admin upgrade status` to monitor progress.");
+ return 0;
+ }
+ }
+ }
+
+ static boolean isClusterFinalized(QueryUpgradeStatusResponse status) {
+ HddsProtos.UpgradeStatus hdds = status.getHddsStatus();
+ return status.getOmFinalized()
+ && hdds.getScmFinalized()
+ && hdds.getNumDatanodesFinalized() == hdds.getNumDatanodesTotal();
+ }
Review Comment:
I added `clusterFinalized` to the proto, it makes sense to go with this way
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15715. Add helpful CLI flags to finalization CLIs [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #10678:
URL: https://github.com/apache/ozone/pull/10678#discussion_r3641225809
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/upgrade/FinalizeSubCommand.java:
##########
@@ -50,12 +63,63 @@ public Integer call() throws Exception {
return 1;
}
client.finalizeUpgrade();
+ if (wait) {
+ out().println("Cluster finalization has been started. Waiting for the cluster to finalize; "
+ + "interrupt with Ctrl-C to stop waiting (finalization continues on the server).");
+ return waitForFinalization(client);
+ }
out().println("Cluster finalization has been started. Monitor progress with `ozone admin upgrade status`");
}
return 0;
}
+ /**
+ * Polls the cluster status until OM, SCM and all healthy datanodes report finalized, or the operator
+ * interrupts the command. A failed status query is reported to stderr and retried on the next poll,
+ * so transient RPC errors do not abort the wait. {@code --wait} is safe to re-run: if the cluster is
+ * already finalized, the first poll returns done and the command exits 0.
+ */
+ private int waitForFinalization(OzoneManagerProtocol client) {
+ while (true) {
+ QueryUpgradeStatusResponse status = null;
+ try {
+ status = client.queryUpgradeStatus();
+ } catch (Exception e) {
+ err().println("Failed to query upgrade status: " + e.getMessage()
+ + ". Retrying, or use `ozone admin upgrade status` to monitor progress.");
+ }
+
+ if (status != null) {
+ // Finalization checks before sleeping, so an already-finalized cluster returns without waiting.
+ if (status.getClusterFinalized()) {
+ out().println("Finalization complete.");
+ return 0;
+ }
+
+ if (isVerbose()) {
+ StatusSubCommand.printVerbose(status, out());
+ } else {
+ StatusSubCommand.printBasic(status, out());
+ }
+ out().flush();
+ }
+
+ try {
+ Thread.sleep(pollIntervalMillis);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ out().println("Waiting interrupted. Use `ozone admin upgrade status` to monitor progress.");
+ return 0;
Review Comment:
This should return 1. We should only return 0 with `--wait` if finalization was known to be successful.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -3664,11 +3665,27 @@ public void finalizeUpgrade() throws IOException {
@Override
public QueryUpgradeStatusResponse queryUpgradeStatus() throws IOException {
- HddsProtos.UpgradeStatus scmStatus = scmClient.getContainerClient().queryUpgradeStatus();
+ HddsProtos.UpgradeStatus scmStatus;
+ try {
+ scmStatus = scmClient.getContainerClient().queryUpgradeStatus();
+ } catch (SCMException e) {
+ // SCM refuses the query while in safe mode
+ if (e.getResult() == SCMException.ResultCodes.SAFE_MODE_EXCEPTION) {
+ throw new OMException(e.getMessage(), e, NOT_SUPPORTED_OPERATION);
+ }
Review Comment:
We need a similar check in the OM background service, otherwise we will get a full stack trace on every bg run when SCM is in safemode. Just one line indicating SCM is in safemode is enough.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15715. Add helpful CLI flags to finalization CLIs [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #10678:
URL: https://github.com/apache/ozone/pull/10678#discussion_r3640021087
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -3664,11 +3665,27 @@ public void finalizeUpgrade() throws IOException {
@Override
public QueryUpgradeStatusResponse queryUpgradeStatus() throws IOException {
- HddsProtos.UpgradeStatus scmStatus = scmClient.getContainerClient().queryUpgradeStatus();
+ HddsProtos.UpgradeStatus scmStatus;
+ try {
+ scmStatus = scmClient.getContainerClient().queryUpgradeStatus();
+ } catch (SCMException e) {
+ // SCM refuses the query while in safe mode
+ if (e.getResult() == SCMException.ResultCodes.SAFE_MODE_EXCEPTION) {
+ throw new OMException(e.getMessage(), e, NOT_SUPPORTED_OPERATION);
+ }
+ throw e;
+ }
+
+ boolean omFinalized = !versionManager.needsFinalization();
+ boolean clusterFinalized = omFinalized
+ && scmStatus.getScmFinalized()
+ && scmStatus.getNumDatanodesFinalized() == scmStatus.getNumDatanodesTotal();
Review Comment:
I think we should rename `shouldFinalize` to `hddsFinalized` in the proto and change this to `omFinalized && hddsFinalized`. That is clearer about the current state of the cluster, and doesn't duplicate the SCM finalization check within the OM.
##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -434,6 +434,9 @@ message UpgradeStatus {
optional int32 numDatanodesFinalized = 2;
optional int32 numDatanodesTotal = 3;
optional bool shouldFinalize = 4;
+ optional uint32 scmApparentVersion = 5;
Review Comment:
Let's add a comment to the above `UpgradeFinalizationStatus` that it is kept for compatibility with old clients, and that this proto is the currently active one.
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1668,6 +1668,9 @@ message QueryUpgradeStatusRequest {
message QueryUpgradeStatusResponse {
optional bool omFinalized = 1;
optional hadoop.hdds.UpgradeStatus hddsStatus = 2;
+ optional uint32 omApparentVersion = 3;
+ // True when OM, SCM and all healthy datanodes are finalized
+ optional bool clusterFinalized = 4;
Review Comment:
One nice thing about a feature branch is we can still change the ordering within protos unique to the branch. I put the OM fields last here so they will still be next to each other if more like software version are added later.
```suggestion
// True when OM, SCM and all healthy datanodes are finalized
optional bool clusterFinalized = 1;
optional hadoop.hdds.UpgradeStatus hddsStatus = 2;
optional bool omFinalized = 3;
optional uint32 omApparentVersion = 4;
```
##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -434,6 +434,9 @@ message UpgradeStatus {
optional int32 numDatanodesFinalized = 2;
optional int32 numDatanodesTotal = 3;
optional bool shouldFinalize = 4;
+ optional uint32 scmApparentVersion = 5;
+ optional uint32 minDatanodeApparentVersion = 6;
+ optional uint32 maxDatanodeApparentVersion = 7;
Review Comment:
This is an extension of the comments on OM and its proto. Here I grouped the fields by type instead of component since there is a mix of components in one message.
```suggestion
optional bool hddsFinalized = 1;
optional bool scmFinalized = 2;
optional int32 numDatanodesFinalized = 3;
optional int32 numDatanodesTotal = 4;
optional uint32 scmApparentVersion = 5;
optional uint32 minDatanodeApparentVersion = 6;
optional uint32 maxDatanodeApparentVersion = 7;
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15715. Add helpful CLI flags to finalization CLIs [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on PR #10678:
URL: https://github.com/apache/ozone/pull/10678#issuecomment-4970567546
Thanks for the updates. Can you check on the CI failures? This should re-run against the base branch as well since we just merged from master.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15715. Add helpful CLI flags to finalization CLIs [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #10678:
URL: https://github.com/apache/ozone/pull/10678#discussion_r3578853711
##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -433,6 +433,10 @@ message UpgradeStatus {
optional int32 numDatanodesFinalized = 2;
optional int32 numDatanodesTotal = 3;
optional bool shouldFinalize = 4;
+ optional uint32 scmSoftwareVersion = 5;
+ optional uint32 scmApparentVersion = 6;
+ optional uint32 minDatanodeApparentVersion = 7;
+ optional uint32 maxDatanodeApparentVersion = 8;
Review Comment:
I changed it in the OM proto to use `uint32`, that was used before for the layout versions too.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15715. Add helpful CLI flags to finalization CLIs [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #10678:
URL: https://github.com/apache/ozone/pull/10678#discussion_r3578823183
##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -433,6 +433,10 @@ message UpgradeStatus {
optional int32 numDatanodesFinalized = 2;
optional int32 numDatanodesTotal = 3;
optional bool shouldFinalize = 4;
+ optional uint32 scmSoftwareVersion = 5;
Review Comment:
Thanks for bringing this up, I decided to remove the software versions of the components in the verbose output, adjusted the PR description too.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15715. Add helpful CLI flags to finalization CLIs [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on PR #10678:
URL: https://github.com/apache/ozone/pull/10678#issuecomment-5069582707
Thanks for the review comments @errose28, I like the renaming and ordering of the protos, addressed everything in the latest commit.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15715. Add helpful CLI flags to finalization CLIs [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 merged PR #10678:
URL: https://github.com/apache/ozone/pull/10678
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org