Add support for SqsTemplate to send more than 10 messages at once - #1659
Add support for SqsTemplate to send more than 10 messages at once#1659joseiedo wants to merge 11 commits into
Conversation
Add more tests simplifying tests stop sequential send when a failure occurs
tomazfernandes
left a comment
There was a problem hiding this comment.
Thanks for the PR @joseiedo. I see you're still pushing commits, so I'll leave just one comment about the partitioning strategy. Let me know your thoughts.
| int totalSize = messagesToUse.size(); | ||
| return IntStream.rangeClosed(0, (totalSize - 1) / pageSize) | ||
| .mapToObj(index -> messagesToUse.subList(index * pageSize, Math.min((index + 1) * pageSize, totalSize))) | ||
| .mapToObj(index -> (Collection<T>) messagesToUse.subList(index * pageSize, Math.min((index + 1) * pageSize, totalSize))) |
There was a problem hiding this comment.
I'm not sure why suddenly this was happening, but the pipeline couldn't compile successfully if I didn't used this cast.
https://github.com/awspring/spring-cloud-aws/actions/runs/30174435089/job/89720833992
|
I added a binpack algorithm here, let me know your thoughts about it! |
tomazfernandes
left a comment
There was a problem hiding this comment.
Thanks for the update @joseiedo, the partitioning logic looks great.
One thing left to adjust is that currently if one sending operation in a batch fails, we'll throw the error and some messages will have been sent and others won't.
For the single batch (<= 10 messages) this works because it's all-or-nothing anyway, but for multiple batches we need a way of letting the user know what was sent and what was not.
My suggestion would be to convert exceptions to Failed entries at the partition boundary, so every future entering combineBatchFutures and the sequential chains always completes normally. Something along the lines of:
private <T> CompletableFuture<SendResult.Batch<T>> sendPartitionedBatch(String endpointName, Collection<Message> partition, Map<String, org.springframework.messaging.Message<T>> originalMessagesById) {
return sendSingleBatch(endpointName, partition, originalMessagesById).exceptionally(t -> createFailedBatchResult(partition, t, endpointName, originalMessagesById));
}Where createFailedBatchResult would mirror createSkippedResult, with the unwrapped cause (strip CompletionException) in errorMessage and additionalInformation. With the default THROW strategy the user still gets SendBatchOperationFailedException, just carrying the full result now.
What do you think?
📢 Type of change
📜 Description
AWS SDK doesn't have support to send a batch of messages of more than 10 items. This PR add an alternative where the received batch is partitioned in smaller batches of 10. For FIFO queues the batches are sent sequentially (if one fails, the others aren't sent), while for non-FIFO they are parallelized.
💡 Motivation and Context
solves #1042
💚 How did you test it?
I added integration test and unit tests with a sample.
📝 Checklist
🔮 Next steps
Check if there's concerns with this approach and update documentation explaining how this works in order to make it clear for users.