HDDS-15034. Query SCM status for ozone admin upgrade status command #10084
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -121,44 +121,13 @@ private void waitForDatanodesToFinalize(SCMUpgradeFinalizationContext context) | |
| // SCM is no longer the leader by throwing NotLeaderException. | ||
| context.getSCMContext().getTermOfLeader(); | ||
|
|
||
| DatanodeFinalizationCounts datanodeFinalizationCounts = | ||
| getNumFinalizedDatanodes(nodeManager); | ||
| int finalizedNodes = datanodeFinalizationCounts.getNumFinalizedDatanodes(); | ||
| int totalHealthyNodes = datanodeFinalizationCounts.getTotalHealthyDatanodes(); | ||
| int unfinalizedNodes = datanodeFinalizationCounts.getNumUnfinalizedDatanodes(); | ||
|
|
||
| allDatanodesFinalized = unfinalizedNodes == 0; | ||
|
|
||
| if (!allDatanodesFinalized) { | ||
| LOG.info("Waiting for datanodes to finalize. Status: {}/{} healthy " + | ||
|
|
@@ -177,4 +146,76 @@ private void waitForDatanodesToFinalize(SCMUpgradeFinalizationContext context) | |
| } | ||
| } | ||
| } | ||
|
|
||
| public static DatanodeFinalizationCounts getNumFinalizedDatanodes( | ||
|
Contributor
There was a problem hiding this comment. This should be an instance method in |
||
| NodeManager nodeManager) { | ||
| int finalizedNodes = 0; | ||
| int totalHealthyNodes = 0; | ||
| int unfinalizedNodes = 0; | ||
|
|
||
| for (DatanodeDetails dn : nodeManager.getAllNodes()) { | ||
| try { | ||
| // Only check HEALTHY nodes. STALE/DEAD nodes will be told to | ||
| // finalize when they recover. | ||
| if (!nodeManager.getNodeStatus(dn).isHealthy()) { | ||
|
Contributor
There was a problem hiding this comment. This doesn't address the previous comment because it is only checking the health state returned by
Contributor
There was a problem hiding this comment. I did wonder if we should let any decommission(ed / ing) and maintenance modes finalize if they are healthy (meaning they are still heartbeating). If they are heartbeating they should either finalize or stop heartbeating and go dead. They can be forced to finalize if they ever transition back to in_service but I don't think that needs a re-register so it would be another code path to worry about later.
Contributor
There was a problem hiding this comment. I do think we should heartbeat to all nodes we can reach that they should finalize. We don't need to count all those nodes towards the HDDS finalization exit criteria, but maybe we should. It is simpler to reason that all live nodes have finalized, and we don't need to worry about decom/maintenance at all. I was considering a scenario where a decom node has just been shut down and might block finalization for the full heartbeat timeout, but that is only 10 minutes. Along the same lines, I'm wondering if we should also count stale nodes towards the total count and wait for them to either finalize or go dead. If we exclude them from the count, we cannot say that all registered nodes have finalized because stale nodes will not have to re-register when they heartbeat again. If we include them, there could be one or a few slow nodes that periodically go stale and are also not processing the commands which would hold up OM. Mostly I think we just need to make a decision on what states we count and why.
Contributor
There was a problem hiding this comment.
Yea, I think we can live with that. People should not be upgrading clusters with nodes in strange states IMO, as it just adds to potential problems. On stale nodes, there are: stale : unfinalized The second we can just count as a finalized node and not worry about it. The second is a problem. It can either go to dead and we can ignore it, or it can go back to healthy and may or may not finalize and go stale again. I guess if it is able to go stale -> healthy, it should must have heartbeat and hence picked up its finalize command, but it may not process it if the DN is in bad shape, and it may not heartbeat again. I think our upgrade write path design could handle a node that slips through un-finalized? Or do we put some extra logic in to fence it out until it finalizes? For a stale node, all its Ratis pipelines get closed on transition to stale anyway, so it is effectively out of the write path at that point. |
||
| continue; | ||
| } | ||
| totalHealthyNodes++; | ||
| DatanodeInfo datanodeInfo = nodeManager.getDatanodeInfo(dn); | ||
| if (datanodeInfo == null) { | ||
| LOG.warn("Could not get DatanodeInfo for {}, skipping in " + | ||
| "finalization wait.", dn.getHostName()); | ||
| continue; | ||
| } | ||
|
|
||
| LayoutVersionProto dnLayout = datanodeInfo.getLastKnownLayoutVersion(); | ||
| int dnMlv = dnLayout.getMetadataLayoutVersion(); | ||
| int dnSlv = dnLayout.getSoftwareLayoutVersion(); | ||
|
|
||
| if (dnMlv < dnSlv) { | ||
| // Datanode has not yet finalized | ||
| unfinalizedNodes++; | ||
| LOG.debug("Datanode {} has not yet finalized: MLV={}, SLV={}", | ||
| dn.getHostName(), dnMlv, dnSlv); | ||
| } else { | ||
| finalizedNodes++; | ||
| } | ||
| } catch (NodeNotFoundException e) { | ||
| // Node was removed while we were iterating. This is OK, skip it. | ||
| LOG.debug("Node {} not found while waiting for finalization, " + | ||
| "skipping.", dn); | ||
| } | ||
| } | ||
|
|
||
| return new DatanodeFinalizationCounts(finalizedNodes, totalHealthyNodes, unfinalizedNodes); | ||
| } | ||
|
|
||
| /** | ||
| * Class to store the number finalized, unfinalized and healthy datanodes. | ||
| */ | ||
| public static final class DatanodeFinalizationCounts { | ||
| private final int numFinalizedDatanodes; | ||
| private final int totalHealthyDatanodes; | ||
| private final int numUnfinalizedDatanodes; | ||
|
Comment on lines
+197
to
+199
Contributor
There was a problem hiding this comment. Do we actually need three fields here? I think we can just track total healthy in-service nodes and number of finalized nodes. The number of unfinalized nodes can be derived from that. These are the only two fields the upgrade status API uses. The finalization wait code can be changed to check that the healthy node count equals the finalized count. We could also wrap that in an
|
||
|
|
||
| public DatanodeFinalizationCounts(int numFinalizedDatanodes, | ||
| int totalHealthyDatanodes, | ||
| int numUnfinalizedDatanodes) { | ||
| this.numFinalizedDatanodes = numFinalizedDatanodes; | ||
| this.totalHealthyDatanodes = totalHealthyDatanodes; | ||
| this.numUnfinalizedDatanodes = numUnfinalizedDatanodes; | ||
| } | ||
|
|
||
| public int getNumFinalizedDatanodes() { | ||
| return numFinalizedDatanodes; | ||
| } | ||
|
|
||
| public int getTotalHealthyDatanodes() { | ||
| return totalHealthyDatanodes; | ||
| } | ||
|
|
||
| public int getNumUnfinalizedDatanodes() { | ||
| return numUnfinalizedDatanodes; | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This upgrade status enum is going to be removed in the new version manager. We will only use the proto facing version for compatibility with the older APIs. This call can be changed to
scm.getLayoutVersionManager().needsFinalization()which easily maps to the same call in the new version manager when we switch. From here we can:isScmFinalizedhelper methodDatanodeFinalizationCounts#allNodesFinalizedmethod as suggested hereshouldFinalizemethod to just a variable:shouldFinalize = scmFinalized && datanodeFinalizationCounts.allNodesFinalized()buildUpgradeStatusto invoke the builder directly in this method since the labelled builder setters are clearer than the unlabeled method parameters and it is now just a passthrough.This minimizes the amount of upgrade specific logic in the server side translator.