diff --git a/CHANGELOG.md b/CHANGELOG.md index 44d4a4a..2548e87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ ### Added - Added support for numbered databases in Valkey clusters (#1696) +### Changed +- Use `SENTINEL REPLICAS` for Sentinel replica discovery, falling back to `SENTINEL SLAVES` on Redis < 5.0 (#776) + ### Fixed - Fixed interrupted pipelines leaving unread replies on reused connections; failed pipelines now discard remaining commands and reset execution state (#1309) - Fixed Relay Pub/Sub subscriptions rejecting message callbacks and failing when the `prefix` option is set (#1292) diff --git a/src/Connection/Replication/SentinelReplication.php b/src/Connection/Replication/SentinelReplication.php index 0ae7af6..6d5248e 100644 --- a/src/Connection/Replication/SentinelReplication.php +++ b/src/Connection/Replication/SentinelReplication.php @@ -424,9 +424,16 @@ class SentinelReplication extends AbstractAggregateConnection implements Replica $slaves = []; $payload = $sentinel->executeCommand( - RawCommand::create('SENTINEL', 'slaves', $service) + RawCommand::create('SENTINEL', 'replicas', $service) ); + // NOTE: SENTINEL REPLICAS is available since Redis 5.0, older sentinels only know SENTINEL SLAVES. + if ($payload instanceof ErrorResponseInterface && stripos($payload->getMessage(), 'unknown sentinel subcommand') !== false) { + $payload = $sentinel->executeCommand( + RawCommand::create('SENTINEL', 'slaves', $service) + ); + } + if ($payload instanceof ErrorResponseInterface) { $this->handleSentinelErrorResponse($sentinel, $payload); } diff --git a/tests/Predis/ClientTest.php b/tests/Predis/ClientTest.php index 024fe05..ef1de09 100644 --- a/tests/Predis/ClientTest.php +++ b/tests/Predis/ClientTest.php @@ -1729,7 +1729,7 @@ class ClientTest extends PredisTestCase $this->assertInstanceOf(SentinelReplication::class, $connection); - // SENTINEL slaves returns a map under RESP3 - calling getSlaves() + // SENTINEL replicas returns a map under RESP3 - calling getSlaves() // exercises the parsing path that previously read fields by // positional index. We do not assert on cardinality because the // CI topology may not include replicas; we only require that the diff --git a/tests/Predis/Connection/Replication/SentinelReplicationTest.php b/tests/Predis/Connection/Replication/SentinelReplicationTest.php index 159b2ae..0c0d156 100644 --- a/tests/Predis/Connection/Replication/SentinelReplicationTest.php +++ b/tests/Predis/Connection/Replication/SentinelReplicationTest.php @@ -298,13 +298,13 @@ class SentinelReplicationTest extends PredisTestCase ->method('executeCommand') ->withConsecutive( [$this->isRedisCommand('SENTINEL', ['get-master-addr-by-name', 'svc'])], - [$this->isRedisCommand('SENTINEL', ['slaves', 'svc'])] + [$this->isRedisCommand('SENTINEL', ['replicas', 'svc'])] ) ->willReturnOnConsecutiveCalls( // SENTINEL get-master-addr-by-name svc ['127.0.0.1', '6381'], - // SENTINEL slaves svc + // SENTINEL replicas svc [ [ 'name', '127.0.0.1:6382', @@ -477,7 +477,7 @@ class SentinelReplicationTest extends PredisTestCase ->withConsecutive( [$this->isRedisCommand('SENTINEL', ['sentinels', 'svc'])], [$this->isRedisCommand('SENTINEL', ['get-master-addr-by-name', 'svc'])], - [$this->isRedisCommand('SENTINEL', ['slaves', 'svc'])] + [$this->isRedisCommand('SENTINEL', ['replicas', 'svc'])] ) ->willReturnOnConsecutiveCalls( // SENTINEL sentinels svc @@ -494,7 +494,7 @@ class SentinelReplicationTest extends PredisTestCase // SENTINEL get-master-addr-by-name svc ['127.0.0.1', '6381'], - // SENTINEL slaves svc + // SENTINEL replicas svc [ [ 'name', '127.0.0.1:6382', @@ -603,7 +603,7 @@ class SentinelReplicationTest extends PredisTestCase ->expects($this->once()) ->method('executeCommand') ->withConsecutive( - [$this->isRedisCommand('SENTINEL', ['slaves', 'svc'])] + [$this->isRedisCommand('SENTINEL', ['replicas', 'svc'])] ) ->willReturnOnConsecutiveCalls( [ @@ -646,7 +646,7 @@ class SentinelReplicationTest extends PredisTestCase ->expects($this->once()) ->method('executeCommand') ->withConsecutive( - [$this->isRedisCommand('SENTINEL', ['slaves', 'svc'])] + [$this->isRedisCommand('SENTINEL', ['replicas', 'svc'])] ) ->willReturnOnConsecutiveCalls( [ @@ -691,6 +691,69 @@ class SentinelReplicationTest extends PredisTestCase $this->assertSame('127.0.0.1:6382', (string) $slaves[0]); } + /** + * @group disconnected + */ + public function testMethodGetSlavesFallsBackToSentinelSlavesWhenReplicasIsUnknown(): void + { + $sentinel1 = $this->getMockSentinelConnection('tcp://127.0.0.1:5381?role=sentinel'); + $sentinel1 + ->expects($this->exactly(2)) + ->method('executeCommand') + ->withConsecutive( + [$this->isRedisCommand('SENTINEL', ['replicas', 'svc'])], + [$this->isRedisCommand('SENTINEL', ['slaves', 'svc'])] + ) + ->willReturnOnConsecutiveCalls( + // SENTINEL replicas svc (Redis < 5.0) + new Response\Error("ERR Unknown sentinel subcommand 'replicas'"), + + // SENTINEL slaves svc + [ + [ + 'name', '127.0.0.1:6382', + 'ip', '127.0.0.1', + 'port', '6382', + 'runid', '112cdebd22924a7d962be496f3a1c4c7c9bad93f', + 'flags', 'slave', + 'master-host', '127.0.0.1', + 'master-port', '6381', + ], + ] + ); + + $replication = $this->getReplicationConnection('svc', [$sentinel1]); + + $slaves = $replication->getSlaves(); + + $this->assertCount(1, $slaves); + $this->assertSame('127.0.0.1:6382', (string) $slaves[0]); + } + + /** + * @group disconnected + */ + public function testMethodGetSlavesDoesNotFallBackToSentinelSlavesOnOtherErrors(): void + { + $this->expectException('Predis\Response\ServerException'); + $this->expectExceptionMessage('ERR No such master with that name'); + + $sentinel1 = $this->getMockSentinelConnection('tcp://127.0.0.1:5381?role=sentinel'); + $sentinel1 + ->expects($this->once()) + ->method('executeCommand') + ->with($this->isRedisCommand( + 'SENTINEL', ['replicas', 'svc'] + )) + ->willReturn( + new Response\Error('ERR No such master with that name') + ); + + $replication = $this->getReplicationConnection('svc', [$sentinel1]); + + $replication->getSlaves(); + } + /** * @group disconnected */ @@ -704,7 +767,7 @@ class SentinelReplicationTest extends PredisTestCase ->expects($this->any()) ->method('executeCommand') ->with($this->isRedisCommand( - 'SENTINEL', ['slaves', 'svc'] + 'SENTINEL', ['replicas', 'svc'] )) ->willThrowException( new Connection\ConnectionException($sentinel1, 'Unknown connection error [127.0.0.1:5381]') @@ -762,7 +825,7 @@ class SentinelReplicationTest extends PredisTestCase ->expects($this->any()) ->method('executeCommand') ->with($this->isRedisCommand( - 'SENTINEL', ['slaves', 'svc'] + 'SENTINEL', ['replicas', 'svc'] )) ->willReturn( [ @@ -817,7 +880,7 @@ class SentinelReplicationTest extends PredisTestCase ->expects($this->exactly(2)) ->method('executeCommand') ->withConsecutive( - [$this->isRedisCommand('SENTINEL', ['slaves', 'svc'])], + [$this->isRedisCommand('SENTINEL', ['replicas', 'svc'])], [$this->isRedisCommand('SENTINEL', ['get-master-addr-by-name', 'svc'])] ) ->willReturnOnConsecutiveCalls( @@ -1209,7 +1272,7 @@ class SentinelReplicationTest extends PredisTestCase ->expects($this->once()) ->method('executeCommand') ->withConsecutive( - [$this->isRedisCommand('SENTINEL', ['slaves', 'svc'])] + [$this->isRedisCommand('SENTINEL', ['replicas', 'svc'])] ) ->willReturnOnConsecutiveCalls( [ @@ -1309,7 +1372,7 @@ class SentinelReplicationTest extends PredisTestCase ->expects($this->any()) ->method('executeCommand') ->with($this->isRedisCommand( - 'SENTINEL', ['slaves', 'svc'] + 'SENTINEL', ['replicas', 'svc'] )) ->willReturn( [ @@ -1398,7 +1461,7 @@ class SentinelReplicationTest extends PredisTestCase ->expects($this->any()) ->method('executeCommand') ->with($this->isRedisCommand( - 'SENTINEL', ['slaves', 'svc'] + 'SENTINEL', ['replicas', 'svc'] )) ->willReturn( [ @@ -1923,7 +1986,7 @@ class SentinelReplicationTest extends PredisTestCase ->withConsecutive( [$this->isRedisCommand('SENTINEL', ['sentinels', 'srv'])], [$this->isRedisCommand('SENTINEL', ['get-master-addr-by-name', 'srv'])], - [$this->isRedisCommand('SENTINEL', ['slaves', 'srv'])] + [$this->isRedisCommand('SENTINEL', ['replicas', 'srv'])] ) ->willReturnOnConsecutiveCalls( // SENTINEL sentinels srv @@ -1938,7 +2001,7 @@ class SentinelReplicationTest extends PredisTestCase ], // SENTINEL get-master-addr-by-name srv ['127.0.0.1', '6381'], - // SENTINEL slaves srv + // SENTINEL replicas srv [] ); @@ -2159,7 +2222,7 @@ class SentinelReplicationTest extends PredisTestCase ->expects($this->any()) ->method('executeCommand') ->with($this->isRedisCommand( - 'SENTINEL', ['slaves', 'svc'] + 'SENTINEL', ['replicas', 'svc'] )) ->willReturn( [