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.
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
feature: add support for
ErrorVisibilityTimeout
and job retention #571base: master
Are you sure you want to change the base?
feature: add support for
ErrorVisibilityTimeout
and job retention #571Changes from all commits
13057cf
1a86abc
6b0a683
fd060de
8477f6c
b1da4ab
97d32bc
7f5614b
fa221e8
cf36b16
bbd92e1
ca95599
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Because of
defer
here, we are now also calling this ifrequeue
is true. This did not happen before. I am not sure if this was a bug/oversight before, or if it is important thatrequeue
does not trigger this code: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.
yeah, it should be called in all cases. This tells our internal counter to decrement the number of active messages in flight (aka consumed from listener and pushed to the priority queue). Since we're done with that message anyway (delete after pushing it back to SQS), it is OK to decrement that number.
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.
Ok, so it was a bug that it was not called before, on these lines? Or is it a bug that we now always call it even when requeuing?
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.
Yeah, I guess that was a bug. I incremented the counter in the listener.go, after requeue, we still had +1 message + another one when the same message read in the listener.go.