Add ExponentialBackoffErrorHandler to manage message visibility tim…#1381
Conversation
tomazfernandes
left a comment
There was a problem hiding this comment.
Looking great @brun0-4ugusto and @rafaelcgpava!
I left a few of comments in the code, please let me know if they make sense.
We can add some debug or trace logs so we can follow whether visibility change was successful and to what value it was changed to for a given message id.
We also need to decide what happens if visibility change fails for a given message or set of messages - we should at least log the error clearly with context.
Abstractions
Regarding abstractions, I can see where we could benefit from code reuse, but since it's not too much logic perhaps some duplication is ok?
We can also have a ErrorHandlerVisibilityHelper class with some utility methods.
Please let me know your thoughts on all of this, thanks!
1ca5ed8 to
91f6156
Compare
|
Hello @tomazfernandes, I hope you’re doing well. We have implemented the requested changes—they were excellent ideas! We also took the opportunity to incorporate the new Thank you again! 😸 |
tomazfernandes
left a comment
There was a problem hiding this comment.
Thanks for the changes, it's looking good!
Left a couple of comments. Also, please add docs on the new error handler.
8bb87d0 to
b7d0fa1
Compare
b7d0fa1 to
f753bee
Compare
f753bee to
f4d5bfc
Compare
…eouts and implement exponential backoff logic for retries in SQS (awspring#1314) Co-authored-by: skilo <[email protected]>
f4d5bfc to
b9cecdf
Compare
|
Hi @tomazfernandes , Thank you for your feedback and comments. We have addressed the points you raised and added documentation for the new error handler. Please let us know if you have any further suggestions or questions. |
|
Thanks for the PR @brun0-4ugusto and @rafaelcgpava! I have two more issues in mind regarding error handling:
What do you think? Would you like to contribute those as well? Let me know your thoughts, thanks! |
|
Hi @tomazfernandes Thanks for the review! |
…#1381) Co-authored-by: skilo <[email protected]>
|
Hi @tomazfernandes, how are you? |
📢 Type of change
📜 Description
New ExponentialBackoffErrorHandler to change message visibility timeout exponentially based on the number of receive Message Count
💡 Motivation and Context
As mentioned in (#1313), this enhancement enables reporting an error back to AWS SQS by exponentially increasing the message's visibility timeout, allowing it to be retried.
See #1314
💚 How did you test it?
Unit test, Integration test, manual tests with a sample application
📝 Checklist
🔮 Next steps