[
https://issues.apache.org/jira/browse/HDDS-16794?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16794:
------------------------------
Attachment: MC-2-do-not-submit-key-purge-after-term-change-during-run.patch
> KeyDeletingService run that outlives a lost and regained OM leadership purges
> keys again and reduces bucket usage twice
> -----------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16794
> URL: https://issues.apache.org/jira/browse/HDDS-16794
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments:
> MC-2-do-not-submit-key-purge-after-term-change-during-run.patch,
> TestBugMC2StaleRunDoublePurge.java
>
>
> h3. Mechanism
> {{KeyDeletingService.KeyDeletingTask.call}} checks {{shouldRun}} (leader
> ready) and the flush of the previous purge once, at the start of a run. It
> then scans {{deletedTable}}, calls SCM {{deleteKeyBlocks}} and submits
> {{PurgeKeys}}. Nothing ties the request to the leadership under which the
> scan was made, and Ratis accepts it whenever this OM is the leader at the
> time of the submit. {{OMKeyPurgeRequest.validateAndUpdateCache}} subtracts
> the sizes in {{bucketPurgeKeysSize}} from {{snapshotUsedBytes}} and
> {{snapshotUsedNamespace}} whether or not the keys still exist, and deleting
> the absent {{deletedTable}} rows is a no-op.
> If the OM loses the leadership and gets it back while a run is between its
> checks and its submit, and the leader in between has purged the same keys,
> the usage is subtracted a second time on every OM.
> HDDS-13760 added the flush check "so that duplicate purge requests are not
> submitted". It covers a second scan on the same OM before the flush, not a
> run that has already passed the check.
> h3. Trigger
> # OM1 is leader and a key is deleted. A KeyDeletingService run on OM1 passes
> its checks and scans the key.
> # The run stalls before it submits the purge, for example in the SCM call.
> OM1 loses the leadership, OM2 becomes leader and its own run purges the key.
> # OM1 becomes leader again.
> # The stalled run continues and submits its purge of the key, which is
> applied in the new term.
> The stall has to cover both leadership changes and the run of the other
> leader, which comes on that leader's own schedule, so with the default
> service interval of 60s it has to last up to about a minute. I expect this to
> be rare (argued, not measured).
> By history the defect is present since 2.1.0: {{ozone-2.1.0}} is the first
> release tag in which the purge reduces the bucket usage (HDDS-13764). Not run
> on older code.
> h3. Impact
> * The pending deletion bytes and namespace of the bucket become too low,
> negative for a bucket with nothing else pending.
> {{OmBucketInfo.getTotalBucketSpace}} and {{getTotalBucketNamespace}} include
> them without a lower bound, so clients see wrong (in the reproduction
> negative) used bytes and namespace, and the quota checks in {{OMKeyRequest}}
> admit more than the quota. In the reproduction a bucket with a namespace
> quota of 1 accepted two keys. The space quota check reads the same total
> (code reading, not run).
> * The stale request repeats the whole batch of the stalled run, not a single
> key.
> * The wrong values are written to the bucket table on all OMs and nothing
> corrects them automatically. The quota repair recomputes these counters when
> an operator starts it (code reading, not run).
> h3. Reproduction
> PASS, unmodified source. [^TestBugMC2StaleRunDoublePurge.java]
> ({{hadoop-ozone/integration-test}}) starts a mini cluster with three OMs and
> one datanode. The deleting services get a long interval and are run by hand
> with {{runPeriodicalTaskNow}}, and the leadership is moved with
> {{OzoneManager.transferLeadership}}. The run on the first leader is made by a
> second {{KeyDeletingService}} on that OM whose SCM client delays the reply of
> {{deleteKeyBlocks}}, so the run is held in the SCM call. After the sequence
> above the client reads {{usedBytes = -1024}} and {{usedNamespace = -1}} for
> the empty bucket, the bucket row holds -1024 pending deletion bytes in
> RocksDB on all three OMs, and with a namespace quota of 1 the bucket accepts
> two keys and rejects the third with {{QUOTA_EXCEEDED}}. A passing test means
> the defect is present.
> h3. Patch
> [^MC-2-do-not-submit-key-purge-after-term-change-during-run.patch], against
> a6332fb59879b80615b9233e953d6813d03ae7a8. It also applies to master at
> 3a22acfaa21.
> A run of {{KeyDeletingService}} records the Raft term right after the leader
> check, before the flush check and the scan, and compares it with the current
> term immediately before each submit of the run ({{PurgeKeys}} and the
> snapshot property updates, for the active object store and for the snapshot
> tasks). If the term is different the request is not submitted and the run
> ends like a run whose purge failed. The term is read from the same Ratis
> division info that {{shouldRun}} uses for leader ready. There is no proto
> change and nothing changes at apply time. The keys stay in {{deletedTable}}
> and the next run, which starts in the new term, purges them once (asserted in
> the new test).
> This narrows the window, it does not close it. The comparison is not atomic
> with the append to the Ratis log, so a run that stalls between the comparison
> and the append for as long as described under Trigger still purges twice. An
> earlier form of the reproduction, which holds the run inside the submit call
> after the comparison, still passes with the patch. The stricter alternative
> is a new optional field in {{PurgeKeysRequest}} with the term of the run, and
> a check in {{OMKeyPurgeRequest}} that rejects the request when the term of
> its log entry is different, which is exact and decided identically on every
> OM. It is not proposed here because it adds a field without a layout feature
> gate to an existing request, and in a ring where OMs of two versions apply
> the same log an OM without the check applies a stale request that an OM with
> the check rejects, so the OMs disagree.
> Subtracting only for keys that are still in {{deletedTable}} was also
> considered and not taken. It cannot be decided consistently at apply time,
> because {{deletedTable}} has no table cache entries and the DB lags the apply
> by the double buffer, so OMs could decide differently.
> Covered by {{testNoPurgeAfterTermChangeDuringRun}} in the existing
> {{TestKeyDeletingService}}: the term changes while the run is in the SCM
> call, the run does not purge, and the next run purges the key once. It fails
> when the comparison in {{KeyDeletingService.submitRequest}} is removed,
> because the first run then purges the key. With the patch 173 unit tests of
> {{hadoop-ozone/ozone-manager}} (deleting service, purge request, open key
> cleanup, key manager, snapshot manager and quota repair suites) and 22 mini
> cluster tests (key purging, OM HA with stopped nodes and the snapshot
> deleting service suite) pass, checkstyle is clean. With the patch the
> reproduction fails at its first assertion of the buggy outcome, because the
> stale run is not submitted and the usage stays 0.
> Not covered by the patch: {{DirectoryDeletingService}} submits
> {{PurgeDirectories}} with the same single check at the start of a run, and
> {{OMDirectoriesPurgeRequestWithFSO}} also reduces the bucket usage without a
> presence check (code reading, not run). It submits from its worker threads,
> so the term would have to be handed to them, and it is left out here. A term
> change between two requests of one run now stops the later requests, which is
> one more way into HDDS-16790 (a key purged by one request with its size in
> the next) until that is fixed.
> The flush check that misses a purge which is not applied yet on a new leader
> is a different defect and is filed separately. Neither fix covers the other:
> that one covers a run that starts after a leader change, this one a run that
> started before it.
> Found by TLA+ model checking and code review of the OM key deletion service
> and key purge paths under HDDS-15926, on commit
> a6332fb59879b80615b9233e953d6813d03ae7a8. 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]
