search

awslabs / amazon-sqs-java-extended-client-lib

An extension to the Amazon SQS client that enables sending and receiving messages up to 2GB via Amazon S3.
Apache License 2.0
211 stars 109 forks source link

Yet another add async support for S3 #79

Closed richardsmithsfdc closed 7 months ago

richardsmithsfdc commented 1 year ago

Issue #, if available: 5

Description of changes:

Add async S3 support, based on latest async support in payload offloading. This PR cannot be merged until payload offloading version 2.2.0 with async support is released:

Creating this as a draft PR for now.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

ziyanli-amazon commented 7 months ago

@richardsmithsfdc Thanks for doing this! Appreciate all the amazing work done in this PR.

richardsmithsfdc commented 7 months ago

@ziyanli-amazon , thanks for merging this. FYI, I didn't do a ton of testing after updating to latest master. It was working before, and the UTs are still passing. I noticed that delete message is not chaining the S3 delete with the SQS delete. I think that should work because the result of the S3 delete doesn't really matter, but just wanted to let you know.

richardsmithsfdc commented 7 months ago

@ziyanli-amazon , I remember the reason I wrote the code to ignore the S3 delete result, which is that if it is deleting multiple S3 messages, and one fails, I still wanted to delete the SQS messages because otherwise it might leave some messages in SQS that have their payloads deleted. This scenario can still occur with the synchronous extended client. In general, though, I think it's poor practice to leave unchained futures, just in case calling code wants to wait for all operations to complete. I will probably make a future PR to solve this issue in both.

Thanks again for merging the PRs!