[
https://issues.apache.org/jira/browse/HDDS-17575?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17575:
------------------------------
Attachment: TestBugMC1LeaderReadyAfterStepDown.java
> 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
> Affects Versions: 1.2.0, 1.3.0, 1.2.1, 1.4.0, 2.0.0, 1.4.1, 2.1.0, 2.2.0,
> 2.1.1, 2.1.2, 2.2.1
> Reporter: Siyao Meng
> Priority: Minor
> 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]
