Siyao Meng created HDDS-17219:
---------------------------------
Summary: Decommission monitor on a stepped down SCM can overwrite
the operational state reported by a datanode, so a later re-promotion drops the
decommission
Key: HDDS-17219
URL: https://issues.apache.org/jira/browse/HDDS-17219
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
MC-7-skip-admin-monitor-op-state-writes-on-follower-scm.patch,
TestBugMC7StaleFollowerMonitor.java
h3. Mechanism
The operational state of a datanode is not replicated through Ratis. The leader
SCM keeps it in memory and, when a heartbeat reports a different persisted
state, sends {{SetNodeOperationalStateCommand}} to the datanode. A follower SCM
copies the persisted state from each heartbeat into its own memory
({{SCMNodeManager.updateDatanodeOpState}}).
{{DatanodeAdminMonitorImpl}} runs on every SCM, and after a step down it keeps
the nodes it was tracking as leader. In {{processTransitioningNodes}} it reads
the node status, decides, and writes the new state later through
{{setNodeOpState}}, without checking that the SCM is still leader or that the
state is still the one it read. On a follower, when the read returns IN_SERVICE
(the datanode has not applied the decommission yet), the monitor aborts the
workflow and writes IN_SERVICE. If a heartbeat reporting DECOMMISSIONING is
applied between that read and the write, the write replaces DECOMMISSIONING
with IN_SERVICE and the node is no longer tracked.
If this SCM becomes leader again before the next heartbeat from that datanode,
{{NodeDecommissionManager.onBecomeLeader}} ({{continueAdminForNode}}) sees
IN_SERVICE and does not resume the workflow, and the next heartbeat makes it
send IN_SERVICE to the datanode, which applies it. The decommission was
accepted and persisted by the datanode, and is now dropped.
h3. Trigger
# SCM A, as leader, accepts a decommission, sets DECOMMISSIONING and sends the
command to the datanode; its monitor starts tracking the node.
# A steps down before the datanode reports DECOMMISSIONING. A heartbeat still
reporting IN_SERVICE is copied on A. This also needs the datanode to hold the
command past its next heartbeat to A: it normally applies a command about a
second after the heartbeat round that delivered it, so it has to be delayed
(for example a busy command thread, or an early heartbeat round triggered by
other events).
# A monitor tick on A reads IN_SERVICE. The datanode has meanwhile applied the
command, and its heartbeat reporting DECOMMISSIONING is applied on A before the
tick writes IN_SERVICE.
# A becomes leader again before the datanode's next heartbeat reaches it.
The interim leader must not process a heartbeat reporting DECOMMISSIONING while
it is leader; otherwise it sends IN_SERVICE itself and the decommission is lost
anyway. HDDS-5090, which added the follower copy and {{onBecomeLeader}},
documents and accepts that loss for a failover before the state reaches the
followers; here the state has already reached the datanode and this SCM, which
is what {{onBecomeLeader}} was added to preserve. HDDS-3204 (a new leader
starts from its own view) is related. The datanode must also take the command
off its queue before any heartbeat response from a leader of a newer term,
because that response carries the new term and {{StateContext.getNextCommand}}
then drops queued commands of an older term; in practice no newer leader may
answer the datanode in that heartbeat round, for example after a failed
election that A wins again (code reading). The read to write window is short (a
log line and an in-memory update) per tracked node and monitor tick, and A has
to lose and regain leadership within about one heartbeat interval (30s by
default). This is rare.
h3. Impact
An accepted decommission is silently undone: the datanode returns to IN_SERVICE
and the operator has to issue the decommission again. No data is lost. The
other writes of the monitor on a follower (DECOMMISSIONED, IN_MAINTENANCE,
IN_SERVICE at the end of maintenance, for a dead node or on recommission) do
not need the heartbeat race; they are only sent if the SCM is re-promoted
before the next heartbeat, and each is the decision the leader would make on
its next tick anyway. Only the abort write racing a heartbeat has a harmful
effect (code reading, not run).
h3. Reproduction
PASS, deterministic, unmodified source. [^TestBugMC7StaleFollowerMonitor.java]
({{hadoop-hdds/server-scm}}) uses the real {{SCMNodeManager}} (from
{{HddsTestUtils.getScm}}), {{NodeDecommissionManager}},
{{DatanodeAdminMonitorImpl}} and {{SCMContext}}. Leadership is changed with
{{SCMContext.updateLeaderAndTerm}} and {{onBecomeLeader}}, heartbeats are
{{processHeartbeat}} calls carrying the persisted state a datanode would
report, and there is no datanode process. The monitor sees the node manager
through a Mockito spy that, once, runs a heartbeat to completion on another
thread right after the monitor's status read; that is the only timing help. The
first test shows the state overwritten to IN_SERVICE and the re-promoted SCM
sending IN_SERVICE to the datanode. Two controls show the correct outcome (the
decommission resumes and completes) when the heartbeat arrives after the tick,
or when one more heartbeat arrives before re-promotion. The interim leader is
not modelled. A passing test means the defect is present.
h3. Patch
[^MC-7-skip-admin-monitor-op-state-writes-on-follower-scm.patch], against
b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. It applies to master at 20d95ed37fe
with a three way merge ({{git apply -3}}), because the lines next to the
monitor construction in {{NodeDecommissionManager}} changed.
{{DatanodeAdminMonitorImpl}} now gets the {{SCMContext}} and
{{setNodeOpState}}, the single place where the monitor writes the operational
state, returns without writing when {{SCMContext.isLeader()}} is false, as
{{continueAdminForNode}} already does. On a follower the state then always
comes from the datanode heartbeats, the monitor still stops tracking nodes as
before, and {{onBecomeLeader}} resumes the workflow from the reported state.
Behaviour on the leader and without SCM HA is unchanged. The check is at the
write, not at the start of the tick, so it also covers a tick that is running
when leadership is lost. One follower side change: an admin request that the
monitor had not yet applied when the SCM stepped down (for example a
recommission still queued for its next tick) is now dropped on that SCM as
well, as it already is on any other new leader (HDDS-5090); the operator
repeats it. Before the patch the follower wrote IN_SERVICE locally, and a
re-promotion before the next heartbeat pushed it to the datanode.
Covered by {{testFollowerDoesNotOverwriteOpStateReportedByHeartbeat}} in the
existing {{TestDatanodeAdminMonitor}}, which delivers the heartbeat right after
the monitor's status read on a follower and then checks that the workflow
resumes and completes after re-promotion. Without the leader check it fails
with IN_SERVICE instead of DECOMMISSIONING. With it
{{TestDatanodeAdminMonitor}}, {{TestNodeDecommissionManager}},
{{TestNodeDecommissionMetrics}} and {{TestSCMNodeManager}} pass, the mini
cluster {{TestDecommissionAndMaintenance}} passes (9 tests, three of them, the
SCM restart cases, on a rerun after a port clash on the shared test machine; it
runs a single SCM, so it covers the leader path only), and checkstyle is clean.
With the patch the first test of the reproduction fails because the state stays
DECOMMISSIONING, and the two controls still pass.
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. 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]
