[
https://issues.apache.org/jira/browse/HDDS-16792?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16792:
------------------------------
Attachment: MC-4-key-deleted-table-entries-by-delete-transaction.patch
> Deleting a key that a stale overwrite commit recreated replaces its pending
> deletedTable entry, so the blocks of one version are never deleted
> ----------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16792
> URL: https://issues.apache.org/jira/browse/HDDS-16792
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments:
> MC-4-key-deleted-table-entries-by-delete-transaction.patch,
> TestBugMC4DeleteAfterStaleCommit.java
>
>
> h3. Mechanism
> {{AbstractOMKeyDeleteResponse.addDeletionToBatch}} writes a deleted key to
> {{deletedTable}} under {{<key path>/<object ID>}} with a plain put
> (HDDS-8462). The object ID does not identify one deleted version of a key.
> {{OMKeyRequest.prepareFileInfo}} builds the open key of an overwrite from the
> existing key and keeps its object ID, and {{OMKeyCommitRequest}} commits that
> open key also when the key was deleted after the open. A key that is deleted,
> recreated by such a commit and deleted again therefore gets the same
> {{deletedTable}} key both times.
> HDDS-10296 fixed this collision for the entry written by an overwriting
> commit, which is keyed with a pseudo object ID derived from the transaction
> index ({{OmUtils.getObjectIdFromTxId}}) since then. The entries written by
> DeleteKey and DeleteKeys still use the real object ID. By history, not run on
> older code: this naming with a plain put is in the 1.4.0 release tag and
> every later one, and the bucket counters named under Impact exist since 2.1.0
> (HDDS-13756).
> The collision loses one version in two ways, both run:
> # The second delete is applied while the entry of the first one is still in
> {{deletedTable}}. The put replaces that entry, and the blocks of the first
> version are never sent to SCM.
> # The second delete is applied after a {{KeyDeletingService}} run has read
> the first entry and before its {{PurgeKeys}} request is applied. The purge
> removes entries by key, so it removes the second entry, and the blocks of the
> second version are never sent to SCM.
> h3. Trigger
> Bucket versioning off (the default), all bucket layouts (run for
> {{OBJECT_STORE}} and {{FILE_SYSTEM_OPTIMIZED}}, {{LEGACY}} shares the
> {{OBJECT_STORE}} code and was not run), client calls only:
> # A key exists. Client 1 opens it for overwrite.
> # Client 2 deletes the key.
> # Client 1 commits. The key exists again and has the object ID of the deleted
> key.
> # The key is deleted again before {{KeyDeletingService}} has purged the entry
> of step 2 ({{ozone.block.deleting.service.interval}}, default 60s).
> No fault is needed. The entry of step 2 stays longer than one interval while
> a snapshot still references the deleted key (argued from
> {{ReclaimableKeyFilter}}, not run).
> h3. Impact
> * The blocks of one deleted version are never sent to SCM for deletion, and
> no OM table references them afterwards. I ran the OM side only, with a
> recording SCM block client and no datanodes.
> * The size of that version and one namespace unit stay in
> {{snapshotUsedBytes}} and {{snapshotUsedNamespace}} of the bucket after
> {{deletedTable}} is empty. Both are part of the usage that the bucket quota
> check compares ({{OmBucketInfo.getTotalBucketSpace}} and
> {{getTotalBucketNamespace}}), so the bucket is charged for them until quota
> repair is run (argued, not run). Quota repair does not bring the blocks back
> into {{deletedTable}} (argued, not run).
> * No existing key is affected. An entry holds deleted versions only, so the
> collision can lose an entry but cannot send blocks of the current version of
> a key for deletion (argued from the code, the reproduction checks only the
> blocks named above).
> h3. Reproduction
> PASS, deterministic, unmodified source.
> [^TestBugMC4DeleteAfterStaleCommit.java] ({{hadoop-ozone/ozone-manager}})
> drives one real OM with its {{KeyDeletingService}} through
> {{OzoneManagerProtocol}} ({{openKey}}, {{commitKey}}, {{deleteKey}}), with a
> recording SCM block client and no datanodes. The service interval is 100 ms
> and the service is held with its own suspend and resume hook while the client
> calls run. {{secondDeleteReplacesPendingEntry}} ({{OBJECT_STORE}} and
> {{FILE_SYSTEM_OPTIMIZED}}) shows one entry that holds only the second block
> after the second delete, the first block never sent to SCM, and
> {{snapshotUsedBytes=805306368}} and {{snapshotUsedNamespace=1}} left on the
> bucket with an empty {{deletedTable}}.
> {{purgeOfFirstEntryRemovesSecondEntry}} runs the commit and the second delete
> while SCM answers the delete call of the service run, and shows the second
> block never sent to SCM with the same counters left. A passing test means the
> defect is present.
> h3. Patch
> [^MC-4-key-deleted-table-entries-by-delete-transaction.patch], against
> a6332fb59879b80615b9233e953d6813d03ae7a8. It also applies to master at
> 3a22acfaa21.
> DeleteKey and DeleteKeys key their {{deletedTable}} entry with the pseudo
> object ID of the delete transaction, the scheme HDDS-10296 uses on the commit
> side. Every delete then has its own entry, so a later delete cannot replace
> it and a purge can only remove the entry it has read. For this,
> {{OMKeysDeleteRequest}} again hands the key info that carries the update ID
> of the delete to the response for {{OBJECT_STORE}} and {{LEGACY}} buckets
> (that copy has been dropped since HDDS-13937, the FSO variant keeps it).
> Entries written before the change keep their keys and are processed as
> before. By code reading, nothing derives an object ID from a {{deletedTable}}
> key or looks an entry up by the object ID of its key info (key deleting
> service, purge, snapshot create, snapshot key move, reclaim filter, quota
> repair, Recon, FSO repair tool), and entries with a pseudo object ID already
> exist since HDDS-10296.
> As on the commit side (HDDS-10296, which was not gated by a layout feature),
> the entry key is computed when the request is applied, so an OM with the
> change and an OM without it name the entry of the same delete differently.
> That needs one delete to be applied by both versions: a rolling restart, or
> an HA upgrade without {{ozone admin om prepare}}, where each OM replays the
> part of the log it had not flushed. The purge sent by the leader then does
> not find the entry on the other OM. It stays there until that OM is leader,
> which sends the blocks to SCM again and releases the pending counters of the
> bucket a second time. With the documented upgrade (prepare, then restart with
> the new version) no delete is applied by both versions. Argued from the code,
> not run. If reviewers prefer, the new name can be gated by a layout feature.
> Covered by {{testDeleteOfKeyRecreatedByStaleCommitKeepsPendingEntries}} in
> the existing {{TestKeyDeletingService}} ({{OBJECT_STORE}} and
> {{FILE_SYSTEM_OPTIMIZED}}, DeleteKey and DeleteKeys): three deletes of one
> key name with a stale commit in between must leave three pending entries, and
> the pending counters of the bucket must return to zero after the purge.
> Without the change all four cases find two entries.
> {{testAOSKeyDeletingWithSnapshotCreateParallelExecution}} looked its entries
> up by object ID and now reads the entry key from the table. With the change
> {{TestKeyDeletingService}} and the key delete request and response suites (93
> tests), a wider run of the OM delete, purge, commit, snapshot and quota
> repair unit suites (852 tests, two classes of them on a rerun after a port
> conflict) and, on a mini cluster,
> {{TestSnapshotDeletingServiceIntegrationTest}},
> {{TestDirectoryDeletingServiceWithFSO}}, {{TestKeyPurging}},
> {{TestSnapshotBackgroundServices}}, {{TestHSync}} and the delete and
> overwrite tests of {{TestOzoneRpcClient}} pass, checkstyle is clean. With the
> patch all three tests of the reproduction fail: the first two find two
> entries where they expect one, the third finds the second block among the
> blocks sent to SCM.
> 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-10296 (the same collision for the entry written by an
> overwriting commit), HDDS-8462 (introduced the entry naming), HDDS-16773 (a
> different defect in DeleteKeys, its fix will touch the loop in
> {{OMKeysDeleteRequest.markKeysAsDeletedInCache}} that this patch changes, so
> the later of the two needs a rebase). 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]
