-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Messenger] Add support for multiple Redis Sentinel hosts #51687
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
[Messenger] Add support for multiple Redis Sentinel hosts #51687
Conversation
3e13640
to
578a794
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.
LGTM thanks, I just have one minor comment
src/Symfony/Component/Messenger/Bridge/Redis/Tests/Transport/RedisExtIntegrationTest.php
Outdated
Show resolved
Hide resolved
578a794
to
3380518
Compare
Thank you @digilist. |
@digilist there are some failures on our integration tests that might be related to this PR. Would you have some availability to have a look please? 🙏 |
Yes, sure. It looks like a merge conflict between #51683 and this PR was resolved incorrectly, as there are now two connection attempts to Sentinel: symfony/src/Symfony/Component/Cache/Traits/RedisTrait.php Lines 226 to 253 in 3c4cfbd
I'll provide a PR. |
This PR was merged into the 6.4 branch. Discussion ---------- [Cache] Fix two initializations of Redis Sentinel | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | n/a | License | MIT After #51687 and #51683 have been merged, there are now two initializations of Redis Sentinel, which also leads to the integration test failure mentioned in [this comment](#51687 (comment)). This PR fixes this. Commits ------- 2d23a1b [Cache] Fix two initializations of Redis Sentinel
Similar to #47003 which added support for multiple Redis Sentinel hosts for the Cache component, this PR adds support for multiple Sentinel hosts for the Messenger component.
This PR is inspired by the implementation in the cache component and works very similar. A DSN could look like this:
redis:?host[localhost:26377]&host[localhost:26379]&sentinel_master=db
.I changed the Sentinel host environment variable for the ingegration to an invalid host at. As a result I noticed that Relay also fails in such case and so I expanded my earlier changes from #51598 to also ignore unreachable hosts with the Relay extension.