Siyao Meng created HDDS-16787:
---------------------------------
Summary: Part commit that fails on bucket quota leaves the part in
the OM table cache, so a later part commit, abort or complete acts on it
Key: HDDS-16787
URL: https://issues.apache.org/jira/browse/HDDS-16787
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
MC-1-publish-multipart-part-to-cache-after-quota-check.patch,
TestBugMC1AbortAfterQuotaFailedPartCommit.java,
TestBugMC2OverwrittenPartBlockLeak.java,
TestBugMC3CompleteWithQuotaFailedPart.java,
TestBugMC4AbortDivergesAcrossOMs.java
h3. Mechanism
{{S3MultipartUploadCommitPartRequest.validateAndUpdateCache}} adds the new part
to the {{multipartPartsTable}} cache (legacy layout: the updated
{{multipartInfoTable}} entry that carries the part inline) and puts a delete
marker on the part's open key in the open key table cache before it calls
{{checkBucketQuotaInBytes}}. When the check throws {{QUOTA_EXCEEDED}},
{{S3MultipartUploadCommitPartResponse.checkAndUpdateDB}} writes nothing and
nothing withdraws those cache entries. They stay visible until the double
buffer has flushed that transaction and the epoch is evicted.
{{OzoneManagerStateMachine.applyTransaction}} does not wait for the flush, so
the next request can be applied while the rejected part still looks committed.
{{S3MultipartUploadAbortRequest}}, {{S3ExpiredMultipartUploadsAbortRequest}}
and {{S3MultipartUploadCompleteRequest}} read the parts through the cache
({{OMMultipartUploadUtils.scanParts}}, or the cached {{OmMultipartKeyInfo}} for
the legacy layout) and act on what they see. A later commit of another part of
the same upload reads the same cache. In the legacy layout it starts from the
cached {{OmMultipartKeyInfo}}, which carries the rejected part inline, adds its
own part, and its response writes the whole entry to RocksDB. From then on the
rejected part is durable and no longer depends on the window.
HDDS-16161 made this request update the bucket usage on a copy and publish it
to the bucket table cache after the quota check. The part, upload and open key
cache writes stayed in front of it.
h3. Trigger
A bucket with a space quota that is almost used up. Opening a part only checks
the quota, it charges nothing, so parts uploaded in parallel can all be opened
and one of them then fails at commit with {{QUOTA_EXCEEDED}}. One more request
for the same upload then has to be applied on an OM before that OM has flushed
the failed commit:
* a commit of another part that still fits, typically a smaller last part
(legacy layout). After that, no abort or complete has to fall inside the window.
* an abort, sent by the client or by {{MultipartUploadCleanupService}} if the
upload is older than the expiry threshold, or a complete.
The window is one double buffer flush per OM. The reply to the client does not
wait for the flush, so the window exists on the leader as well, and on
followers it is independent of when the client received the error.
The cache writes precede the quota check in every release tag from ozone-1.4.0
to ozone-2.2.1 as well (read from the tagged source, not run on a release).
Those releases have only the legacy layout.
h3. Impact
Reproduced on unmodified source unless marked otherwise. The attached classes
hold the flush with {{OzoneManagerDoubleBuffer.pause}}, which only changes
timing.
* *OMs diverge.* Whether an OM still shows the rejected part when it applies
the next request depends on its own flush, not on the Ratis log. On a three OM
cluster with one follower's flush held, all OMs applied the same log and the
held follower persisted {{usedBytes=-3145728}} and two deleted blocks while the
other two persisted {{usedBytes=0}} and one. A client read 0, then -3145728
after a leader transfer with no write in between.
[^TestBugMC4AbortDivergesAcrossOMs.java]
* *Rejected part persisted (legacy layout).* With a 6 byte quota, part 1 (4
bytes) committed, part 2 (4 bytes) rejected and part 3 (2 bytes) committed
before the flush, {{multipartInfoTable}} in RocksDB lists parts 1, 2 and 3. An
abort sent after the cache entry had been evicted left {{usedBytes}} at -4 in
the cache and in RocksDB. Whether an OM persists the rejected part depends on
its own flush in the same way as above (argued from the code, the three OM run
covers the abort only). [^TestBugMC1AbortAfterQuotaFailedPartCommit.java]
* *Negative bucket usage.* An abort inside the window releases the bytes of the
rejected part although they were never charged. With a 6 byte quota and two 4
byte parts, {{usedBytes}} ends at -4 in the cache and in RocksDB, for a client
abort and for the expired upload cleanup. The bucket then accepted 8 bytes of
committed parts. A complete that lists only the accepted part releases the
rejected part as an unused part in the same way: in a request level run (both
layouts, not attached) {{usedBytes}} ended at 0 although the completed key
holds the 200 byte part. [^TestBugMC1AbortAfterQuotaFailedPartCommit.java]
* *Leaked block.* If the rejected commit was a re-upload of an already
committed part number, the abort sees only the rejected part. The committed
part's block is in no OM table and not in {{deletedTable}}, and
{{KeyDeletingService}} never asks SCM to delete it. Shown for the split parts
table and for the legacy layout. A later accepted commit of the same part
number has the same effect without an abort: in a request level run (both
layouts, not attached) it queued the rejected part's blocks for deletion, not
the committed part's block, and {{usedBytes}} ended at 0 with a 200 byte part
committed. [^TestBugMC2OverwrittenPartBlockLeak.java]
* *Completed key whose block is sent for deletion.* A complete that lists the
rejected part returns OK. The key references the part's block while the part's
open key row is still in RocksDB. A retry of the part commit
({{NO_SUCH_MULTIPART_UPLOAD_ERROR}} branch of
{{S3MultipartUploadCommitPartResponse}}) or {{OpenKeyCleanupService}} then
moves that block to {{deletedTable}} and {{KeyDeletingService}} asks SCM to
delete it while the key is still in {{keyTable}}. This needs a client that
completes with a part whose commit did not succeed. The test stops at the
delete request to a testing SCM client. The datanode side delete and a failing
read were not exercised. [^TestBugMC3CompleteWithQuotaFailedPart.java]
The wrong values are written to RocksDB and survive the eviction of the cache
entries. The patch does not repair buckets and uploads that are already
affected.
h3. Reproduction
The four attached classes are self contained and pass on unmodified source when
the bug is present (target path and Maven command are in each class comment).
Three run one real OM through {{OmTestManagers}} in
{{hadoop-ozone/ozone-manager}}. They send every request through
{{OzoneManagerProtocol}} or through one manual pass of the cleanup services,
with a 4 byte block size and short expiry thresholds. The first runs a
pre-finalized OM so that uploads use the legacy layout, the second runs both
layouts. The fourth runs a three OM mini cluster in
{{hadoop-ozone/integration-test}} through the public client. No state is
injected. The only timing aid is the existing
{{OzoneManagerDoubleBuffer.pause}} / {{unpause}} test hook, so these runs show
that the window exists, not how often it is hit. Runs without the hook
(concurrent clients) hit the window at low rates in the original investigation
and were not repeated for this report.
h3. Patch
[^MC-1-publish-multipart-part-to-cache-after-quota-check.patch], against
d067ba38e7aea917db7efd290b4af53987ec985a. It also applies to master at
a6332fb5987.
The three cache writes ({{multipartPartsTable}}, {{multipartInfoTable}}, open
key delete marker) move from before {{checkBucketQuotaInBytes}} to after it,
next to the bucket table cache write that HDDS-16161 already placed there.
Nothing between the old and the new position reads those tables, and for the
legacy layout {{addPartKeyInfo}} works on the copy returned by {{Table.get}},
so no other change is needed. A commit that fails now leaves no cache state,
for the default and the FSO request (the FSO class inherits the method), and
the success path publishes the same entries as before. Withdrawing the entries
on failure would also work but is more code for the same result.
One visible change besides the symptoms above: the open key of a rejected
commit is no longer hidden while the delete marker is cached, so a retry of
that commit with the same open key session is evaluated again on every OM.
Before, by code reading, it failed with {{KEY_NOT_FOUND}} until the marker was
evicted.
Covered by five cases in the existing
{{TestS3MultipartUploadCommitPartRequest}}, inherited by
{{TestS3MultipartUploadCommitPartRequestWithFSO}}:
{{testQuotaExceededCommitIsNotVisibleToLaterRequests}},
{{testAbortAfterQuotaExceededCommitReleasesOnlyCommittedParts}},
{{testAbortAfterQuotaExceededOverwriteDeletesCommittedPart}} and
{{testCompleteRejectsPartWhoseCommitExceededQuota}} run for both part layouts,
{{testCommitAfterQuotaExceededCommitPersistsOnlyAcceptedPart}} for the legacy
layout. Without the change all 18 runs fail (for example "expected: 0L but was:
-200L", "expected: INVALID_PART but was: OK" and persisted parts 1 and 2 where
only part 2 is expected). With it the multipart request and response unit
suites and the two cleanup service suites pass (163 tests), as do the multipart
cases of {{TestOzoneRpcClient}} and {{TestOzoneManagerHAWithStoppedNodes}} (56
tests), checkstyle is clean, and the four attached reproductions no longer show
the bug.
Found by TLA+ model checking and code review of the OM expired multipart upload
cleanup paths under HDDS-15926, on commit
d067ba38e7aea917db7efd290b4af53987ec985a. Checked against HDDS issues and
apache/ozone pull requests for duplicates before filing. Related but not
duplicates: HDDS-16763 (a failed FSO CompleteMultipartUpload leaves parent
directories in the cache), HDDS-16765 (a rejected overwrite commit leaves a
marker on another writer's cached open key entry), HDDS-16766 (an FSO directory
delete depends on flush and cache eviction timing, so OMs apply it
differently), HDDS-16785 (a part commit rejected with
NO_SUCH_MULTIPART_UPLOAD_ERROR leaves its open key behind, a different exit of
the same request) and HDDS-16161 (resolved, bucket usage only). 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]
