Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.2k
Added redeliver_timeout and claim_interval options#12976
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| There should never be two or more `messenger:consume` commands running with the same | ||
| config (stream, group and consumer name) to avoid having a message handled more than once. | ||
| Using the ``HOSTNAME`` as the consumer might often be a good idea. In case you are using | ||
| Kubernetes to orchestrate your containers, consider using a ``StatefulSet``. |
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.
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.
Well, with this PR it's not really relevant anymore. You can use a regularDeployment which changes hostnames after restarts but the PR is here to fix exactly that. TheStatefulSet is just so that you always get the same hostnames back.
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 thats correct still think its good to have the hostname which doesn't change as it avoids spaming redis with new consumers as they recommend to reuse consumer names.
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.
Then that should probably be mentioned here but other than that, there's nothing to add regarding k8s :)
…is) (toooni)This PR was merged into the 5.1-dev branch.Discussion----------[Messenger] Add receiving of old pending messages (redis)| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| Deprecations? | no| Tickets || License | MIT| Doc PR |symfony/symfony-docs#12976This PR makes it possible for the redis transport to get abandoned messages from not running/idle consumers by using `XPENDING` and `XCLAIM`.Usually it would be best to let the claiming of pending messages be handled by a separate command. Since the messenger component's commands are fixed, we do need to set a `claimTimeout`. The `claimTimeout` defines how long an idle message should be left alone until it will get claimed by the current consumer (Must be a value higher than the longest running handling time of a message or else the message will be handled twice).Using this solution makes the remarks (symfony/symfony-docs#11869 (review)) regarding not being able to use the hostname as consumer name obsolete. I would even recommend the hostname as the consumer name.**Questions**- [x] Which value should we use as default `claimTimeout`?- [x] How should the `claimTimeout` be configured?- [x] Feature or Bugfix?I will create a docs PR and a PR for the other branches as soon as the questions are resolved.Commits-------9cb6fdf Implemted receiving of old pending messages
@Nyholm can you have a look at this PR please? |
messenger.rst Outdated
| ================== ===================================== ========================= | ||
| .. caution:: | ||
| There should never be two or more `messenger:consume` commands running with the same |
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.
There should never be more than onemessenger:consume...
I think this looks good. It needs a rebase and syntax fix on the first column in the table |
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.
Thank you
I am happy with these changes.
Thank you@toooni! I'm sorry for the long wait, there has been a great lack of Messenger knowledge in the doc reviewer time for a long while :( At last, this is now merged in 5.1 (and I'll merge it into 5.2 from there). |
Wooho. Congratulations! |
* 5.1: [symfony#12976] Added the new options to the versionadded directive [symfony#12976] Minor syntax fix Added redeliver_timeout and claim_interval options [Security] Remove extra argument from call to EntityManager#flush()
Adds new optional config parameters introduced bysymfony/symfony#35384