-
Notifications
You must be signed in to change notification settings - Fork 866
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
Bytes Based batching for SendMessageRequest Batching #5540
Merged
joviegas
merged 5 commits into
feature/master/sqs-batch-manager
from
joviegas/sendMaxByteSize
Aug 29, 2024
Merged
Bytes Based batching for SendMessageRequest Batching #5540
joviegas
merged 5 commits into
feature/master/sqs-batch-manager
from
joviegas/sendMaxByteSize
Aug 29, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joviegas
force-pushed
the
joviegas/sendMaxByteSize
branch
2 times, most recently
from
August 28, 2024 18:26
21b4a5d
to
996f76f
Compare
joviegas
force-pushed
the
joviegas/sendMaxByteSize
branch
from
August 28, 2024 18:27
996f76f
to
a946bd6
Compare
millems
approved these changes
Aug 29, 2024
Comment on lines
63
to
66
this.changeMessageVisibilityBatchManager = | ||
new ChangeMessageVisibilityBatchManager(RequestBatchConfiguration.builder(builder.overrideConfiguration).build(), | ||
scheduledExecutor, | ||
client); |
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.
nit: indentation is weird
joviegas
merged commit Aug 29, 2024
b3b9bb4
into
feature/master/sqs-batch-manager
4 of 13 checks passed
Quality Gate failedFailed conditions See analysis details on SonarCloud Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 11, 2024
* Codegenerate BatchManager API under AsyncClient and Initail Interfaces for BatchManager (#5321) * Codegenerate BatchManager API under AsyncClient and Add empty initial Batchmanager interfaces and Implementations * Addressed review comments * Added Internal classes required for BatchManager Implementation * Revert "Added Internal classes required for BatchManager Implementation" This reverts commit 318969b. * Internal classes and RequestBatchManager Impelementation (#5418) * Added Internal classes required for BatchManager Implementation * Added Batch Send Implementation * Handled review comments * Handled review comments * Handled review comments * Made RequestBatchManager class a Abstract class * Checkstyle issues * Removed unused methods * New lines removed * Made public static to private state for sqsBatch functions * Constants added * Sonar cloud issues fixed * commit to check why test on codebuild * Increased Timeouts for get * Added abstract methods * Handled comments to remove Builders * Handled comments to take care when batchmanager closed while pending requests * Handled comments * Checkstyle issue * Added Consumer builders args for existing APIs of BatchManager (#5514) * Receive Batch Manager Implementation (#5488) * Add Recieve Buffer Queue And its related configuration * Update ReceiveBatch manager * Recieve Batch Manager Implementation * Receive Batch Manager Implemetation * Handled review comments * Checkstyle failure * Flsky test case fixed * Flaky test case fixed * Hamdled review comments * Handled comments * Removed ReceiveMessageCompletableFuture * SdkClosable implemented * Added ReceiveMessageBatchManager class for completeness * Checkstyle issues * Null checks * Handled comments from Zoe * Updated the defaults to 50ms same as V1 after surface area review * Revert "Updated the defaults to 50ms same as V1 after surface area review" This reverts commit e7d2295. * Bytes Based batching for SendMessageRequest Batching (#5540) * Initial changes * Initial changes 2 * Byte Based batching for SendMessage API * Byte Based batching for SendMessage API * Handled comments * Checkstyle issue * Add User Agent for Sqs Calls made using Automatic Batching Manager (#5546) * Add User Agent for Sqs Calls made using Automatic Batching Manager as hll/abm * Review comments * Update comments from PR 5488 (#5550) * Update comments of PR 5488 * Update comments from PR 5488 * Handled surface area review comments (#5563) * Initial version * Intermediate changes * Update after internal poll * ResponseCOnfiguration construction updated * RequestOverride configuration check added to Bypass batch manager * Handled review comments * Removed TODO since validations are handled in BatchPverrideConfiguration * Fix issue where the Scheduled Timeout was incorrectly completing the futures with empty messages (#5571) * Fix issue where the Scheduled Timeout was incorrectly completing the futures with empty messages * Handled review comments * Integ test for Automatic Request Batching (#5576) * feat(sqs): add BatchManager for client-side request batching to Amazon SQS The new BatchManager allows for simple request batching using client-side buffering, improving cost efficiency and reducing the number of requests sent to Amazon SQS. The client-side buffering supports up to 10 requests per batch and is supported by the SqsAsyncClient. Batched requests, along with receive message polling, help to increase throughput. * Add check for scheduledExecutor such that it not null when creating SqsAsyncBatchManager (#5582) * Add check for scheduledExecutor such that it not null when creating SqsAsyncBatchManager * Update services/sqs/src/test/java/software/amazon/awssdk/services/sqs/batchmanager/SqsAsyncBatchManagerBuilderTest.java Co-authored-by: David Ho <70000000+davidh44@users.noreply.github.com> * Update services/sqs/src/test/java/software/amazon/awssdk/services/sqs/batchmanager/SqsAsyncBatchManagerBuilderTest.java Co-authored-by: David Ho <70000000+davidh44@users.noreply.github.com> * Update services/sqs/src/test/java/software/amazon/awssdk/services/sqs/batchmanager/SqsAsyncBatchManagerBuilderTest.java Co-authored-by: David Ho <70000000+davidh44@users.noreply.github.com> --------- Co-authored-by: David Ho <70000000+davidh44@users.noreply.github.com> * Updating Timeouts in gets so that we dont wait infinitely --------- Co-authored-by: David Ho <70000000+davidh44@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
Reference Doc for size of SendMessageBatch Size
https://docs.aws.amazon.com/AWSSimpleQueueService/latest/APIReference/API_SendMessageBatch.html
Reference Doc for size of message-attributes
https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-message-metadata.html#sqs-message-attributes
In our implementation, we’ve accounted for an ATTRIBUTE_MAPS_PAYLOAD_BYTES size of 16 KB. This represents a worst-case scenario where the user sends the maximum possible attribute map in SendMessage.
This consideration primarily affects batching in cases where the messages payload comes close to 240 KB. For smaller messages, this will have little to no impact on batching behavior.
Modifications
Is this config exposed to Public API
No, this is not exposed in public api we are setting a Default of 256K payload size.
Testing
Screenshots (if appropriate)
Types of changes
License