-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
e355a4e
to
62b159c
Compare
Needs |
src/Symfony/Component/Messenger/Tests/Transport/RedisExt/ConnectionTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/Tests/Transport/RedisExt/ConnectionTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/Tests/Transport/RedisExt/ConnectionTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/Tests/Transport/RedisExt/ConnectionTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/Tests/Transport/RedisExt/Fixtures/long_receiver.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/Tests/Transport/RedisExt/RedisExtIntegrationTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/Tests/Transport/RedisExt/RedisExtIntegrationTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/Tests/Transport/RedisExt/RedisExtIntegrationTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/Tests/Transport/RedisExt/RedisExtIntegrationTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/Tests/Transport/RedisExt/RedisExtIntegrationTest.php
Outdated
Show resolved
Hide resolved
62b159c
to
e0d5932
Compare
src/Symfony/Component/Messenger/Tests/Transport/RedisExt/ConnectionTest.php
Outdated
Show resolved
Hide resolved
781ae29
to
580fdb9
Compare
cba457c
to
52d3d68
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.
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)
src/Symfony/Component/Messenger/Transport/RedisExt/Connection.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/Transport/RedisExt/Connection.php
Outdated
Show resolved
Hide resolved
97af470
to
a4cceee
Compare
a4cceee
to
4461d36
Compare
@chalasr thank you for your review. did fix it and register the factory in the framework bundle. |
src/Symfony/Component/Messenger/Transport/RedisExt/RedisSender.php
Outdated
Show resolved
Hide resolved
Adding a test in symfony/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php Line 680 in a7d2019
|
d0b63af
to
4ed23bd
Compare
4ed23bd
to
ff0b855
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.
Tried this on a real project (will allow me to drop enqueue/messenger-adapter 🎉 ), works just fine.
I think implementing MessageCountAwareInterface
can be done later, not a blocker.
Thank you @alexander-schranz. |
…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#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: - 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 group The sender will still be simple by calling the `XADD` redis function. #EU-FOSSA Commits ------- ff0b855 Refractor redis transport using redis streams 7162d2e Implement redis transport
Nice work thanks @alexander-schranz ! |
@soyuka thank you for initial work! |
…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 transport This will add documentation how to configure and using the redis transport with the messenger component. symfony/symfony#30917 #EUFOSSA Commits ------- c22fade Add documentation for the Redis transport
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:
GET: Will use
XREADGROUP
to read the one message from the streamREJECT: Reject will just remove the message with
XDEL
from the stream as adding it back to the stream is handled by symfony worker itselfACK: 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-FOSSA