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