Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Independent Multipart Uploads #15346

Merged
merged 9 commits into from
Jul 19, 2022

Conversation

klauspost
Copy link
Contributor

Description, Motivation and Context

In distributed mode a lock was held to merge each multipart upload as it was added.

This lock was highly contested and retries are expensive (timewise) in distributed mode.

Instead each part adds its metadata information uniquely. This eliminates the per object lock required for each to merge.
The metadata is read back and merged by "CompleteMultipartUpload" without locks when constructing final object.

Worst case benchmark; 4000 parts uploaded in parallel - each part-size 10MiB, concurrency of 160:

Operation: PUT
Duration: 3m39s -> 3s
* Average: +9337.34% (+9836.4 MiB/s) throughput, +9337.34% (+983.6) obj/s
* Requests: Avg: -14.1379s (-99%), P50: -9.387000726s (-99%), P99: -1m11.439559267s (-99%), Best: -114.190865ms (-60%), Worst: -2m15.824492552s (-99%)
* Fastest: +6368.98% (+10865.9 MiB/s) throughput, +6368.98% (+1086.6) obj/s
* 50% Median: +10862.26% (+10935.8 MiB/s) throughput, +10862.25% (+1093.6) obj/s
* Slowest: +11665.94% (+8078.0 MiB/s) throughput, +11665.93% (+807.8) obj/s

(covers upload phase)

ListObjectParts will also need to merge part data on each call, which will make it slower, but this is by far less used than Multipart uploads, so this tradeoff makes sense.

Technically multipart uploads should be retainable across an upgrade, but stopping them would by far be the safest.

How to test this PR?

Plenty of tests cover this already.

Types of changes

  • Optimization (provides speedup with no functional changes)

@minio minio deleted a comment from minio-trusted Jul 19, 2022
@minio-trusted
Copy link
Contributor

Mint Automation

Test Result
mint-large-bucket.sh ✔️
mint-fs.sh ✔️
mint-gateway-s3.sh ✔️
mint-erasure.sh ✔️
mint-dist-erasure.sh ✔️
mint-compress-encrypt-dist-erasure.sh ✔️
mint-pools.sh ✔️
Deleting image on docker hub
Deleting image locally

@@ -229,7 +229,10 @@ func (client *storageRESTClient) NSScanner(ctx context.Context, cache dataUsageC
rr.CloseWithError(err)
return cache, err
}
updates <- update
select {
case <-ctx.Done():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to add a return here @klauspost

@harshavardhana harshavardhana merged commit f939d1c into minio:master Jul 19, 2022
poornas added a commit to poornas/minio that referenced this pull request Nov 26, 2022
poornas added a commit to poornas/minio that referenced this pull request Nov 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants