[
https://issues.apache.org/jira/browse/HDDS-16787?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16787:
------------------------------
Attachment: TestBugMC1AbortAfterQuotaFailedPartCommit.java
> 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
> Priority: Critical
> 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]
