Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.2k
Documented redis consumer group relation#11869
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
Ah, maybe for k8s a |
Toflar commentedJul 3, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
It indeed is 😄 Works like a charm, I just deployed that to staging and now I have deterministic worker hostnames 🎉 |
4872db2
to525dfdc
CompareUh oh!
There was an error while loading.Please reload this page.
messenger.rst Outdated
framework: | ||
messenger: | ||
transports: | ||
redis: 'redis://localhost:6379/messages/symfony/%env(HOSTNAME)%' |
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.
What would we use here if we weren’t using Docker?
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.
You just have to make sure to pass individual consumer names. In whatever way you do that. So I can't tell you, it depends on what you use :)
alexander-schranzJul 3, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
hostname (php -r "var_dump(gethostname());"
) would also work in many cases for none docker setup as its unique in its network. If not because servers are not in the same network it I think he need to set it manually like the example I posted above or find another variable e.g.:
# Worker 1DSN=redis://localhost:6379/messages/symfony/consumer-1# Worker 2DSN=redis://localhost:6379/messages/symfony/consumer-2
Or find other case uniqueness like a domain prefix.
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.
E.g. forsupervisor
we need to find out how we can somehow pass on thenumprocs
as individual identifiers. I haven't tested but something like this might be working:
[program:messenger-consume]command=php /path/to/your/app/bin/console messenger:consume async --time-limit=3600user=ubuntunumprocs=2autostart=trueautorestart=trueprocess_name=%(program_name)s_%(process_num)02denvironment = PROCESS_NUM=%(process_num)d
And then maybe use
redis: 'redis://localhost:6379/messages/symfony/consumer-%env(PROCESS_NUM)%'
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.
Cool! Can we try that? To create a generic solution, we could say to use some generic env var like REDIS_CONSUMER - the difference would just be then how youset that, depending on your setup.
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.
I tried now but I'm having some issues with finding the right spot 😄 It's not anything strictly related to the Redis transport. It's for any transport that needs deterministic values so I figured I'd place it as a tip where it imho belongs: the supervisor config part 😄
to be another worker with exactly the same ``HOSTNAME`` as the one you destroyed. In other words, you have to | ||
make sure you're using deterministic ``HOSTNAME`` values such as ``worker-1``, ``worker-2`` etc. | ||
In case you are using Kubernetes to orchestrate your containers, consider using a ``StatefulSet`` rather than | ||
a ``Deployment`` for example. |
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.
Let’s shorten this :). These cautions are great for the few people who have this case, but they tend to “weigh down” the docs over time.
I think adding a comment in the code blocks above might be enough:
For Kubernetes, use a StatefulSet to avoid randomly host 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.
This caution block can be removed since abandoned messages are now claimed:symfony/symfony#35384
xavismeh commentedJul 4, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
UPDATE: if a message matches multiple transports, it will be processed by all of them... This PR could be an opportunity to also inform something very specific to redis: the |
… Redis and maybe others
@xavismeh That's documenting another feature of the Redis transport, I think we should do that in a separate PR :) |
@weaverryan could you please give this PR a final review and merge it if everything is ok? I don't want to get this rotten 👍 |
Maybe@Nyholm can help us here too to make the final review. Thanks! |
xavismeh commentedSep 24, 2019
Experimentations at my level ended badly so I could not be able anymore to tell which feature is missing documentation :/ I'm helpless on this, sorry :) |
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.
I like this PR.
It assumes that you know about redis streams and consumer groups. I was quite confused when I first reviewed this. Maybe there should be a link and/or a sentence about that.
framework: | ||
messenger: | ||
transports: | ||
redis: 'redis://localhost:6379/messages/symfony/%env(REDIS_CONSUMER)%' |
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.
I wound use a transport value like this. I would have that in an environment variable, right?
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.
Sorry, I don't understand your question here, can you elaborate maybe? 😊
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.
I think@Nyholm meant thatredis: 'redis://localhost:6379/messages/symfony/%env(REDIS_CONSUMER)%'
would actually beredis: '%env(MESSENGER_TRANSPORT_DSN)%'
or something like that.
However, for the purpose of this doc it's more appropriate as it is now.
.. tip:: | ||
To get a deterministic worker name like for e.g. the Redis transport, you may pass the process number as an |
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.
I miss informationwhy I would like to do this.
We do this to leverage the Redis stream "consumer group" feature, right?
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.
Correct. To me thewhy doesn't really matter here. It's just a tip and as we're in the Subervisor section here I don't want to get specific about Redis. I'm sure there are other use cases where you may want to set an environment variable.
At some point you'll likely want to scale the number of workers working on your queue. | ||
Make sure you assign the correct ``consumer`` and ``group`` values in that case. | ||
The more likely case is that you want every worker to work on the queue independently, reducing |
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.
Why is it likely?
reducing the time needed to process the pending messages
I just googled about redis stream and their consumer groups. Maybe a link to redis docs should be added here?
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 was hesitant getting too much into how Redis streams work because that's already documented in the Redis docs (e.g.https://redis.io/topics/streams-intro). Actually that's the first link you get when you google for"redis streams"
:)
Why is it likely?
Why scaling is likely? If one worker was enough, a simple database queue would probably be enough. I don't think one would choose Redis if the load wasn't bigger :)
toooni commentedJan 18, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Please wait with merging this PR. |
…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
…Seldaek)This PR was merged into the 5.1-dev branch.Discussion----------[RedisMessengerBridge] Add a delete_after_ack optionThis allows Messenger to clean up processed messages from memory, avoiding a mem "leak" in redis| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| Deprecations? | no| Tickets |#33715| License | MIT| Doc PR | symfony/symfony-docs#... TODO - will pile it on tosymfony/symfony-docs#11869 as it kinda binds together and a bigger refactor of the docs here is much needed to avoid all these gotchasRight now by default a redis transport for messenger will leak memory as all messages stay in redis forever. You can configure `stream_max_entries` to automatically trim to a max of X entries, but that means if you have big peaks in messages you might start losing messages which have not been processed.This PR provides an alternative to that, by deleting message as they are processed. This is ideal as it avoids having to find the right number for `stream_max_entries` (do you want to risk losing data or use more memory than needed on average?). The only catch is that if you have multiple groups consuming the same stream, the first one processing a message will delete it, so other groups will not see it. For that reason `setup()` attempts to detect this and fails hard if it is misconfigured to prevent data loss.Commits-------7c416a7 [RedisMessengerBridge] Add a delete_after_ack option to automatically clean up processed messages from memory
jak commentedJul 30, 2020
I've just found this PR immensely useful after finding scaling redis consumers problematic! Thank you for your contributions! Now thatsymfony/symfony#35384 is merged, it would be great to see these very helpful docs added to help others like me |
Hi@Toflar! I've never used the Messenger component and I haven't looked at this PR. However, it seems to have limited activity for a long time. Is this PR still relevant and do you have time to finish it? |
Hey, I've moved away from using Redis Streams for my queue so I haven't even used the changes insymfony/symfony#35384. I don't know if this is still relevant and where it would fit in best.@jak would you mind picking this up? |
Since#12976 has been merged. I suggest closing this |
Nice! Let's close then. Thanks for checking this! |
…aelo)This PR was merged into the 5.2 branch.Discussion----------[Messenger] More on unique Redis consumer namesI managed to miss the warning that the (strem,group,consumer) combination must be unique. I think it would make it less easy to miss if it was also mentioned in the Supervisor section, so I've tried to add a mention of it there. I also expanded the original warning block a bit.There are many different combinations of transports and ways to manage workers, so I understand that the docs should not delve too deep into each one, but I think it's worthwhile to cover the Redis + Supervisor combo quite well, since it's one of the easier combinations to get started with, which also can work really well. Let me know what you think.Related:#11869 #35358Commits-------e73e923 [Messenger] More on unique Redis consumer names
framework: | ||
messenger: | ||
transports: | ||
redis: 'redis://localhost:6379/messages/symfony/%env(REDIS_CONSUMER)%' |
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.
REDIS_CONSUMER
Environment variable not found? How I can get this variable from supervior configuration?
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.
It is in the code block just before.
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.
@Nyholm What I did is: I added the lineenvironment = REDIS_CONSUMER=consumer-%(process_num)d
to the configuration file/etc/supervisor/conf.d/messenger-worker.conf
, now my problem is in Symfony application the variableREDIS_CONSUMER
is not defined, so how I can defined it?
I've documented the relation of
consumer
andgroup
in how they behave depending on your needs.Slightly related:
One thing I just encouter in a project is that if you use the
HOSTNAME
variable it works just fine as long as you're not destroying the container while it's still working on a message. If I destroy the container, the next one gets a newHOSTNAME
leaving the message that the previous container was working on in a pending state forever. In fact, it would be better to have something likeworker-1
,worker-2
etc. but I have no idea how to build something like that in e.g. a k8s deployment where you just define the replicas.If we can fix that somehow: ideas welcome 😄
If not: Maybe we should document that here too?