[jira] [Updated] (HDDS-17223) A datanode that registers again with a new address, version or ports overwrites the operational state held by the leader SCM

["Siyao Meng (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/HDDS-17223?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Siyao Meng updated HDDS-17223:
------------------------------
    Attachment: MC-1-keep-scm-op-state-on-datanode-reregistration.patch

> A datanode that registers again with a new address, version or ports overwrites the operational state held by the leader SCM
> ----------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-17223
>                 URL: https://issues.apache.org/jira/browse/HDDS-17223
>             Project: Apache Ozone
>          Issue Type: Bug
>            Reporter: Siyao Meng
>            Priority: Major
>         Attachments: MC-1-keep-scm-op-state-on-datanode-reregistration.patch, TestBugMC1ReRegisterOpState.java
>
>
> h3. Mechanism
> When a registered datanode registers again with a changed address, version or port set, {{SCMNodeManager.register}} calls {{updateNodeAndTopology}}, and {{NodeStateManager.updateNode}} builds the new {{DatanodeInfo}} with {{newNodeStatus}}, that is with the operational state and expiry the datanode reports from its {{datanode.id}}. That replaces the state held by the leader SCM. For a registered node the leader is meant to be the source of truth: the javadoc of {{SCMNodeManager.updateDatanodeOpState}} says so, and a re-registration without such a change keeps SCM's state and sends {{SetNodeOperationalStateCommand}} on the next heartbeat. After the overwrite SCM and the datanode agree, so the heartbeat never corrects it. The version and port branches fire no event, so nothing tracks the node afterwards. The address branch fires {{NODE_ADDRESS_UPDATE}}, whose handler calls {{NodeDecommissionManager.continueAdminForNode}} and tracks the node again.
> h3. Trigger
> The datanode's persisted state is older than SCM's whenever it restarts before the heartbeat that would update it, typically because it was stopped for an upgrade:
> # Recommission (or end maintenance for) a datanode while it is down. {{recommission}} does not check health, so SCM goes back to IN_SERVICE while {{datanode.id}} still says DECOMMISSIONING or IN_MAINTENANCE.
> # Start decommission or maintenance and stop the datanode before its next heartbeat, so it never persists the new state.
> # The datanode comes back with a new version (every rolling upgrade restart), a new address or a changed port set.
> By code reading, the monitor ending maintenance at its end time while the datanode is down ({{DatanodeAdminMonitorImpl.completeMaintenance}} does this whether the node is dead or not) leads to the same state as case 1 without any operator action. That variant was not run.
> h3. Impact
> All run with the real SCM on the leader:
> * Version or port change after a recommission: SCM goes back to DECOMMISSIONING, nothing tracks the node, and a repeated decommission request is a no op because SCM already says DECOMMISSIONING. The node stays like this until it is recommissioned again or SCM restarts or fails over, which resumes the decommission the operator cancelled (followers took DECOMMISSIONING from the same registration, and {{onBecomeLeader}} tracks the node again).
> * Address change after a recommission: the node is tracked again and the cancelled decommission completes, the node ends DECOMMISSIONED.
> * Maintenance ended while the datanode was down, and it comes back with a new version after its maintenance end time: SCM goes back to IN_MAINTENANCE with the old, already expired end time. Only the admin monitor ends an expired maintenance, and only for tracked nodes, so the node stays IN_MAINTENANCE.
> * Decommission or maintenance started and the datanode stopped right away for an upgrade: SCM goes back to IN_SERVICE and the monitor aborts the request ("Aborting admin for this node"). The maintenance variant was run separately and is not in the attached test.
> No data is lost, but the admin state is wrong and needs manual repair.
> h3. Reproduction
> PASS on unmodified source, deterministic. [^TestBugMC1ReRegisterOpState.java] ({{hadoop-hdds/server-scm}}) runs one real SCM as leader through {{SCMNodeManager.register}} and {{processHeartbeat}}, {{NodeDecommissionManager}} admin calls, the real {{NODE_ADDRESS_UPDATE}} handler and {{DatanodeAdminMonitorImpl.run}}. The simulated datanode changes its persisted state only when SCM sends {{SetNodeOperationalStateCommand}}. Four tests show the version, address, maintenance and stop right after decommission cases above, a fifth is the control without a change, which keeps IN_SERVICE and sends the command. A passing test means the defect is present, except for the control.
> h3. Patch
> [^MC-1-keep-scm-op-state-on-datanode-reregistration.patch], against b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. It also applies to master at 20d95ed37fe.
> {{NodeStateMap.updateNode}} now keeps the operational state and expiry of the existing entry when it installs the new {{DatanodeInfo}}, under the same write lock, so a concurrent admin or monitor update cannot be lost in between. {{NodeStateManager.updateNode}} no longer derives the status from the report. The reported persisted state is still stored, so the next heartbeat corrects the datanode as for a re-registration without changes. First registration, for example after an SCM restart, still takes the datanode's state. This keeps SCM owned state from the old entry in the same place where the container list of HDDS-16632 is lost, so a fix for that issue fits next to it.
> Covered by {{testReregistrationKeepsOperationalState}} in the existing {{TestSCMNodeManager}} (no change, address, version, ports), and the existing {{TestNodeStateManager.testUpdateNode}}, which asserted the old behaviour, now expects SCM's state; it now gives the maintenance an end time and checks that the end time is kept too. Without the change both fail for the three changing cases. With it the SCM node package tests (174), {{TestDecommissionAndMaintenance}} on a mini cluster (9, run with the patch for HDDS-17222 and one other datanode patch applied too) and checkstyle pass. With the patch the four bug tests of the reproduction fail and the control passes.
> Follower SCMs, and by code reading Recon, now also keep their own state at re-registration and take the datanode's state on the next heartbeat, as they already do for a re-registration without changes.
> This patch depends on HDDS-17222 and must not be committed without it; HDDS-17222 is safe on its own. With this patch alone, a datanode that is stopped right after decommission or maintenance starts, before it persists the new state, no longer has the request aborted with a warning but stays silently in DECOMMISSIONING or ENTERING_MAINTENANCE, because the admin monitor still reads the persisted state from the replaced node (HDDS-17222). With both patches it completes (run).
> Found by TLA+ model checking and code review of the SCM datanode decommission and maintenance admin paths under HDDS-15926, on commit b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. Checked against HDDS issues and apache/ozone pull requests for duplicates before filing. Related but not duplicates: HDDS-16632 (container list) and HDDS-16892 (pending container allocations), both lost through the same {{updateNode}} replacement, and HDDS-16630 and HDDS-16633 (topology on the same path). The attached patch is a proposal for review. Generated with Specula (Claude Opus 5.5).



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org