Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Closed

Conversation

Toflar
Copy link
Contributor

I've documented the relation ofconsumer andgroup in how they behave depending on your needs.

Slightly related:
One thing I just encouter in a project is that if you use theHOSTNAME 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?

glaubinix reacted with thumbs up emoji
@Toflar
Copy link
ContributorAuthor

Ah, maybe for k8s aStatefulSet is the way to go...

@Toflar
Copy link
ContributorAuthor

Toflar commentedJul 3, 2019
edited
Loading

It indeed is 😄 Works like a charm, I just deployed that to staging and now I have deterministic worker hostnames 🎉
So remember: Write Symfony docs from time to time and you'll suddenly rethink stuff and find better solutions to a problem 😄

@ToflarToflarforce-pushed themessenger-redis-consumer-group branch from4872db2 to525dfdcCompareJuly 3, 2019 08:37
messenger.rst Outdated
framework:
messenger:
transports:
redis: 'redis://localhost:6379/messages/symfony/%env(HOSTNAME)%'
Copy link
Member

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?

Copy link
ContributorAuthor

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 :)

Copy link
Contributor

@alexander-schranzalexander-schranzJul 3, 2019
edited
Loading

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.

Copy link
ContributorAuthor

@ToflarToflarJul 3, 2019
edited
Loading

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)%'

Copy link
Member

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.

Copy link
ContributorAuthor

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.
Copy link
Member

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

Nyholm reacted with thumbs up emoji
Copy link
Contributor

@tooonitoooniJul 30, 2020
edited
Loading

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
Copy link

xavismeh commentedJul 4, 2019
edited
Loading

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: theauto_setup configuration flag can be set either as an option (same as Amqp IIRC) or using a query string in the DSN. However, the behavior you've documented seems not to be working as expected. Will investigate further ...

@Toflar
Copy link
ContributorAuthor

@xavismeh That's documenting another feature of the Redis transport, I think we should do that in a separate PR :)

@OskarStarkOskarStark added this to the4.3 milestoneAug 15, 2019
@OskarStark
Copy link
Contributor

@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 👍
Thank you 🙏

@javiereguiluz
Copy link
Member

Maybe@Nyholm can help us here too to make the final review. Thanks!

@xavismeh
Copy link

@xavismeh That's documenting another feature of the Redis transport, I think we should do that in a separate PR :)

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 :)

Copy link
Member

@NyholmNyholm left a 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)%'
Copy link
Member

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?

Copy link
ContributorAuthor

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? 😊

Copy link

@sneakyvvsneakyvvOct 17, 2019
edited
Loading

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
Copy link
Member

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?

Copy link
ContributorAuthor

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
Copy link
Member

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?

Copy link
ContributorAuthor

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
Copy link
Contributor

toooni commentedJan 18, 2020
edited
Loading

Please wait with merging this PR.
The current redis implementation does not yet handle abandoned messages because of parting consumers. I am currently updating the current solution to useXPENDING andXCLAIM to claim abandoned messages by another consumer. (symfony/symfony#35384)
This makes at least the warning about not using the hostname as the consumer name obsolete.

fabpot added a commit to symfony/symfony that referenced this pull requestFeb 8, 2020
…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
fabpot added a commit to symfony/symfony that referenced this pull requestApr 17, 2020
…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
Copy link

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

@wouterj
Copy link
Member

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?

@Toflar
Copy link
ContributorAuthor

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?

@Nyholm
Copy link
Member

Since#12976 has been merged. I suggest closing this

@javiereguiluz
Copy link
Member

Nice! Let's close then. Thanks for checking this!

javiereguiluz added a commit that referenced this pull requestApr 21, 2021
…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)%'

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?

Copy link
Member

@NyholmNyholmJul 16, 2021
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

najmi9 reacted with thumbs up emoji

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?

@ToflarToflar deleted the messenger-redis-consumer-group branchAugust 3, 2023 11:48
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@weaverryanweaverryanweaverryan requested changes

@tooonitooonitoooni left review comments

@NyholmNyholmNyholm left review comments

@sneakyvvsneakyvvsneakyvv left review comments

@alexander-schranzalexander-schranzalexander-schranz left review comments

@najmi9najmi9najmi9 left review comments

Assignees
No one assigned
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.

13 participants
@Toflar@xavismeh@OskarStark@javiereguiluz@toooni@jak@wouterj@Nyholm@weaverryan@sneakyvv@alexander-schranz@najmi9@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp