Siyao Meng created HDDS-16791:
---------------------------------
Summary: 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
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]
