[
https://issues.apache.org/jira/browse/HDDS-16791?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16791:
------------------------------
Attachment: TestBugMC5UpdateOnlyPurgeRejected.java
> PurgeKeys requests that only carry keysToUpdate are rejected, so a partly
> reclaimable deleted key is not rewritten and its blocks are resent to SCM
> ---------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16791
> URL: https://issues.apache.org/jira/browse/HDDS-16791
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments:
> MC-5-accept-purge-keys-request-with-only-keys-to-update.patch,
> TestBugMC5UpdateOnlyPurgeRejected.java
>
>
> h3. Mechanism
> When a {{deletedTable}} entry holds one version that can be reclaimed and one
> that a snapshot still needs, {{KeyDeletingService}} sends the reclaimable
> blocks to SCM and then puts the entry into {{keysToUpdate}} of a PurgeKeys
> request, so that OM rewrites the entry with only the retained version. If
> nothing else in that store can be purged, the request has no deleted keys and
> no renamed keys. {{OMKeyPurgeRequest.validateAndUpdateCache}} rejects such a
> request with {{KEY_DELETION_ERROR}} ("No keys found to be purged or renamed
> in the request.") without looking at {{keysToUpdate}}. The entry is not
> rewritten and the next run finds the same blocks again.
> The check was added by HDDS-11411 for deleted keys and extended to renamed
> keys by HDDS-13026. No comment, commit message or test says that a request
> with only {{keysToUpdate}} should be refused, and {{OMKeyPurgeResponse}}
> applies {{keysToUpdate}} whenever another key shares the request. HDDS-11411
> is first contained in release 2.0.0 (by history, not run on older code).
> h3. Trigger
> # Create a key and take a snapshot of the bucket.
> # Overwrite the key with a commit that leaves an allocated block uncommitted.
> The {{deletedTable}} entry now holds the old version, which the snapshot
> still references, and the uncommitted blocks, which are reclaimable.
> # {{KeyDeletingService}} runs while that entry is the only reclaimable work
> of the active object store.
> Default settings, no fault. For the active object store the condition ends
> when another purgeable key shares the request. Snapshot variant: a second
> snapshot is taken after step 2 while the entry still holds both versions. The
> entry then sits in the deleted table of the second snapshot and, if that
> deleted table holds no fully reclaimable key and no reclaimable rename entry,
> the request for that snapshot is rejected in every run for as long as both
> snapshots exist.
> h3. Impact
> * The same block is sent to SCM for deletion in every run of the service
> (default interval 1m). Run: 3 of 3 runs in both variants, on the OM side with
> a recording SCM block client. By code reading SCM creates a new delete
> transaction per call and duplicates are detected by transaction ID, not by
> block, so every resend is a new SCM transaction and datanode command for a
> block that is already deleted. That part was not run against a real SCM or
> datanode.
> * The updates that follow a successful purge are skipped. In the active store
> variant the exclusive size of the snapshot stayed 0 during the rejected runs
> and became 268435456 in the first accepted run. In the snapshot variant the
> second snapshot is never marked as deep cleaned, so it is scanned again in
> every run.
> * {{keysReclaimedInInterval}} counts the same block again in every run (1, 2,
> 3 over three runs).
> * No data is lost and no client call fails.
> h3. Reproduction
> PASS, deterministic, unmodified source.
> [^TestBugMC5UpdateOnlyPurgeRejected.java] ({{hadoop-ozone/ozone-manager}})
> drives one real OM per test through {{OzoneManagerProtocol}}, with a
> recording SCM block client and no datanodes, and calls the service task
> directly three times. The first test covers the active object store,
> including the run in which an unrelated deleted key makes the request
> acceptable. The second covers the snapshot variant. Both print the requests,
> their responses, the blocks sent to SCM and the content of the entry for
> every run. A passing test means the defect is present.
> h3. Patch
> [^MC-5-accept-purge-keys-request-with-only-keys-to-update.patch], against
> a6332fb59879b80615b9233e953d6813d03ae7a8. It also applies to master at
> 3a22acfaa21.
> {{OMKeyPurgeRequest.validateAndUpdateCache}} rejects a request only when the
> deleted keys, the renamed keys and {{keysToUpdate}} are all empty.
> {{OMKeyPurgeResponse}} already applies {{keysToUpdate}}, so no other code
> change is needed. An accepted request also records the purge transaction,
> like any other PurgeKeys request. The change is not gated by a layout
> feature, like HDDS-13026 on the same condition. Not tested with OMs of
> different versions applying the same log. By code reading, an OM without the
> change still rejects such a request, so the entry (and for a snapshot the
> recorded purge transaction) can differ between OMs until the entry is purged
> or its snapshot is deleted. What remains on the older OM is a reference to
> blocks that are already deleted.
> Covered by {{testRequestWithOnlyKeysToUpdate}} in the existing
> {{TestOMKeyPurgeRequestAndResponse}}, which also checks that a request with
> nothing to purge, update or rename is still rejected. Without the change it
> fails with "expected: <OK> but was: <KEY_DELETION_ERROR>". With it
> {{TestOMKeyPurgeRequestAndResponse}},
> {{TestOMDirectoriesPurgeRequestAndResponse}},
> {{TestOMSnapshotPurgeRequestAndResponse}}, {{TestKeyDeletingService}} and
> {{TestKeyManagerImpl}} pass (98 tests) and checkstyle is clean.
> {{TestKeyPurging}} and {{TestSnapshotDeletingServiceIntegrationTest}} on a
> mini cluster (9 tests) pass in a run that had this patch and a second,
> unrelated change to {{KeyDeletingService}} applied together. With the patch
> both tests of the reproduction fail at their first assertion on the response
> status, and the output of the first run shows the request accepted, the entry
> rewritten, the exclusive size set and the second snapshot deep cleaned. In a
> variant of the reproduction without assertions, the second and third run send
> no block and no PurgeKeys request.
> Not part of the patch: the reclaim metrics are updated whether or not a purge
> request was applied. This change removes the case that inflated them in every
> run. Other failed purges are still counted and then counted again by the run
> that retries them. That is the same kind of issue as HDDS-16437.
> 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. Related but not
> duplicates: HDDS-16464 (deep clean flag of a snapshot set too early),
> HDDS-16755 (a different defect on the same kind of entry), HDDS-16437. 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]
