Skip to content
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

Deduplicate message batches in SQS Poller #2918

Merged
merged 11 commits into from
Sep 29, 2023
Merged

Conversation

jterapin
Copy link
Contributor

Fixes an issue mentioned in #2908

This PR ensures that there's no duplicate messages before yield. To successfully delete a message, it must have the "most recently receipt handle" as mentioned here under Receipt handle.

Also, ran rubocop to keep the files up to today's standards.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

To make sure we include your contribution in the release notes, please make sure to add description entry for your changes in the "unreleased changes" section of the CHANGELOG.md file (at corresponding gem). For the description entry, please make sure it lives in one line and starts with Feature or Issue in the correct format.

For generated code changes, please checkout below instructions first:
https://github.com/aws/aws-sdk-ruby/blob/version-3/CONTRIBUTING.md

Thank you for your contribution!

@jterapin jterapin marked this pull request as ready for review September 28, 2023 18:52
Copy link
Contributor

@alextwoods alextwoods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice - approach looks good!

gems/aws-sdk-sqs/CHANGELOG.md Outdated Show resolved Hide resolved
gems/aws-sdk-sqs/lib/aws-sdk-sqs/queue_poller.rb Outdated Show resolved Hide resolved
gems/aws-sdk-sqs/lib/aws-sdk-sqs/queue_poller.rb Outdated Show resolved Hide resolved
gems/aws-sdk-sqs/lib/aws-sdk-sqs/queue_poller.rb Outdated Show resolved Hide resolved
gems/aws-sdk-sqs/lib/aws-sdk-sqs/queue_poller.rb Outdated Show resolved Hide resolved
gems/aws-sdk-sqs/lib/aws-sdk-sqs/queue_poller.rb Outdated Show resolved Hide resolved
@mullermp mullermp changed the title Fix SQS QueuePoller Deduplicate message batches in SQS Poller Sep 28, 2023
Copy link
Contributor

@mullermp mullermp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approach looks good... looks like rubocop screwed up most things.

gems/aws-sdk-sqs/lib/aws-sdk-sqs/queue_poller.rb Outdated Show resolved Hide resolved
gems/aws-sdk-sqs/CHANGELOG.md Outdated Show resolved Hide resolved
gems/aws-sdk-sqs/lib/aws-sdk-sqs/queue_poller.rb Outdated Show resolved Hide resolved
gems/aws-sdk-sqs/lib/aws-sdk-sqs/queue_poller.rb Outdated Show resolved Hide resolved
gems/aws-sdk-sqs/lib/aws-sdk-sqs/queue_poller.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@alextwoods alextwoods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

# duplicated messages will have a different receipt handle
# so we need to provide the most recent receipt to
# delete a batch - thus, filtering below by message_id
messages = messages.reverse.uniq(&:message_id).reverse!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@jterapin jterapin merged commit 457de83 into version-3 Sep 29, 2023
26 checks passed
@jterapin jterapin deleted the fix_sqs_queue_poller branch September 29, 2023 17:43
okkez added a commit to reproio/ecs_deploy that referenced this pull request Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants