[
https://issues.apache.org/jira/browse/HDDS-16861?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16861:
------------------------------
Attachment: TestBugMC3QuotaRepairSkippedOnLeaderChange.java
> Upgrade quota repair is skipped on every OM if leadership moves before the
> leader applies CompleteFinalizeUpgrade
> -----------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16861
> URL: https://issues.apache.org/jira/browse/HDDS-16861
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments:
> MC-3-start-upgrade-quota-repair-on-next-ready-leader.patch,
> TestBugMC3QuotaRepairSkippedOnLeaderChange.java
>
>
> h3. Mechanism
> {{QuotaRepairUpgradeAction.execute}} starts {{QuotaRepairTask}} only if the
> OM applying {{CompleteFinalizeUpgrade}} is {{LEADER_AND_READY}} at that
> moment. Otherwise it logs "Skip quota repair operation during upgrade on the
> node as this is not a leader node." and returns, and
> {{ComponentVersionManager.finalizeUpgrade}} persists QUOTA as finalized.
> Followers always take that branch. If the OM that was leader at commit time
> is no longer the ready leader when its apply thread reaches the action, every
> OM takes it.
> Nothing starts the repair later. Every VERSION file is past QUOTA, so replay
> finds nothing to finalize. Repair state is in memory only, and nothing starts
> it on leader election. The code comment ("on leader node, repair will be
> triggered where finalize is called. For other nodes, it will be ignored.")
> assumes some OM is the ready leader when it applies the step.
> h3. Trigger
> # A cluster is pre-finalized at a layout before QUOTA, with
> {{ozone.om.upgrade.quota.recalculate.enabled}} at its default ({{true}}).
> # The admin finalizes and the leader commits {{CompleteFinalizeUpgrade}}.
> # Before the leader applies it, leadership moves, for example to a follower
> that has already applied it as a follower.
> The window between the commit and the leader's own apply is normally short.
> The reproduction widens it with a latch, and the leader change could come
> from a step down, a GC pause or a partition. Argued from code, not run: a
> leader crash after the commit has every OM apply the entry while not the
> ready leader, with the same result, and a restart of all OMs after the commit
> also loses the repair, because the repair state is in memory only.
> h3. Impact
> * No OM runs the quota recalculation that
> {{ozone.om.upgrade.quota.recalculate.enabled}} promises at upgrade, and the
> quota repair status reports no run (run).
> * Bucket and volume usage counters that drifted under releases before QUOTA
> stay as they were. The skip does not make counters wrong: a write that such a
> counter rejects after finalization was rejected before it too. {{ozone repair
> om quota start}} does the same repair by hand.
> * Affects 2.0.0 onward. The action and its follower skip are the same since
> 2.0.0, and in 2.x the action also runs while each OM applies the finalize
> request, so the same window exists (argued from code, not run).
> * Suggested priority Minor: it needs a leader change in a short window during
> the one time finalization, and it only skips a best effort correction that a
> documented command also does.
> h3. Reproduction
> PASS on the unmodified source.
> [^TestBugMC3QuotaRepairSkippedOnLeaderChange.java]
> ({{hadoop-ozone/integration-test}}) runs a 3 OM HA mini cluster pre-finalized
> at {{INITIAL_VERSION}}. The only test aid is a wrapper, installed through
> {{OMVersionManager.getUpgradeActionsForTesting}}, that holds the old leader's
> apply thread in front of the real, unchanged QUOTA action. The test then:
> # moves leadership with {{OzoneManager.transferLeadership}} to a follower
> that has already applied the entry;
> # releases the old leader;
> # writes through the new leader, so its leader ready notification has
> returned.
> It asserts that all 3 OMs logged the skip, all are at the software version in
> memory and in their VERSION files, and {{QuotaRepairTask.getStatus}} still
> reports no run. A passing test means the defect is present.
> h3. Patch
> [^MC-3-start-upgrade-quota-repair-on-next-ready-leader.patch], against
> a6b7bdb937109ba2893688b41a89470bc95c88cf. It also applies to master at
> b0aa6475b78.
> The patch defers the repair instead of dropping it:
> * An OM that finalizes QUOTA while it is not the ready leader still logs the
> skip and now marks the repair as pending in memory
> ({{OzoneManager.deferUpgradeQuotaRepair}}).
> * {{OzoneManagerStateMachine.notifyLeaderReady}} queues the start of a
> pending repair on the state machine apply executor, behind the entries
> already handed to it. It starts the repair once, if this OM is the ready
> leader. Ratis signals leader ready before the new leader has applied its
> backlog, so starting it inline could run a second repair next to a
> {{QuotaRepair}} that was committed but not yet applied, and correct the
> counters twice.
> * An applied {{QuotaRepair}} request clears the pending mark on every OM, so
> the followers of a normal finalization do not repeat the repair on a later
> failover.
> The leader path at apply time is unchanged. There are no new requests, proto
> fields, DB keys or configuration, so OMs with and without the patch apply the
> same log in the same way.
> Limits:
> * The mark is lost if every OM that holds it restarts before one becomes the
> ready leader. A pending marker in the meta table would close that, at the
> cost of a metadata change.
> * Followers hold the mark until they apply the leader's quota repair, so a
> leader change while that repair is still running also starts a repair on the
> new leader. The old leader's run then fails at submit, because its Ratis
> server is no longer the leader (argued from code).
> * A leader repair that fails before submit (any error, including a cluster
> without buckets) leaves the mark set on every follower, so a full repair can
> start at a failover long after the upgrade. The same holds for a follower
> that finalized from an installed snapshot that already contains the repair.
> * A repair still running on a leader that lost leadership can still apply if
> that OM becomes leader again before it finishes, so two repairs can both
> apply if leadership moves back and forth during one repair run.
> * Any applied {{QuotaRepair}} clears the mark, including one started from the
> CLI with {{--buckets}}.
> A smaller alternative is a WARN only change: extend the skip message with "If
> no quota repair was started for this upgrade (check {{ozone repair om quota
> status}} on the leader), run {{ozone repair om quota start}}." It adds no
> state and cannot start a repair, at the cost of relying on the operator.
> Covered by two tests in the existing {{TestOMUpgradeFinalization}}:
> * {{testQuotaRepairRunsAfterLeaderChangeDuringFinalization}} reproduces the
> leader change as above. It asserts that exactly one repair runs and succeeds,
> and that moving leadership to the remaining OM afterwards does not start
> another. Without the change it times out waiting for the repair.
> * {{testNewLeaderDoesNotRepeatQuotaRepairQueuedForApply}} uses the existing
> {{OzoneManagerRequestHandler}} fault injector to hold a follower's apply of
> the leader's {{QuotaRepair}}, then moves leadership to that follower. It
> asserts that no second repair starts. It fails if the pending repair is
> started inline in {{notifyLeaderReady}} (a second repair runs and applies),
> and passes with the patch.
> With the patch, these pass:
> * {{TestQuotaRepairTask}}, {{TestOMQuotaRepairRequest}},
> {{TestOMVersionManager}}, {{TestOzoneManagerStateMachine}},
> {{TestOMUpgradeFinalizeService}} and {{TestOMCompleteFinalizeUpgradeRequest}}
> (106 tests);
> * {{TestOMUpgradeFinalization}}, {{TestOMBucketLayoutUpgrade}},
> {{TestUpgradeFinalizationWithOldClients}} and {{TestOzoneRepairShell}} on a
> mini cluster (17 tests).
> Checkstyle is clean. With the patch the reproduction fails because the new
> leader has run the repair.
> Relates to HDDS-16860, a separate defect in the same action. The patches for
> this issue and HDDS-16860 change the same few lines of
> {{QuotaRepairUpgradeAction.execute}}, so whichever lands later needs a
> trivial rebase. The open pull request for HDDS-16275 (apache/ozone#11190)
> also changes {{OzoneManagerStateMachine}} and merges cleanly with this patch;
> it gates only {{applyTransaction}} while the state machine is paused for a
> checkpoint install, so the deferred repair start, which is submitted to the
> apply executor directly, is neither rejected nor waited for by it.
> Found by TLA+ model checking and code review of the OM upgrade finalization
> paths under HDDS-15926, on commit a6b7bdb937109ba2893688b41a89470bc95c88cf.
> 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]
