Repository navigation
HDDS-16798. Stale or Dead Datanodes whose software is not upgraded may still communicate with a finalized SCM - #11692
Conversation
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
4 open issues
This gate is intended to prevent STALE/DEAD nodes from updating their heartbeat timestamp unless… · New SwallowingNodeNotFoundExceptionwithout any logging can make operational troubleshooting… · New Given the dispatcher’s new behavior depends onheartbeat.hasDatanodeVersion(), add a unit test… · NewdnIDis computed but not used foraddDatanodeCommand, which repeatsdatanodeDetails.getID().… · New
What changed in this PR
Ensures SCM finalization fences stale/dead datanodes running incompatible software by forcing re-registration on heartbeat, and adds unit tests to validate the behavior.
Changes:
- Add heartbeat-time version gating for unhealthy (STALE/DEAD) datanodes and issue
ReregisterCommandwhen incompatible. - Expose
shouldFenceDatanodeviaNodeManagerAPI and update implementations to be callable from the dispatcher. - Add unit tests for stale/dead nodes with incompatible versions.
| File | Description |
|---|---|
| hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconNodeManager.java | Makes shouldFenceDatanode publicly callable to align with the new NodeManager API. |
| hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMDatanodeHeartbeatDispatcher.java | Adds unhealthy-node version gate and triggers re-registration when incompatible. |
| hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java | Introduces shouldFenceDatanode as a default interface method for version-based fencing. |
| hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java | Updates shouldFenceDatanode visibility to satisfy/override the new interface method. |
| hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestSCMDatanodeHeartbeatDispatcher.java | Adds tests to ensure stale/dead incompatible nodes get a re-register command and heartbeat is not processed. |
Review effort: Lite
…y still communicate with a finalized SCM
…nodeIfNeeded for fenced nodes
|
@errose28 Thanks for the comments. I have hopefully address them all in the latest push. |
errose28
left a comment
There was a problem hiding this comment.
LGTM. Just one minor test comment that would be nice to fix before merge.
| } | ||
|
|
||
| @Test | ||
| public void testHealthyNodeWithVersionReportIsNeverFenced() throws Exception { |
There was a problem hiding this comment.
This test trivially passes regardless of the node status because MockNodeManager#shouldFenceDatanode always returns false. It needs to be mocked to return true to ensure that the status is actually what caused the node to be let through.
|
Thanks @sodonnel for the patch, @errose28 for the review.
Merged without this fixup because test-only changes are much cheaper for CI than doing another round here. Let's do it in follow-up or can include it in any related fix in the near future. |



What changes were proposed in this pull request?
Only all HEALTHY nodes need to be upgraded for SCM start finalization, but once SCM finalizes, it should not allow communication with nodes in the older software. However this is checked at the time of registration and STALE and DEAD nodes can heartbeat back to SCM without reregistering.
This is fixed by intercepting the heartbeat for stale and dead nodes and telling them to re-register. Then they are fenced using the same logic as newly started nodes.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16798
How was this patch tested?
New unit tests added