[ 
https://issues.apache.org/jira/browse/HDDS-16786?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-16786:
------------------------------
    Attachment: TestBugCR3ExpiredPartlessMpuBatch.java

> MultipartUploadCleanupService can build an abort request larger than the 
> Ratis log entry limit, which makes the OM leader step down
> -----------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-16786
>                 URL: https://issues.apache.org/jira/browse/HDDS-16786
>             Project: Apache Ozone
>          Issue Type: Bug
>            Reporter: Siyao Meng
>            Priority: Major
>         Attachments: 
> CR-3-split-expired-mpu-abort-request-by-ratis-byte-limit.patch, 
> TestBugCR3ExpiredPartlessMpuBatch.java
>
>
> h3. Mechanism
> {{MultipartUploadCleanupService}} puts everything that one scan of 
> {{multipartInfoTable}} selects into a single 
> {{MultipartUploadsExpiredAbortRequest}} and does not bound its size. The scan 
> ({{OmMetadataManagerImpl.getExpiredMultipartUploads}}) is bounded only by the 
> summed part count of the selected uploads 
> ({{ozone.om.open.mpu.parts.cleanup.limit.per.task}}, default 1000). An 
> expired upload without parts adds 0, so any number of them is selected by one 
> scan.
> When the request is larger than 
> {{ozone.om.ratis.log.appender.queue.byte-limit}} (default 32MB), Ratis 
> ({{RaftLogBase.appendImpl}}) refuses the log entry with a 
> {{StateMachineException}} and makes the leader step down. Nothing is aborted, 
> and the next run selects the same uploads and builds the same request.
> h3. Trigger
> One scan selects more than 32MB of upload names. Default configuration, no 
> fault injection. This needs many expired uploads that were initiated and 
> never received a part, which any client that may initiate multipart uploads 
> can create.
> Each upload adds its {{multipartInfoTable}} key 
> ({{/volume/bucket/key/uploadId}}, the upload ID is 55 characters) plus 4 to 6 
> bytes to the request. With 1024 character key names about 31,000 uploads 
> exceed 32MB (run). With key names of 40 to 60 characters about 250,000 are 
> needed (calculated from the entry size, not run).
> A normal run aborts every expired upload without parts that it reaches, so 
> the backlog has to build up between two runs or out of reach of the scan. Two 
> ways, both argued and not run: that many uploads expire within one service 
> interval, or they sort after a long series of expired uploads with parts. 
> Those use up the 1000 part budget of every run, so the scan does not get to 
> the uploads without parts for many runs and then reaches all of them at once.
> h3. Impact
> * Once the scan reaches the backlog, every cleanup run on the leader fails 
> and causes an OM leader step-down, at least once per service interval (24h by 
> default). In the single OM test the OM had no leader for about 5 seconds each 
> time. In HA every OM has its own timer and a new leader builds the same 
> request (HA was not exercised).
> * Once the scan reaches the backlog, no expired upload is aborted. Uploads 
> with parts that are selected into the same request are affected too (this 
> follows from the single request, it was not run separately).
> * The service does not report the failure. It ignores the failed response and 
> still counts the uploads as submitted. The only traces are the Ratis step 
> down message ({{StepDownReason:STATE_MACHINE_EXCEPTION}}) and an ERROR 
> "Unknown error occurs" from {{OzoneManagerRatisServer}} with 
> {{RaftLogIOException: Log entry size 34038083 exceeds the max buffer limit of 
> 33554432}}.
> HDDS-8977 fixed the same failure signature for {{DirectoryDeletingService}} 
> by bounding the request size.
> h3. Reproduction
> PASS on unmodified source, deterministic, about 45 seconds. 
> [^TestBugCR3ExpiredPartlessMpuBatch.java] ({{hadoop-ozone/ozone-manager}}) 
> starts one real OM (single node Ratis, real 
> {{MultipartUploadCleanupService}}) with the default parts limit and the 
> default Ratis byte limit, initiates 31,000 uploads with 1024 character key 
> names through {{initiateMultipartUpload}} and uploads no part. Only the 
> expiry threshold is shortened, and the service task is called directly 
> instead of waiting for the 24h interval. Output:
> {noformat}
> one scan with maxParts=1000 selects 31000 uploads, abort request = 34038018 
> bytes, Ratis log entry limit = 33554432 bytes
> run 1: Ratis error = 'Log entry size 34038083 exceeds the max buffer limit of 
> 33554432', Raft term 1 -> 2, expired uploads remaining = 31000 of 31000
> run 2: Ratis error = 'Log entry size 34038083 exceeds the max buffer limit of 
> 33554432', Raft term 2 -> 3, expired uploads remaining = 31000 of 31000
> {noformat}
> Not exercised: an OM HA ring with three OMs, and a backlog with key names of 
> ordinary length.
> h3. Suggested fix
> The patch below splits the request by serialized size and changes nothing 
> else. Open questions it leaves:
> * Should an upload without parts count towards 
> {{ozone.om.open.mpu.parts.cleanup.limit.per.task}}? Today it costs nothing, 
> so the number of uploads selected per run is unbounded, the scan result is 
> held in memory, and one request can still carry up to about 30MB of upload 
> names in a single transaction. Counting it would bound all of that, but it 
> changes the meaning of a documented limit and would slow the cleanup of such 
> uploads to 1000 per run at the default, behind which expired uploads with 
> parts would wait.
> * {{KeyLifecycleService.PartCountLimitedList}} (the 
> AbortIncompleteMultipartUpload lifecycle action) uses the same part 
> accounting for the abort request it builds per bucket: an upload without 
> parts adds 0, so the list never becomes full for such uploads, and that 
> request has no size bound either. This is from code reading only, it was not 
> run. That service sends the list in a loop within one run, so counting such 
> an upload as one would not cost throughput there.
> * The service still ignores a failed response to its request. Only a 
> {{ServiceException}} is logged.
> h3. Patch
> [^CR-3-split-expired-mpu-abort-request-by-ratis-byte-limit.patch], against 
> d067ba38e7aea917db7efd290b4af53987ec985a. It also applies to master at 
> a6332fb5987.
> {{MultipartUploadCleanupService}} now splits the scan result into as many 
> {{MultipartUploadsExpiredAbortRequest}}s as needed to keep each within 90% of 
> {{ozone.om.ratis.log.appender.queue.byte-limit}}, the bound that 
> {{KeyDeletingService}}, {{DirectoryDeletingService}}, 
> {{SnapshotDeletingService}} and {{RevokedSTSTokenCleanupService}} already 
> apply to their requests. A scan result that fits is still sent as one request 
> with the same content. The scan, the parts limit and the request format are 
> unchanged and there is no new configuration. An upload whose entry is larger 
> than the bound on its own is sent in a request of its own. Each request gets 
> its own Ratis call ID. The service used its run counter for that, with which 
> a second request in the same run would be answered from the Ratis retry cache 
> without being applied.
> Covered by 
> {{TestMultipartUploadCleanupService#abortsExpiredUploadsWithinRatisByteLimit}}:
>  six expired uploads in two buckets and one cleanup run by a service instance 
> configured with the default byte limit, with a limit that has room for two 
> uploads and with a limit below a single upload. It asserts the number of 
> applied abort requests (1, 4 and 6) and that no expired upload is left in the 
> table. Without the change the two cases with a lowered limit fail with 
> "expected: 4L but was: 1L" and "expected: 6L but was: 1L". With the change 
> the cleanup service, expired abort request and response, metadata manager, 
> open key cleanup and key manager unit suites pass (78 tests) and checkstyle 
> is clean. The attached reproduction then prints "run 1: Ratis error = '', 
> Raft term 1 -> 1, expired uploads remaining = 0 of 31000" and fails its 
> assertion on the Ratis error: one run aborts all 31,000 uploads without a 
> leader change.
> Found by code review of the OM expired multipart upload cleanup paths, as 
> part of the TLA+ verification effort under HDDS-15926, on commit 
> d067ba38e7aea917db7efd290b4af53987ec985a. Checked against HDDS issues and 
> apache/ozone pull requests for duplicates before filing. 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]

Reply via email to