[
https://issues.apache.org/jira/browse/HDDS-17229?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17229:
------------------------------
Attachment: TestBugCR10StaleRestoreTopology.java
> A heartbeat that arrives while SCM marks a stale datanode DEAD leaves it
> HEALTHY but missing from the network topology
> ----------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17229
> URL: https://issues.apache.org/jira/browse/HDDS-17229
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments: CR-10-skip-stale-restore-after-node-marked-dead.patch,
> TestBugCR10StaleRestoreTopology.java
>
>
> h3. Mechanism
> In {{NodeStateManager.checkNodesHealth}} the STALE case calls
> {{updateNodeState}} twice with the same STALE {{NodeStatus}} snapshot: first
> with the dead condition and TIMEOUT, then with the healthy condition and
> RESTORE. Each call reads the last heartbeat time again. If the first call
> marks the node DEAD, {{updateClusterMap}} removes it from the
> {{NetworkTopology}}. If a heartbeat is recorded before the second call
> ({{SCMNodeManager.processHeartbeat}} calls
> {{NodeStateManager.updateLastHeartbeatTime}} on an RPC handler thread, which
> does not coordinate with the health check), the second call moves the node to
> HEALTHY. It passes the STALE snapshot as the old status, and
> {{updateClusterMap}} only adds a node back when the old status is DEAD, so
> the node is not added back.
> Nothing adds it later. Apart from recovery from DEAD, the topology is only
> added to on first registration and on re-registration with a changed address,
> ports or version. Further heartbeats and health checks leave it out until the
> node dies and recovers again or SCM restarts.
> This came with HDDS-16623, which moved topology updates from the event
> handlers into {{updateNodeState}}. Before that,
> {{HealthyReadOnlyNodeHandler}}, which handled the event fired by RESTORE
> (then a STALE to HEALTHY_READONLY transition), added the node to the topology
> unconditionally, which covered this case. The defect exists only since
> HDDS-16623 (fix version 2.3.0, not yet released), so no released version is
> affected.
> h3. Trigger
> # A datanode stops heartbeating, goes STALE, and its last heartbeat becomes
> older than the dead interval.
> # The health check marks it DEAD and removes it from the topology.
> # A heartbeat from the datanode is recorded before the same health check
> reaches its RESTORE check.
> No fault injection is needed, but the window is narrow: it lies between two
> consecutive calls in one iteration of the health check loop (a node state map
> update, a topology removal under the topology write lock and an event
> dispatch), and only for a node whose heartbeat returns right at the dead
> deadline.
> h3. Impact
> The datanode is HEALTHY in SCM but absent from {{NetworkTopology}}: in the
> reproduction {{getNumOfLeafNode}} returns 0 and {{chooseRandom}} returns null
> with the node HEALTHY. Placement that picks nodes from the topology (for
> example {{SCMContainerPlacementRackAware}} and
> {{SCMContainerPlacementRackScatter}}) cannot choose it (argued from code,
> placement itself not run). The state is silent and persists until the node
> goes DEAD and recovers again, re-registers with changed details, or SCM
> restarts.
> h3. Reproduction
> PASS, deterministic, unmodified source.
> [^TestBugCR10StaleRestoreTopology.java] ({{hadoop-hdds/server-scm}}) uses the
> real {{NodeStateManager}} and {{NetworkTopologyImpl}} with default
> configuration and the periodic health check paused. It sets the last
> heartbeat time back past the stale and then the dead interval, and delivers a
> heartbeat through {{NodeStateManager.updateLastHeartbeatTime}} on a separate
> thread while the health check thread is held in its DEAD_NODE event publish.
> Afterwards the node is HEALTHY, missing from the topology, the events are
> DEAD_NODE then UNHEALTHY_TO_HEALTHY_NODE, and five more heartbeats and health
> checks do not change that. A passing test means the defect is present.
> h3. Patch
> [^CR-10-skip-stale-restore-after-node-marked-dead.patch], against
> b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. It also applies to master at
> 20d95ed37fe.
> In the STALE case the RESTORE check now runs only if the TIMEOUT check did
> not change the node ({{updateNodeState}} returns whether it applied a
> transition). For this interleaving the node stays DEAD until the next health
> check (3s by default), so it is handled like a node whose heartbeat arrives
> just after the health check: {{DeadNodeHandler}}, which skips a node that is
> no longer DEAD when it runs, now processes it as DEAD (closes its containers,
> destroys its pipelines, removes its replicas and notifies
> ReplicationManager), and the next check moves it to HEALTHY through the DEAD
> case and adds it back to the topology. This is the intended outcome, as the
> node was silent past the dead interval. Before the patch the RESTORE call ran
> microseconds after DEAD_NODE was fired, so the handler usually found the node
> HEALTHY and skipped it.
> Covered by
> {{testHeartbeatWhileMarkedDeadDoesNotLeaveHealthyNodeOutOfTopology}} in the
> existing {{TestNodeStateManager}}, which delivers the heartbeat from the
> DEAD_NODE publish. Without the change it fails because the node is not in the
> topology. With it, {{TestNodeStateManager}}, {{TestSCMNodeManager}},
> {{TestDeadNodeHandler}}, {{TestNodeDecommissionManager}},
> {{TestSCMNodeMetrics}}, {{TestNodeReportHandler}} and both
> {{TestContainerPlacement}} classes pass (110 tests), and checkstyle is clean.
> With the patch the reproduction fails because the node is DEAD after the
> racing health check; the regression test shows that the next check adds it
> back. HDDS-16633 has the same symptom from a different trigger
> (re-registration) and is not changed by this patch.
> Found by code review of the SCM datanode decommission and maintenance admin
> paths, as part of the TLA+ verification effort under HDDS-15926, on commit
> b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. 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]
