Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Cache] fix using multiple Redis Sentinel hosts when the first one is not resolvable#51598
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
nicolas-grekas merged 1 commit intosymfony:5.4fromdigilist:51570-fix-redis-sentinel-only-trying-first-hostSep 14, 2023
Merged
[Cache] fix using multiple Redis Sentinel hosts when the first one is not resolvable#51598
nicolas-grekas merged 1 commit intosymfony:5.4fromdigilist:51570-fix-redis-sentinel-only-trying-first-hostSep 14, 2023
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
1190150 to4ab66afComparebd789ca to193af22CompareUh oh!
There was an error while loading.Please reload this page.
193af22 to578a152CompareMember
nicolas-grekas commentedSep 14, 2023
Thank you@digilist. |
nicolas-grekas added a commit that referenced this pull requestSep 20, 2023
…sts (digilist)This PR was merged into the 6.4 branch.Discussion----------[Messenger] Add support for multiple Redis Sentinel hosts| Q | A| ------------- | ---| Branch? | 6.4| Bug fix? | no| New feature? | yes| Deprecations? | no| Tickets | n/a| License | MIT| Doc PR | n/aSimilar 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.Commits-------3380518 [Messenger] Add support for multiple Redis Sentinel hosts
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See ticket#51570 for details on this bugfix.
As mentioned in the ticket, I am not sure if it's wise to catch all exceptions or if it would be better to only check for specific ones. But since I cannot think about any reasons other than an unreachable host to raise any exception, I decided to catch all exceptions for now.