[
https://issues.apache.org/jira/browse/HDDS-16796?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16796:
------------------------------
Attachment: CR-3-set-committed-deleted-flag-authoritatively.patch
> deletedTable entries inherit the committed flag from their key info, so the
> purge can release pending deletion usage that was never added
> -----------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16796
> URL: https://issues.apache.org/jira/browse/HDDS-16796
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments: CR-3-set-committed-deleted-flag-authoritatively.patch
>
>
> h3. Mechanism
> A {{deletedTable}} entry carries a flag that records whether the replicated
> size of that key info and one namespace unit were added to the bucket's
> {{snapshotUsedBytes}} and {{snapshotUsedNamespace}} when the key left the key
> table ({{OmKeyInfo.isDeletedKeyCommitted}}, set with
> {{OmKeyInfo.withCommittedKeyDeletedFlag}}). When the entry is purged,
> {{KeyManagerImpl.getPendingDeletionKeys}}, {{KeyDeletingService}} and
> {{OMKeyPurgeRequest.updateBucketSize}} subtract the size and the namespace
> unit only for key infos that carry the flag (HDDS-13756).
> The flag is part of the {{OmKeyInfo}} that each producer moves into
> {{deletedTable}}, and the producers do not all set it from their own intent.
> The shared mover {{OmUtils.prepareKeyForDelete}} keeps whatever the key info
> it is handed carries, and two sites that build the entry without that mover,
> {{S3MultipartUploadCompleteResponse}} for the key and the unused parts of a
> completed upload and {{OMKeyRequest.wrapUncommittedBlocksAsPseudoKey}} for
> the pseudo key of uncommitted blocks, do not clear it either. Only
> {{AbstractOMKeyDeleteResponse}} decides the flag for the entry it writes.
> A producer that does not add a key's size to the pending deletion counters
> can therefore write an entry that the purge treats as charged, and the purge
> then subtracts a size and a namespace unit that were never added. The
> counters have no lower bound, so they go negative and
> {{OmBucketInfo.getTotalBucketSpace}} and {{getTotalBucketNamespace}}, which
> the quota checks in {{OMKeyRequest}} compare against, fall below the bucket's
> real usage. Nothing recomputes them automatically.
> h3. Trigger
> # A key info that already carries the committed flag value is moved into
> {{deletedTable}} by a producer that does not add its size to the bucket's
> pending deletion counters. Run through the shared mover and through
> {{S3MultipartUploadCompleteResponse}} (an unused part of a completed upload).
> # {{KeyDeletingService}} purges the entry.
> The other producers that route through the shared mover (multipart abort, an
> overwritten or aborted part, {{FSORepairTool}}) and the uncommitted blocks
> pseudo key reach the same state, from code reading, not run.
> By history, not run on older code: the flag and the pending deletion counters
> arrived together with HDDS-13756, so the inheritance is present from the
> 2.1.0 release tag onwards. 2.0.0 has neither.
> h3. Impact
> * After the purge the bucket's {{snapshotUsedBytes}} and
> {{snapshotUsedNamespace}} are lower by the size and the namespace unit of an
> entry that never added them, and go negative for a bucket with nothing else
> pending. In the regression run of the shared mover the entry comes back with
> the flag cleared, and the purge therefore leaves the counters alone.
> * The charged usage the quota checks read is then below the bucket's real
> usage, so the bucket admits more than its space and namespace quota allow.
> * No key and no data is affected, and the counters stay wrong until an
> operator starts a quota repair ({{ozone repair om quota start}}), which
> recomputes them (code reading, not run).
> h3. Reproduction
> The two regression tests in the attached patch cover it and fail on
> unmodified source:
> * {{TestOmUtils.prepareKeyForDeleteSetsCommittedFlagAuthoritatively}}
> ({{hadoop-ozone/common}}): the shared mover must decide the flag of the entry
> it builds, for the three argument form and for both values of the new
> argument. Without the change the three argument form and the {{false}} form
> both return a flagged entry.
> *
> {{TestS3MultipartUploadCompleteResponse.testCompleteDoesNotInheritClientCommittedFlag}}
> ({{hadoop-ozone/ozone-manager}}, also in the inherited {{...WithFSO}}): an
> unused part of a completed upload must not produce a flagged entry. Without
> the change it fails with {{expected: <false> but was: <true>}}.
> h3. Patch
> [^CR-3-set-committed-deleted-flag-authoritatively.patch], against
> a6332fb59879b80615b9233e953d6813d03ae7a8. It also applies to master at
> fa7b58e3901.
> {{OmUtils.prepareKeyForDelete}} takes the flag as a new argument and sets it
> on the entry it builds instead of keeping the value of the key info it is
> handed. The three argument form stays and passes {{false}}, so a producer
> that does not charge the counters needs no change. The producers that do
> charge them pass their intent: {{AbstractOMKeyDeleteResponse}} passes the
> value it already computed (both of its overloads),
> {{OMDirectoriesPurgeResponseWithFSO}} passes {{true}} for the sub files of a
> purged directory instead of flagging the key info beforehand. The two sites
> that build the entry without the mover clear the flag explicitly:
> {{S3MultipartUploadCompleteResponse}} for the key and the unused parts it
> moves, and {{OMKeyRequest.wrapUncommittedBlocksAsPseudoKey}} for the pseudo
> key. {{OMKeyCommitRequest}}, which flags the version it overwrites after the
> move, is unchanged.
> So every {{deletedTable}} entry is flagged by the code that writes it, and an
> entry whose source key info carried a value is written with the flag the
> producer intends. Nothing changes for an entry the OM itself flagged, there
> is no wire or protocol change, and entries already in {{deletedTable}} are
> read as before.
> With the patch the key delete and purge response suites, the directories
> purge and key purge request suites, the quota repair task suite, the key and
> part commit response suites, the multipart abort, commit part and complete
> response suites and {{TestKeyDeletingService}} pass, and checkstyle on
> {{hadoop-ozone/common}} and {{hadoop-ozone/ozone-manager}} is clean.
> One coordination note with HDDS-16793, which is a different defect in the
> same accounting and whose patch is also open for review: that patch pre-sets
> the flag on the shared test fixture {{OMRequestTestUtils.deleteKey}}, which
> this change then overrides, so {{TestQuotaRepairTask}} has two failures with
> both applied. Whichever lands second carries the fixture line over to the
> four argument form of the mover. Verified by running both.
> Found by code review of the OM key deletion service and key purge paths, as
> part of the TLA+ verification effort under HDDS-15926, on commit
> a6332fb59879b80615b9233e953d6813d03ae7a8. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. Related but not
> duplicates: HDDS-13756 (introduced the flag and the pending deletion
> counters), HDDS-16793 (quota repair counts entries the purge never releases),
> HDDS-16176 (the size formula in the same release path). 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]
