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.

digilist force-pushed themessenger-multiple-sentinel branch 2 times, most recently from3e13640 to578a794Compare

September 18, 2023 21:19

nicolas-grekas reviewed

Sep 19, 2023

View reviewed changes

Copy link

Member

nicolas-grekas left a 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 OutdatedShow resolvedHide resolved

[Messenger] Add support for multiple Redis Sentinel hosts

3380518

digilist force-pushed themessenger-multiple-sentinel branch from578a794 to3380518Compare

September 19, 2023 09:56

Copy link

Member

nicolas-grekas commentedSep 20, 2023

Thank you@digilist.

nicolas-grekas merged commitdbf4bda intosymfony:6.4

Sep 20, 2023

nicolas-grekas mentioned this pull request

Sep 20, 2023

[Messenger] Add support for multiple Redis Sentinel hostssymfony/symfony-docs#18912

Closed

nicolas-grekas added this to the6.4 milestone

Sep 25, 2023

nicolas-grekas added Feature Messenger labels

Sep 25, 2023

Copy link

Member

nicolas-grekas commentedSep 28, 2023•
edited
Loading

@digilist there aresome failures on our integration tests that might be related to this PR. Would you have some availability to have a look please? 🙏

Copy link

ContributorAuthor

digilist commentedSep 28, 2023

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 in3c4cfbd

	if (version_compare(phpversion('redis'),'6.0.0','>=') &&$isRedisExt) {
	$options = [
	'host' =>$host,
	'port' =>$port,
	'connectTimeout' =>$params['timeout'],
	'persistent' =>$params['persistent_id'],
	'retryInterval' =>$params['retry_interval'],
	'readTimeout' =>$params['read_timeout'],
	];

	if ($passAuth) {
	$options['auth'] =$params['auth'];
	}

	$sentinel =new \RedisSentinel($options);
	}else {
	$extra =$passAuth ? [$params['auth']] : [];

	$sentinel =new$sentinelClass($host,$port,$params['timeout'], (string)$params['persistent_id'],$params['retry_interval'],$params['read_timeout'], ...$extra);
	}

	try {
	$sentinel =new$sentinelClass($host,$port,$params['timeout'], (string)$params['persistent_id'],$params['retry_interval'],$params['read_timeout'], ...$extra);
	if ($address =$sentinel->getMasterAddrByName($params['redis_sentinel'])) {
	[$host,$port] =$address;
	}
	}catch (\RedisException\|\Relay\Exception$redisException) {
	}

I'll provide a PR.

digilist mentioned this pull request

Sep 28, 2023

[Cache] Fix two initializations of Redis Sentinel#51775

Merged

nicolas-grekas added a commit that referenced this pull request

Sep 28, 2023

bug#51775[Cache] Fix two initializations of Redis Sentinel (digilist)

1b0aed0

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       | MITAfter#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

This was referencedOct 21, 2023

Release v6.4.0-BETA1#52220

Merged

Release v7.0.0-BETA1#52224

Merged

Labels

Feature Messenger

3 participants

Movatterモバイル変換

Navigation Menu

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[Messenger] Add support for multiple Redis Sentinel hosts#51687

[Messenger] Add support for multiple Redis Sentinel hosts#51687

Uh oh!