awslabs / aws-c-s3

C99 library implementation for communicating with the S3 service, designed for maximizing throughput on high bandwidth EC2 instances.
Apache License 2.0
93 stars 37 forks source link

Add async write() function - fixes "stalled" uploads deadlocking S3 Client #418

Closed graebm closed 4 months ago

graebm commented 5 months ago

Issue: The S3 client can deadlock if too many uploads are "stalled" (not providing data when the S3 client asks for it).

Mountpoint (which wraps aws-c-s3 with a filesystem-like API) had a user that opened 100+ files at once. The user wrote data to some of the later files they opened, and waited for those writes to complete. But aws-c-s3 was waiting on data from the first few files. Both sides were waiting on each other. It was a deadlock.

Description of changes:

Design: The memory-ownership for this API evolved over time. Here were the stages: 1) At first, the user was required to keep memory valid until the async-write completed. (as of commit 5614546)

Some quick benchmarking showed similar performance from all 3 approaches (tested 30GiB upload, and simultaneous upload of 100 5GiB objects, calling write in 256KiB chunks), so 🤷‍♀️. Going with approach #3 since it doesn't risk deadlock, and doesn't risk enormous memory usage.

TODO:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

codecov-commenter commented 5 months ago

Codecov Report

Attention: Patch coverage is 88.76404% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 89.52%. Comparing base (0ce756e) to head (04457df).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/awslabs/aws-c-s3/pull/418/graphs/tree.svg?width=650&height=150&src=pr&token=J4KP54FVLF&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs)](https://app.codecov.io/gh/awslabs/aws-c-s3/pull/418?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs) ```diff @@ Coverage Diff @@ ## main #418 +/- ## ========================================== + Coverage 89.40% 89.52% +0.12% ========================================== Files 20 20 Lines 5872 5949 +77 ========================================== + Hits 5250 5326 +76 - Misses 622 623 +1 ``` | [Files](https://app.codecov.io/gh/awslabs/aws-c-s3/pull/418?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs) | Coverage Δ | | |---|---|---| | [source/s3.c](https://app.codecov.io/gh/awslabs/aws-c-s3/pull/418?src=pr&el=tree&filepath=source%2Fs3.c&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs#diff-c291cmNlL3MzLmM=) | `96.00% <ø> (ø)` | | | [source/s3\_auto\_ranged\_put.c](https://app.codecov.io/gh/awslabs/aws-c-s3/pull/418?src=pr&el=tree&filepath=source%2Fs3_auto_ranged_put.c&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs#diff-c291cmNlL3MzX2F1dG9fcmFuZ2VkX3B1dC5j) | `92.75% <100.00%> (+0.16%)` | :arrow_up: | | [source/s3\_meta\_request.c](https://app.codecov.io/gh/awslabs/aws-c-s3/pull/418?src=pr&el=tree&filepath=source%2Fs3_meta_request.c&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs#diff-c291cmNlL3MzX21ldGFfcmVxdWVzdC5j) | `93.14% <94.87%> (+0.89%)` | :arrow_up: | | [source/s3\_client.c](https://app.codecov.io/gh/awslabs/aws-c-s3/pull/418?src=pr&el=tree&filepath=source%2Fs3_client.c&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs#diff-c291cmNlL3MzX2NsaWVudC5j) | `87.69% <33.33%> (-0.48%)` | :arrow_down: |