-
-
Notifications
You must be signed in to change notification settings - Fork 132
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
[#244] MQ: Add initial-backoff option to enqueue #245
Conversation
@st3fan Hi Stefan, definitely open for a PR- and this is looking good, thanks!
Allowed + wanted 👍 |
Thank you for tips - I'll work on some improvements and better tests. I also left a question at https://github.com/ptaoussanis/carmine/pull/245/files#diff-9f2681c4ea1ec7b7c79bb31ad60f7b4847786cd2b4f3df935e8eb8c58869d364R176 but now I am actually wondering if the behaviour under that comment is correct .. if there are two messages available, should |
07ee929
to
9af62e4
Compare
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.
Requesting one last argument tweak (sorry for any confusion).
Otherwise PR looks solid to me. Tests look good.
Once you can update, I'll add the enqueue
backwards-compatibility and cut a new release.
Thanks a lot for the great work on this.
Cheers
6ef7866
to
76dfe61
Compare
https://github.com/ptaoussanis/carmine/releases/tag/v3.1.0 is now on Clojars 👍 Cheers! |
For #244 - This patch adds an
:initial-backoff-ms
option toenqueue
. I implemented this by allowing an initial value to be set in theqk-backoffs
hash on message creation. This seems to do the trick.Also included a new test to validate the
:initial-backoff-ms
behaviour. I think it would be interesting to do a bit of a stress test with this feature - are longer running tests allowed/wanted?