-
-
Notifications
You must be signed in to change notification settings - Fork 353
Add automatic SQS request batching support component: sqs #1438
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
Conversation
56119f4 to
23ee5eb
Compare
|
Hi @khc41, thanks again for the PR. I’ve taken a closer look and wanted to share some thoughts. Overall, I think adding support for batching via the new AWS SDK implementation is a valuable direction. That said, I believe the best path forward would be to scope the integration a bit more narrowly than what’s currently proposed. Listener Use CaseFor the Template and Custom FlowsThe value of batching is clearer for Focus on the AdapterTo me, the most compelling piece in this PR is the I’d suggest focusing the contribution around:
This direction keeps the structure composable and aligned with the framework’s current architecture, while still enabling users to adopt the batching client where it makes the most impact. Happy to hear your thoughts, and open to adjustments if you see strong use cases on the listener side. Thanks again for all the work on this! |
|
Hi @tomazfernandes, Thank you for your thoughtful and detailed feedback. I completely agree with your suggestions. You've made an excellent point about the @SqsListener use case; the added complexity likely outweighs the benefits since it already handles batching internally. Focusing on SqsTemplate and custom flows is the right direction. As you proposed, I will update the PR to:
Thanks again for the great guidance! I'll push the changes soon. |
… SQS automatic request batching.
|
Hi @tomazfernandes, Thank you again for the detailed feedback. I've pushed the updates to align with your suggestions. The PR is now focused on providing the I'd appreciate it if you could take another look when you have a moment. Thanks! |
|
Hi @tomazfernandes, After giving it some more thought, I wanted to leave a comment to explain the reasoning behind guiding users to use @qualifier in the documentation. My concern is that if the BatchingSqsClientAdapter were registered as the primary SqsAsyncClient bean, it could unintentionally affect other components, most notably the @SqsListener infrastructure. While the AWS SDK documentation states that receiveMessage can be bypassed under certain conditions, other operations in the listener's lifecycle, such as deleteMessage or changeMessageVisibility, would still risk being batched. Therefore, I concluded that explicitly separating them using @qualifier is the safest and most predictable approach. I believe this aligns well with your previous feedback, but please let me know if you have a different perspective. I'd appreciate it if you could take a look when you have a moment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @khc41, thanks for the updates.
I've left a couple of comments - would you be able to run a couple of tests to assert the exact behavior for sending and receiving messages, so we can document it?
Let me know if it makes sense to you, thanks.
docs/src/main/asciidoc/sqs.adoc
Outdated
|
|
||
| ==== Automatic Batching with AWS SDK | ||
|
|
||
| Spring Cloud AWS supports automatic message batching using AWS SDK's `SqsAsyncBatchManager`. This feature optimizes SQS operations by automatically batching requests under the hood to improve performance and reduce AWS API calls. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about we call out earlier that this feature is mostly intended to be used with the SqsTemplate rather than a general client to be used as a default bean, including in @SqsListener?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the docs with this note at the beginning of the section
Please note that this feature is primarily intended for use with
SqsTemplateand is not recommended for use with `@SqsListener due to the complexities involved in message acknowledgment and processing.
docs/src/main/asciidoc/sqs.adoc
Outdated
|
|
||
| ===== Important Considerations & Best Practices | ||
|
|
||
| WARNING: **Asynchronous Operations and False Positives**. The batching client processes operations asynchronously. A call to `sqsTemplate.sendAsync(...)` might return a `CompletableFuture` that completes successfully before the message is actually sent to AWS. The actual transmission happens later in a background thread. This can lead to **false positives**. Always attach error handling to the `CompletableFuture` to detect and handle real transmission failures. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually standard behavior for sendAsync operations, so I wonder what's the specific behavior for this client.
For instance, if we execute a send operation and join the thread, will it wait until sendRequestFrequency to complete the Future?
Also, this part of the documentation states: When you poll the SqsAsyncBatchManager#receiveMessage method in your application, the batch manager fetches messages from its internal buffer, which the SDK automatically updates in the background.
I would assume this background buffering would only start after the first call to receiveMessage, but that's worth checking as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. This is standard for sendAsync, but here's the specific behavior for this batching client:
- For send operations: Yes, .join() waits until the sendRequestFrequency to complete the Future. Verified this with the new integration test
shouldSendMessageJoinCompletesAfterFrequency. - For receiveMessage buffering: Your assumption is correct. It starts only after the first call to receiveMessage. Confirmed via AWS SDK source code (buffer is created on demand via computeIfAbsent), but testing directly is challenging due to internal implementation. Here's the relevant snippet:
public CompletableFuture<ReceiveMessageResponse> batchRequest(ReceiveMessageRequest request) {
String ineligibleReason = this.checkBatchingEligibility(request);
if (ineligibleReason == null) {
return ((ReceiveBatchManager)this.receiveBatchManagerMap.computeIfAbsent(this.generateBatchKey(request), (key) -> this.createReceiveBatchManager(request))).processRequest(request);
} else {
log.debug(() -> String.format("Batching skipped. Reason: %s", ineligibleReason));
return this.sqsClient.receiveMessage(request);
}
}|
Hi @tomazfernandes, Got it, thanks for the feedback. I'll run those tests to clarify the behavior and update the docs accordingly. |
…mization and add integration test for message send frequency
|
Hi @tomazfernandes, Thanks again for the detailed feedback. I've pushed the updates addressing all your points. Here's a quick summary of what's changed:
I'd appreciate it if you could take another look when you have a moment. |
…th SqsAsyncClient and enhance warnings about asynchronous operations
| * By configuring these globally on the manager, you ensure consistent batching performance. If you require per-request | ||
| * attribute configurations, using the standard {@code SqsAsyncClient} without this adapter may be more appropriate. | ||
| * @author Heechul Kang | ||
| * @since 3.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be included in the upcoming 4.0.0 release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...g-cloud-aws-sqs/src/main/java/io/awspring/cloud/sqs/operations/BatchingSqsClientAdapter.java
Outdated
Show resolved
Hide resolved
…ehavior and enhance error handling instructions
|
Thanks @tomazfernandes! |
|
Thanks for the updates @khc41! There's a failing test in the CI though, could you take a look please? |
… sleep and add wait time for message reception
|
Hi @tomazfernandes, I've pushed a fix for the flaky CI test. It was a timing issue caused by I replaced all Could you please take another look when you have a moment? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @khc41! Looking forward to more!
📢 Type of change
📜 Description
Added automatic SQS batching support using AWS SDK's SqsAsyncBatchManager.
This feature enables automatic request batching for SQS operations to improve performance and reduce AWS API calls. When enabled, Spring Cloud AWS automatically wraps the
SqsAsyncClientwith aBatchingSqsClientAdapterthat usesSqsAsyncBatchManagerinternally.Key Features:
sendMessage,receiveMessage,deleteMessage, andchangeMessageVisibilityoperationsSqsTemplateand@SqsListenercodeConfiguration Example:
Important: Operations are processed asynchronously. Applications should handle returned
CompletableFutureobjects to detect actual transmission errors.💡 Motivation and Context
AWS SDK 2.28.0+ introduced automatic request batching capabilities that can significantly improve SQS performance and reduce costs by combining multiple API calls into fewer requests.
Previously, users had to manually implement batching logic or use explicit batch operations. This change enables declarative batching at the configuration level, providing:
This addresses the community request for supporting AWS SDK's new automatic batching feature in Spring Cloud AWS.
💚 How did you test it?
Unit Tests:
BatchingSqsClientAdapterTests: Comprehensive testing of all adapter methods with mock SqsAsyncBatchManagerIntegration Tests:
BatchingSqsClientAdapterIntegrationTests: Real SQS environment testing with LocalStackAuto-Configuration Tests:
SqsAutoConfigurationTestwith comprehensive batch configuration testingspring.cloud.aws.sqs.batch.enabled📝 Checklist
🔮 Next steps
This implementation provides full support for AWS SDK's automatic batching capabilities. Future enhancements could include:
Related AWS SDK Documentation: Use automatic request batching for Amazon SQS
Resolves #1355