Siyao Meng created HDDS-17229:
---------------------------------
Summary: 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
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]
