Use SENTINEL REPLICAS for Sentinel replica discovery

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)

src/Connection/Replication/SentinelReplication.php

@@ -423,9 +423,16 @@ class SentinelReplication extends AbstractAggregateConnection implements Replica
     {
         $slaves = [];
 
+        $payload = $sentinel->executeCommand(
+            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);

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

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(
                 [