Siyao Meng created HDDS-17575:
---------------------------------
Summary: SCM that steps down while notifyLeaderReady runs stays
leader ready as a follower and is already ready when it is elected again
Key: HDDS-17575
URL: https://issues.apache.org/jira/browse/HDDS-17575
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
MC-1-set-scm-leader-ready-only-in-the-same-leader-term.patch,
TestBugMC1LeaderReadyAfterStepDown.java
h3. Mechanism
Ratis calls {{SCMStateMachine.notifyLeaderReady}} on the state machine apply
thread and {{SCMStateMachine.notifyNotLeader}} from {{changeToFollower}} on
another thread, and nothing orders the two. {{notifyLeaderReady}} flushes the
transaction buffer, reloads the deleted block summary and then calls
{{SCMContext.setLeaderReady}}, which sets the flag without checking that this
SCM is still the leader, or still in the same term. If the step down
({{updateLeaderAndTerm(false, 0)}}, which clears the flag) lands before that
call, the follower ends up with {{isLeader=false}} and {{isLeaderReady=true}}.
{{updateLeaderAndTerm(true, term)}} does not clear the flag, so it stays set
until the SCM next steps down from a leadership, including through the next
election of this SCM.
The {{SCMContext.isLeaderReady}} Javadoc says "True - it is leader and ready".
h3. Trigger
A step down of a newly elected SCM (higher term election, {{ozone admin scm
transfer}}, Ratis admin step down) that lands between Ratis declaring the
leader ready and {{setLeaderReady}}. On master since HDDS-16143 (fix version
2.3.0, not released yet), which moved the transaction buffer flush and the
deleted block summary reload into {{notifyLeaderReady}}, that window contains
both. In released versions it is only the few calls between Ratis' readiness
check and {{setLeaderReady}}, so the practical exposure is master. An earlier
run that stepped each new leader down as soon as it appeared, without timing
help, did not hit it in 5 tries, so in practice it needs a slow flush or
unlucky timing.
h3. Impact
* On the follower, every service gated on {{SCMContext.isLeaderReady}} alone
becomes RUNNING: {{ReplicationManager}}, {{SCMBlockDeletingService}},
{{BackgroundPipelineCreator}}, {{SecretKeyManagerService}}, the container
balancer check. Shown for the first two ({{shouldRun()}} true on the follower).
They cannot send datanode commands, because {{SCMContext.getTermOfLeader}}
throws on a follower, and Ratis writes fail there, so on the follower this is
wasted work and errors.
* {{SCMDatanodeProtocolServer.register}} uses the flag to decide whether a
follower's {{errorNodeNotPermitted}} is final (HDDS-17090). With the stale flag
a lagging follower's rejection is final again and the datanode shuts down (code
reading, not run).
* When the SCM is elected again it is leader ready from
{{updateLeaderAndTerm(true, term)}} on, before it has applied the previous
terms' entries and before that term's {{notifyLeaderReady}} reloads the deleted
block summary, which is the ordering HDDS-10690 established. The reproduction
shows the flag already true inside the new term's {{notifyLeaderReady}}; the
earlier part follows from the code. {{ReplicationManager}} and
{{SCMBlockDeletingService}} are already RUNNING, so they also skip the
{{hdds.scm.wait.time.after.safemode.exit}} wait after the election.
h3. Reproduction
[^TestBugMC1LeaderReadyAfterStepDown.java] ({{hadoop-ozone/integration-test}},
target path and Maven command in the class comment) runs three SCMs, one OM and
three datanodes on unmodified source. A test side log4j appender holds the
apply thread of the new leader at the INFO line logged during
{{DeletedBlockLogImpl.onLeaderReady}}, after the flush and before
{{setLeaderReady}}, while leadership is moved away with the Ratis admin
{{transferLeadership}} API. The hold only changes timing. It passes on
unmodified source when the bug is present (35s):
{noformat}
After step down: ... ratis{role=FOLLOWER term=4 ready=false}
scmContext{isLeader=false isLeaderReady=true} ReplicationManager.shouldRun=true
SCMBlockDeletingService.shouldRun=true
Re-elected, inside notifyLeaderReady: ... ratis{role=LEADER term=5 ready=true}
scmContext{isLeader=true isLeaderReady=true} ...
{noformat}
In the same run the new leader held at the same point in its first election
reports {{isLeaderReady=false}}, as expected. With the patch the test fails:
the follower stays {{isLeaderReady=false}} and both services report
{{shouldRun()}} false.
h3. Patch
[^MC-1-set-scm-leader-ready-only-in-the-same-leader-term.patch], against
424482eb9cfc4b835e1bf73bcdaa9e7d70df59f0. It also applies to master at
257a71ab277.
{{SCMContext.setLeaderReady}} takes the term and sets the flag under its write
lock only if the SCM is still the leader of that term, and returns whether it
did. {{notifyLeaderReady}} reads the Ratis current term when it starts (the
same source {{notifyLeaderChanged}} uses) and notifies the services only if the
flag was set. Since {{notifyNotLeader}} updates {{SCMContext}} under the same
lock, either the step down comes first and the flag is not set, or it comes
after and clears it. A step down plus a new election inside the callback fails
the term check, and the new term's own {{notifyLeaderReady}} sets the flag. The
term is read when the callback starts, because the {{StateMachine}} API does
not pass the term of the entry Ratis checked; a step down and a re-election
that both fit between Ratis' check and that read would still pass, which needs
at least one election timeout with no Ozone code running (argued, not run).
Nothing changes for an SCM that stays leader. The existing test callers are
updated for the new signature.
Covered by {{testLeaderReadyNotSetWhenLeadershipChangesDuringLeaderReady}} in
the existing {{TestSCMStateMachine}} (step down during the flush, step down and
new term during the flush, no change). Without the change it fails because the
follower is leader ready. With it {{TestSCMStateMachine}}, {{TestSCMContext}},
{{TestContainerBalancer}}, {{TestSCMNodeManager}}, {{TestSCMHAManagerImpl}} and
{{TestReplicationManager}} pass (181 tests), checkstyle is clean.
Found by TLA+ model checking and code review of the OM and SCM Ratis state
machine integration (leader readiness, snapshot install and log failure
handling) under HDDS-15926, on commit 424482eb9cfc4b835e1bf73bcdaa9e7d70df59f0.
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
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
