[
https://issues.apache.org/jira/browse/HDDS-17223?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17223:
------------------------------
Attachment: TestBugMC1ReRegisterOpState.java
> 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: [email protected]
For additional commands, e-mail: [email protected]
