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

[Messenger] Add a redis stream transport#30917

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

Merged
fabpot merged 2 commits intosymfony:masterfromalexander-schranz:redis-messenger
Apr 27, 2019

Conversation

@alexander-schranz
Copy link
Contributor

@alexander-schranzalexander-schranz commentedApr 6, 2019
edited
Loading

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?Yes
Fixed tickets#28681
LicenseMIT
Doc PRsymfony/symfony-docs#11341

As discussed in#28681 this will refractor@soyuka implementation of redis using the redis stream features so we don't need to handle parking the messages ourself and redis is doing it for us.

Some interesting links about streams:

+-----------R|    GET    | -> XREADGROUP+-----------+      |      | handleMessage      V+-----------+  No|  failed?  |---------------------------++-----------+                           |      |                                 |      | Yes                             |      V                                 |+-----------+  No                       ||   retry?  |---------------------------++-----------+                           |      |                                 |      | Yes                             |      V                                 V+-----------R                     +-----------R|   REJECT  | -> XDEL             |    ACK    | -> XACK+-----------+                     +-----------+

GET: Will useXREADGROUP to read the one message from the stream
REJECT: Reject will just remove the message withXDEL from the stream as adding it back to the stream is handled by symfony worker itself
ACK: Will use theXACK Method to ack the message for the specific group

The sender will still be simple by calling theXADD redis function.

#EU-FOSSA

trickreich, chalasr, derrabus, and BafS reacted with thumbs up emojiderrabus reacted with rocket emoji
@curry684
Copy link
Contributor

Needs[Messenger] prefix ;)

@lyrixxlyrixx changed the titleAdd a redis stream transport[Messenger] Add a redis stream transportApr 6, 2019
Copy link
Member

@chalasrchalasr left a comment
edited
Loading

Choose a reason for hiding this comment

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

Thanks for this, I want it for 4.3 :)
The framework integration is missing, we need to register the redis transport factory in FrameworkBundle's messenger.xml (and add a test for it)

@alexander-schranz
Copy link
ContributorAuthor

@chalasr thank you for your review. did fix it and register the factory in the framework bundle.

@chalasr
Copy link
Member

Adding a test in

as for amqp would be nice.

@alexander-schranzalexander-schranzforce-pushed theredis-messenger branch 4 times, most recently fromd0b63af to4ed23bdCompareApril 25, 2019 22:59
Copy link
Member

@chalasrchalasr left a comment
edited
Loading

Choose a reason for hiding this comment

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

Tried this on a real project (will allow me to drop enqueue/messenger-adapter 🎉 ), works just fine.
I think implementingMessageCountAwareInterface can be done later, not a blocker.

@fabpot
Copy link
Member

Thank you@alexander-schranz.

@fabpotfabpot merged commitff0b855 intosymfony:masterApr 27, 2019
fabpot added a commit that referenced this pull requestApr 27, 2019
…ander-schranz)This PR was merged into the 4.3-dev branch.Discussion----------[Messenger] Add a redis stream transport| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | Yes| Fixed tickets |#28681| License       | MIT| Doc PR        |symfony/symfony-docs#11341As discussed in#28681 this will refractor@soyuka implementation of redis using the redis stream features so we don't need to handle parking the messages ourself and redis is doing it for us.Some interesting links about streams: -https://redis.io/topics/streams-intro -https://brandur.org/redis-streams```+-----------R|    GET    | -> XREADGROUP+-----------+      |      | handleMessage      V+-----------+  No|  failed?  |---------------------------++-----------+                           |      |                                 |      | Yes                             |      V                                 |+-----------+  No                       ||   retry?  |---------------------------++-----------+                           |      |                                 |      | Yes                             |      V                                 V+-----------R                     +-----------R|   REJECT  | -> XDEL             |    ACK    | -> XACK+-----------+                     +-----------+```**GET**: Will use `XREADGROUP` to read the one  message from the stream**REJECT**: Reject will just remove the message with `XDEL` from the stream as adding it back to the stream is handled by symfony worker itself**ACK**: Will use the `XACK` Method to ack the message for the specific groupThe sender will still be simple by calling the `XADD` redis function.#EU-FOSSACommits-------ff0b855 Refractor redis transport using redis streams7162d2e Implement redis transport
@alexander-schranzalexander-schranz deleted the redis-messenger branchApril 27, 2019 17:09
@soyuka
Copy link
Contributor

Nice work thanks@alexander-schranz !

@alexander-schranz
Copy link
ContributorAuthor

@soyuka thank you for initial work!

chalasr reacted with thumbs up emoji

@nicolas-grekasnicolas-grekas modified the milestones:next,4.3Apr 30, 2019
@fabpotfabpot mentioned this pull requestMay 9, 2019
wouterj added a commit to symfony/symfony-docs that referenced this pull requestMay 11, 2019
…chranz)This PR was submitted for the master branch but it was squashed and merged into the 4.3 branch instead (closes#11341).Discussion----------Add documentation for the Redis transportThis will add documentation how to configure and using the redis transport with the messenger component.symfony/symfony#30917#EUFOSSACommits-------c22fade Add documentation for the Redis transport
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@lyrixxlyrixxlyrixx left review comments

@jderussejderussejderusse left review comments

@fabpotfabpotfabpot approved these changes

@chalasrchalasrchalasr approved these changes

+2 more reviewers

@jewome62jewome62jewome62 left review comments

@onEXHoviaonEXHoviaonEXHovia left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

14 participants

@alexander-schranz@curry684@onEXHovia@weaverryan@soyuka@Simperfit@chalasr@fabpot@lyrixx@jewome62@jderusse@nicolas-grekas@sroze@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp