[
https://issues.apache.org/jira/browse/HDDS-17222?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17222:
------------------------------
Attachment: MC-3-admin-monitor-read-persisted-state-of-stored-node.patch
> Decommission or maintenance never completes if the datanode registers again
> with a new version or ports before SCM sees its persisted state
> -------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17222
> URL: https://issues.apache.org/jira/browse/HDDS-17222
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments:
> MC-3-admin-monitor-read-persisted-state-of-stored-node.patch,
> TestBugMC3FrozenTrackedNode.java
>
>
> h3. Mechanism
> {{DatanodeAdminMonitorImpl}} keeps, in its {{TrackedNode}}, the
> {{DatanodeInfo}} object that was stored when the workflow started.
> {{processTransitioningNodes}} only moves the node on when SCM's operational
> state, looked up by node ID, equals {{getPersistedOpState()}} of that kept
> object ("Ensure the DN has received and persisted the current maint state").
> When the datanode registers again with a changed version, port set or
> address, {{NodeStateManager.updateNode}} stores a new {{DatanodeInfo}}, and
> from then on {{SCMNodeManager.updateDatanodeOpState}} writes the persisted
> state reported by heartbeats only to the new object. The kept object never
> changes, so the check stays false. Nothing replaces it: a repeated admin
> request is a no op because SCM already holds the new state, and
> {{onBecomeLeader}} and the re-track done by the {{NODE_ADDRESS_UPDATE}}
> handler go through {{startTrackingNode}}, whose {{Set.add}} keeps the old
> entry because {{TrackedNode}} equality is by node ID.
> h3. Trigger
> # Start decommission or maintenance of a datanode.
> # The datanode receives {{SetNodeOperationalStateCommand}} and persists the
> new state.
> # Before its next heartbeat (30s by default) it restarts with a new version,
> for example in a rolling upgrade, or with a changed port set, and registers
> again.
> h3. Impact
> The node stays DECOMMISSIONING or ENTERING_MAINTENANCE, and after the restart
> SCM and the datanode agree, so heartbeats carry no commands. It is only
> resolved by recommissioning the node, an SCM restart or a failover to another
> SCM (which build the tracked set again), or the node dying. A failback to the
> same SCM keeps the old entry, because the tracked set is not cleared when an
> SCM steps down. No data is lost. Run for decommission after a version change
> and after a port change, and for maintenance after a version change; the
> address change takes the same path but was not run.
> h3. Reproduction
> PASS on unmodified source, deterministic. [^TestBugMC3FrozenTrackedNode.java]
> ({{hadoop-hdds/server-scm}}) uses the real {{SCMNodeManager}},
> {{NodeDecommissionManager}} and {{DatanodeAdminMonitorImpl}} as leader,
> driven through {{register}}, {{processHeartbeat}}, {{decommissionNodes}} and
> {{startMaintenanceNodes}}, {{onBecomeLeader}} and the monitor's {{run}};
> container and replication managers are mocks and the node has no containers
> or pipelines. The datanode applies the command it gets and registers again
> with the new version or ports. The rows without a change are the control and
> complete (DECOMMISSIONED, IN_MAINTENANCE). In the other rows the node stays
> in the transitional state with the tracked object still at persisted
> IN_SERVICE while the stored one says DECOMMISSIONING or ENTERING_MAINTENANCE;
> a passing test means the defect is present.
> h3. Patch
> [^MC-3-admin-monitor-read-persisted-state-of-stored-node.patch], against
> b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. It also applies to master at
> 20d95ed37fe.
> The check in {{processTransitioningNodes}} reads the persisted state from the
> node currently stored in the {{NodeManager}} instead of from the tracked
> object; if the node is gone it throws {{NodeNotFoundException}}, which the
> loop already handles. Re-tracking the node would not be enough, because the
> version and port branches of {{register}} fire no event. In the test
> scaffolding {{SimpleMockNodeManager.getNode}} now returns the registered node
> instead of null, and {{setNodeStatus}} also updates the stored node's
> persisted state, as a heartbeat would.
> Covered in the existing {{TestDatanodeAdminMonitor}}, for decommission and
> maintenance, by {{testWorkflowCompletesAfterStoredNodeIsReplaced}} (the new
> stored node already has the persisted state, so the workflow completes) and
> {{testWorkflowWaitsForPersistedStateOfReplacedStoredNode}} (the datanode
> registered again before it persisted the state, so the workflow waits and
> completes only after the next heartbeat reports it). Without the change both
> fail with DECOMMISSIONING instead of DECOMMISSIONED and ENTERING_MAINTENANCE
> instead of IN_MAINTENANCE. With it the SCM node package tests (174),
> {{TestDecommissionAndMaintenance}} on a mini cluster (9, run with the patches
> for two related issues applied too) and checkstyle pass. With the patch the
> three bug rows of the reproduction fail because the node completes like the
> control. After an address change the tracked object still carries the old
> hostname, which the decommission metrics use as key; that is not changed here.
> This patch is safe on its own. Any change that makes SCM keep its own
> operational state when a datanode registers again with a new address, version
> or ports depends on it: with such a change alone, a datanode that is stopped
> right after decommission or maintenance starts, before it persists the new
> state, ends up in this stuck state, where today the request is dropped; with
> this patch as well 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, HDDS-16892, HDDS-16630 and HDDS-16633, other state
> lost through the same {{updateNode}} replacement. 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]
