[jira] [Updated] (HDDS-17229) A heartbeat that arrives while SCM marks a stale datanode DEAD leaves it HEALTHY but missing from the network topology

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

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

Siyao Meng updated HDDS-17229:
------------------------------
    Attachment: CR-10-skip-stale-restore-after-node-marked-dead.patch

> 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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org