terricain / aioboto3

Wrapper to use boto3 resources with the aiobotocore async backend
Apache License 2.0
743 stars 76 forks source link

Fix bug that file_reader may stall the main thread #316

Closed rlindsberg closed 1 year ago

rlindsberg commented 1 year ago

closes #315

rlindsberg commented 1 year ago

I did some experiments again. By default it uses 10 coroutines for uploader and 1 coroutine for file_reader. When the multipart id was greater than 10 000, I got exception from AWS:

An error occurred (InvalidArgument) when calling the UploadPart operation: Part number must be an integer between 1 and 10000...

At this time, the io_queue length is 0. Then file_reader continues to add stuff to the queue until it reaches 100 and reaches a dead lock.

There is a potential issue that the upload speed is low. Then queue length will be full basically all the time. Then after one uploader coroutine poping from io_queue and awaiting on:

https://github.com/terrycain/aioboto3/blob/bbd5c2352f032a58d46b3416e39c9a15881983b6/aioboto3/s3/inject.py#L233

, there is a chance that file_reader puts a new item in the queue. Thus, file_reader will be stuck on:

https://github.com/terrycain/aioboto3/blob/bbd5c2352f032a58d46b3416e39c9a15881983b6/aioboto3/s3/inject.py#L306

However, I could only reproduce this issue once a few days ago. Now after this PR, this problem seems to have disappeared.

@terrycain Thanks for the review and I will now merge this PR.

rlindsberg commented 1 year ago

Seems like I don't have permission. Can you merge it please? Shall we bump the version on PyPI as well?

terricain commented 1 year ago

Yeah if the upload speed is slow, file_reader will block when adding to a queue... I think that's fine as either a user will cancel the call and everything'll be interrupted with CancelledError or it'll just take a while.

As for the multipart id going over 10000, the user'll just need to adjust Config.multipart_chunksize.

I'll do a release on the weekend hopefully and include this :)

rlindsberg commented 1 year ago

@terrycain It's wierd that Github says that the commit is unverified: https://github.com/terrycain/aioboto3/commit/2aee647468ad4c4b79cbbafe99e10e59994c9112

I have signed my commit so it should be a green check, right?

terricain commented 11 months ago

Odd, when i do a git log --show-signature locally I dont see a signature. 🤷🏻

terricain commented 11 months ago

This is out in v11.3.1

rlindsberg commented 11 months ago

Odd, when i do a git log --show-signature locally I dont see a signature. 🤷🏻

If you try this you will se the signature:

git clone https://github.com/rlindsberg/aioboto3.git
git log --show-signature
rlindsberg commented 11 months ago

This is out in v11.3.1

Thank you so much!