Siyao Meng created HDDS-17268:
---------------------------------
Summary: Datanode ignores the heartbeat SCM term before term init
and updates it without the lock, so a stale SCM leader's command can run
Key: HDDS-17268
URL: https://issues.apache.org/jira/browse/HDDS-17268
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
h3. Mechanism
The datanode fences commands of a stale SCM leader with
{{StateContext.termOfLeaderSCM}} (HDDS-3988): {{getNextCommand}} drops a
command whose term is lower than the latest term seen. HDDS-7621 added the term
to heartbeat responses so that the datanode also follows a new leader that
sends no commands. Two defects in {{StateContext.updateTermOfLeaderSCM(long)}},
the method that applies the heartbeat term, weaken that fence.
* *Heartbeat term ignored before init.* The method returns without doing
anything while {{termOfLeaderSCM}} is empty, which it is after every datanode
start. {{initTermOfLeaderSCM}} later sets the term from the queued commands
only. If the first commands come from the old leader, the datanode settles on
the old term and runs them, although it had already received the new term in
heartbeat responses. The javadoc of {{initTermOfLeaderSCM}} says the datanode
must "detect latest leader SCM before handling any SCMCommand, so that it won't
be disturbed by stale leader SCM".
* *Unlocked check then set.* {{HeartbeatEndpointTask.processResponse}} calls
the method without {{StateContext.lock}}, while {{addCommand}} and
{{getNextCommand}} call it under the lock. Each SCM endpoint has its own
heartbeat thread. Two responses with terms 2 and 3 processed at the same time
can leave the term at 2 after 3 was applied, and {{getNextCommand}} then
returns a queued term 2 command instead of dropping it.
An old leader does deliver commands of its term. It stamps the term when it
creates a command and can create them until Ratis notifies it that it lost
leadership. {{CommandQueue}} is not cleared on step down, and
{{SCMNodeManager.processHeartbeat}} hands queued commands out on followers too,
without a term in the response.
h3. Trigger
Heartbeat term ignored before init: a datanode starts (or restarts) while SCM
leadership has moved from SCM A (term 1) to SCM B (term 2), and A still has
commands of term 1 queued for that datanode, or A has not yet stepped down. B
answers the datanode's heartbeats with term 2 but has no command for it yet.
A's command is then the first in the datanode's queue.
Unlocked update: two SCMs answer the same datanode as leader at the same time
(an old leader at term 2 that has not stepped down yet, the new leader at term
3), the two responses are processed concurrently and the term 2 write lands
between the read and the write of the term 3 update.
h3. Impact
* A command decided by the old leader runs on the datanode after the new leader
took over, for example a force delete of a container replica. In the
reproduction a force delete of term 1 deleted the replica after the datanode
had received five heartbeat responses with term 2. The new leader did not
decide that delete and does not know about it: when an SCM becomes leader,
{{ReplicationManager.notifyStatusChanged}} clears the pending replication and
delete ops, with the comment "They are no longer needed as the DN will discard
any commands when the term changes". Its own over replication handling can
therefore delete another replica of the same container, which is the double
delete of HDDS-16449, there described as masked by this datanode term gate.
Shown for one replica, multiple replicas and the resulting redundancy were not
exercised. Re-replication restores redundancy afterwards.
* The damage is bounded in time. ReplicationManager commands carry a datanode
deadline ({{hdds.scm.replication.event.timeout}} 12m minus
{{hdds.scm.replication.event.timeout.datanode.offset}} 6m) after which
{{DeleteContainerCommandHandler}} and {{ReplicationSupervisor}} drop them, so a
stale ReplicationManager delete runs only within about 6 minutes of its
creation, and the new leader's ReplicationManager waits
{{hdds.scm.wait.time.after.safemode.exit}} (5m) after it becomes ready before
it acts. The reproduction sends a command without a deadline.
* With the race, the term goes back from 3 to 2 until the next heartbeat of the
new leader (default interval 30s), and term 2 commands queued in that time are
dispatched. The race needs two SCMs answering as leader at once, so it is rare.
* A datanode that already has its term is not affected by the first defect; it
drops the same command ("Detect and drop a SCMCommand ... from stale leader
SCM").
h3. Reproduction
Both attached classes pass on unmodified source when the bug is present (target
path and Maven command in each class comment). Not run with real SCMs or a real
SCM failover.
* [^TestBugMC4StaleLeaderCommandBeforeTermInit.java]
({{hadoop-hdds/container-service}}): a real {{DatanodeStateMachine}} daemon
over Hadoop RPC against three {{ScmTestMock}} SCMs that put their term into the
heartbeat response only while they believe they lead, like
{{SCMDatanodeProtocolServer.sendHeartbeat}}. Two cases: the old leader still
believes it leads, or it is a follower that still holds the command. In both,
the term 1 force delete removes the container (about 15s).
* [^TestBugMC5HeartbeatTermRegression.java]
({{hadoop-hdds/container-service}}): two real {{HeartbeatEndpointTask}}s on
their own threads against mocked SCM proxies (term 2 with a close container
command, term 3 without). A Mockito spy on the {{StateContext}} holds the term
2 write between its check and its write until the term 3 update has finished,
which makes the race deterministic. The term ends at 2 and {{getNextCommand}}
returns the term 2 command.
h3. Patch
[^MC-4-use-heartbeat-term-before-term-init-and-lock-term-update.patch], against
b10a7d427bdf83807a7e48a1be507d3b02d5a503. It also applies to master at
e0de543cbe9.
While the term is not initialized, {{updateTermOfLeaderSCM(long)}} records the
highest term it is given, and {{initTermOfLeaderSCM}} uses the larger of that
and the highest queued command term. The method takes {{StateContext.lock}},
the lock its other two callers already hold, which also keeps recording and
init atomic. {{termOfLeaderSCM}} becomes volatile for the command handlers that
read it from their own threads. Nothing changes once the term is set, and init
still happens at the same moment.
This narrows the window but does not close it: if the datanode initializes its
term before it has processed a heartbeat response of the new leader (init only
needs a majority of SCMs in HEARTBEAT state), a command of the old term is
still accepted. Waiting for a leader heartbeat term before init would close it,
but would stop command processing when no SCM sends a term (an SCM older than
HDDS-7621, or no leader), so it is left out. Clearing {{CommandQueue}} when an
SCM steps down would remove the follower route on the SCM side.
Covered by {{dropsCommandOfStaleLeaderIfNewerTermWasSeenBeforeInit}},
{{initsTermFromCommandIfOlderTermWasSeenBeforeInit}} (negative case) and
{{concurrentTermUpdatesKeepHighestTerm}} in the existing {{TestStateContext}}.
Without the change the first fails with the term 1 command returned and the
third with term 2 instead of 3. With it {{TestStateContext}},
{{TestHeartbeatEndpointTask}}, {{TestHeartbeatEndpointTaskDnsRefresh}},
{{TestDeleteContainerCommandHandler}}, {{TestReplicationSupervisor}},
{{TestRunningDatanodeState}} and {{TestCloseContainerCommandHandler}} pass (113
tests), checkstyle is clean, and both attached reproductions fail because the
stale command is dropped.
Found by TLA+ model checking and code review of the datanode registration and
heartbeat paths under HDDS-15926, on commit
b10a7d427bdf83807a7e48a1be507d3b02d5a503. Checked against HDDS issues and
apache/ozone pull requests for duplicates before filing. Related but not
duplicates: HDDS-3988 (introduced the term fencing), HDDS-7621 (added the
heartbeat term and the early return), HDDS-5330 (stale commands in sub queues
after the term is set), HDDS-16449 (redundant delete after an SCM leadership
transfer, masked by the datanode term gate) and HDDS-16804
({{ReplicationSupervisor}} loses a command of the new leader behind an equal
task of the old leader). 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]
