Separate id and alias in predis cluster (client-side sharding).

This change does not affect distribution but simply separates the two
concepts of connection ID (ip:port pair) and alias (value set via the
"alias" connection parameter), the method getConnectionByAlias() has
been added to reflect this change.

The method getConnectionBySlot() has also been added.

CHANGELOG.md

@@ -36,6 +36,10 @@ v2.0.0 (201x-xx-xx)
   get a single connection from the pool by using its ID. It is also possible to
   retrive a connection by role using the method getConnectionByRole().
 
+- The concept of connection ID (ip:port pair) and connection alias (the `alias`
+  parameter) in `Predis\Connection\Cluster\PredisCluster` has been separated.
+  This change does not affect distribution and it is safe for existing clusters.
+
 - Client option classes now live in the `Predis\Configuration\Option` namespace.
 
 - Classes for Redis commands have been moved into the new `Predis\Command\Redis`

examples/custom_cluster_distributor.php

@@ -104,8 +104,8 @@ for ($i = 0; $i < 100; ++$i) {
     $client->get("key:$i");
 }
 
-$server1 = $client->getClientBy('id', 'first')->info();
-$server2 = $client->getClientBy('id', 'second')->info();
+$server1 = $client->getClientBy('alias', 'first')->info();
+$server2 = $client->getClientBy('alias', 'second')->info();
 
 if (isset($server1['Keyspace'], $server2['Keyspace'])) {
     $server1 = $server1['Keyspace'];

src/Connection/Cluster/PredisCluster.php

@@ -27,8 +27,24 @@ use Predis\NotSupportedException;
  */
 class PredisCluster implements ClusterInterface, \IteratorAggregate, \Countable
 {
-    private $pool;
+    /**
+     * @var NodeConnectionInterface[]
+     */
+    private $pool = array();
+
+    /**
+     * @var NodeConnectionInterface[]
+     */
+    private $aliases = array();
+
+    /**
+     * @var StrategyInterface
+     */
     private $strategy;
+
+    /**
+     * @var Predis\Cluster\Distributor\DistributorInterface
+     */
     private $distributor;
 
     /**
@@ -36,7 +52,6 @@ class PredisCluster implements ClusterInterface, \IteratorAggregate, \Countable
      */
     public function __construct(StrategyInterface $strategy = null)
     {
-        $this->pool = array();
         $this->strategy = $strategy ?: new PredisStrategy();
         $this->distributor = $this->strategy->getDistributor();
     }
@@ -82,14 +97,13 @@ class PredisCluster implements ClusterInterface, \IteratorAggregate, \Countable
     {
         $parameters = $connection->getParameters();
 
+        $this->pool[(string) $connection] = $connection;
+
         if (isset($parameters->alias)) {
-            $this->pool[$parameters->alias] = $connection;
-        } else {
-            $this->pool[] = $connection;
+            $this->aliases[$parameters->alias] = $connection;
         }
 
-        $weight = isset($parameters->weight) ? $parameters->weight : null;
-        $this->distributor->add($connection, $weight);
+        $this->distributor->add($connection, $parameters->weight);
     }
 
     /**
@@ -97,27 +111,15 @@ class PredisCluster implements ClusterInterface, \IteratorAggregate, \Countable
      */
     public function remove(NodeConnectionInterface $connection)
     {
-        if (($id = array_search($connection, $this->pool, true)) !== false) {
+        if (false !== $id = array_search($connection, $this->pool, true)) {
             unset($this->pool[$id]);
             $this->distributor->remove($connection);
 
-            return true;
-        }
-
-        return false;
-    }
+            if ($this->aliases && $alias = $connection->getParameters()->alias) {
+                unset($this->aliases[$alias]);
+            }
 
-    /**
-     * Removes a connection instance using its alias or index.
-     *
-     * @param string $connectionID Alias or index of a connection.
-     *
-     * @return bool Returns true if the connection was in the pool.
-     */
-    public function removeById($connectionID)
-    {
-        if ($connection = $this->getConnectionById($connectionID)) {
-            return $this->remove($connection);
+            return true;
         }
 
         return false;
@@ -144,9 +146,37 @@ class PredisCluster implements ClusterInterface, \IteratorAggregate, \Countable
     /**
      * {@inheritdoc}
      */
-    public function getConnectionById($connectionID)
+    public function getConnectionById($id)
+    {
+        if (isset($this->pool[$id])) {
+            return $this->pool[$id];
+        }
+    }
+
+    /**
+     * Returns a connection instance by its alias.
+     *
+     * @param string $alias Connection alias.
+     *
+     * @return NodeConnectionInterface|null
+     */
+    public function getConnectionByAlias($alias)
+    {
+        if (isset($this->aliases[$alias])) {
+            return $this->aliases[$alias];
+        }
+    }
+
+    /**
+     * Retrieves a connection instance by slot.
+     *
+     * @param string $key Key string.
+     *
+     * @return NodeConnectionInterface|null
+     */
+    public function getConnectionBySlot($slot)
     {
-        return isset($this->pool[$connectionID]) ? $this->pool[$connectionID] : null;
+        return $this->distributor->getBySlot($slot);
     }
 
     /**

tests/Predis/ClientTest.php

@@ -690,7 +690,7 @@ class ClientTest extends PredisTestCase
         $nodes = array('tcp://host1?alias=node01', 'tcp://host2?alias=node02');
         $client = $this->getMock('Predis\Client', array('dummy'), array($nodes, array('cluster' => 'predis')), 'SubclassedClient');
 
-        $this->assertInstanceOf('SubclassedClient', $client->getClientBy('id', 'node02'));
+        $this->assertInstanceOf('SubclassedClient', $client->getClientBy('alias', 'node02'));
     }
 
     /**
@@ -715,7 +715,7 @@ class ClientTest extends PredisTestCase
             }))
             ->will($this->returnValue('value'));
 
-        $this->assertSame('value', $client->getClientBy('id', 'node02', $callable));
+        $this->assertSame('value', $client->getClientBy('alias', 'node02', $callable));
     }
 
     /**

tests/Predis/Connection/Cluster/PredisClusterTest.php

@@ -32,8 +32,8 @@ class PredisClusterTest extends PredisTestCase
      */
     public function testAddingConnectionsToCluster()
     {
-        $connection1 = $this->getMockConnection();
-        $connection2 = $this->getMockConnection();
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
 
         $cluster = new PredisCluster();
 
@@ -41,17 +41,17 @@ class PredisClusterTest extends PredisTestCase
         $cluster->add($connection2);
 
         $this->assertSame(2, count($cluster));
-        $this->assertSame($connection1, $cluster->getConnectionById(0));
-        $this->assertSame($connection2, $cluster->getConnectionById(1));
+        $this->assertSame($connection1, $cluster->getConnectionById('127.0.0.1:7001'));
+        $this->assertSame($connection2, $cluster->getConnectionById('127.0.0.1:7002'));
     }
 
     /**
      * @group disconnected
      */
-    public function testAddingConnectionsToClusterUsesConnectionAlias()
+    public function testAddingConnectionsWithAliasParameterToCluster()
     {
-        $connection1 = $this->getMockConnection('tcp://host1:7001?alias=node1');
-        $connection2 = $this->getMockConnection('tcp://host1:7002?alias=node2');
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001?alias=node01');
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002?alias=node02');
 
         $cluster = new PredisCluster();
 
@@ -59,8 +59,8 @@ class PredisClusterTest extends PredisTestCase
         $cluster->add($connection2);
 
         $this->assertSame(2, count($cluster));
-        $this->assertSame($connection1, $cluster->getConnectionById('node1'));
-        $this->assertSame($connection2, $cluster->getConnectionById('node2'));
+        $this->assertSame($connection1, $cluster->getConnectionByAlias('node01'));
+        $this->assertSame($connection2, $cluster->getConnectionByAlias('node02'));
     }
 
     /**
@@ -68,9 +68,9 @@ class PredisClusterTest extends PredisTestCase
      */
     public function testRemovingConnectionsFromCluster()
     {
-        $connection1 = $this->getMockConnection();
-        $connection2 = $this->getMockConnection();
-        $connection3 = $this->getMockConnection();
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001?alias=node01');
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
+        $connection3 = $this->getMockConnection('tcp://127.0.0.1:7003');
 
         $cluster = new PredisCluster();
 
@@ -78,29 +78,11 @@ class PredisClusterTest extends PredisTestCase
         $cluster->add($connection2);
 
         $this->assertTrue($cluster->remove($connection1));
-        $this->assertFalse($cluster->remove($connection3));
-        $this->assertSame(1, count($cluster));
-    }
-
-    /**
-     * @group disconnected
-     */
-    public function testRemovingConnectionsFromClusterByAlias()
-    {
-        $connection1 = $this->getMockConnection();
-        $connection2 = $this->getMockConnection('tcp://host1:7001?alias=node2');
-        $connection3 = $this->getMockConnection('tcp://host1:7002?alias=node3');
-
-        $cluster = new PredisCluster();
+        $this->assertNull($cluster->getConnectionByAlias('node02'));
 
-        $cluster->add($connection1);
-        $cluster->add($connection2);
-        $cluster->add($connection3);
+        $this->assertFalse($cluster->remove($connection3));
 
-        $this->assertTrue($cluster->removeById(0));
-        $this->assertTrue($cluster->removeById('node2'));
-        $this->assertFalse($cluster->removeById('node4'));
-        $this->assertSame(1, count($cluster));
+        $this->assertCount(1, $cluster);
     }
 
     /**
@@ -108,12 +90,12 @@ class PredisClusterTest extends PredisTestCase
      */
     public function testConnectForcesAllConnectionsToConnect()
     {
-        $connection1 = $this->getMockConnection();
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
         $connection1
             ->expects($this->once())
             ->method('connect');
 
-        $connection2 = $this->getMockConnection();
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
         $connection2
             ->expects($this->once())
             ->method('connect');
@@ -131,12 +113,12 @@ class PredisClusterTest extends PredisTestCase
      */
     public function testDisconnectForcesAllConnectionsToDisconnect()
     {
-        $connection1 = $this->getMockConnection();
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
         $connection1
             ->expects($this->once())
             ->method('disconnect');
 
-        $connection2 = $this->getMockConnection();
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
         $connection2
             ->expects($this->once())
             ->method('disconnect');
@@ -154,13 +136,13 @@ class PredisClusterTest extends PredisTestCase
      */
     public function testIsConnectedReturnsTrueIfAtLeastOneConnectionIsOpen()
     {
-        $connection1 = $this->getMockConnection();
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
         $connection1
             ->expects($this->once())
             ->method('isConnected')
             ->will($this->returnValue(false));
 
-        $connection2 = $this->getMockConnection();
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
         $connection2
             ->expects($this->once())
             ->method('isConnected')
@@ -179,13 +161,13 @@ class PredisClusterTest extends PredisTestCase
      */
     public function testIsConnectedReturnsFalseIfAllConnectionsAreClosed()
     {
-        $connection1 = $this->getMockConnection();
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
         $connection1
             ->expects($this->once())
             ->method('isConnected')
             ->will($this->returnValue(false));
 
-        $connection2 = $this->getMockConnection();
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
         $connection2
             ->expects($this->once())
             ->method('isConnected')
@@ -204,8 +186,8 @@ class PredisClusterTest extends PredisTestCase
      */
     public function testCanReturnAnIteratorForConnections()
     {
-        $connection1 = $this->getMockConnection();
-        $connection2 = $this->getMockConnection();
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
 
         $cluster = new PredisCluster();
 
@@ -215,8 +197,27 @@ class PredisClusterTest extends PredisTestCase
         $this->assertInstanceOf('Iterator', $iterator = $cluster->getIterator());
         $connections = iterator_to_array($iterator);
 
-        $this->assertSame($connection1, $connections[0]);
-        $this->assertSame($connection2, $connections[1]);
+        $this->assertSame(array(
+            '127.0.0.1:7001' => $connection1,
+            '127.0.0.1:7002' => $connection2,
+        ), iterator_to_array($iterator));
+    }
+
+    /**
+     * @group disconnected
+     */
+    public function testReturnsCorrectConnectionUsingSlot()
+    {
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
+
+        $cluster = new PredisCluster();
+
+        $cluster->add($connection1);
+        $cluster->add($connection2);
+
+        $this->assertSame($connection1, $cluster->getConnectionBySlot(1839357934));
+        $this->assertSame($connection2, $cluster->getConnectionBySlot(2146453549));
     }
 
     /**
@@ -224,8 +225,8 @@ class PredisClusterTest extends PredisTestCase
      */
     public function testReturnsCorrectConnectionUsingKey()
     {
-        $connection1 = $this->getMockConnection('tcp://host1:7001');
-        $connection2 = $this->getMockConnection('tcp://host1:7002');
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
 
         $cluster = new PredisCluster();
 
@@ -245,8 +246,8 @@ class PredisClusterTest extends PredisTestCase
     {
         $commands = $this->getCommandFactory();
 
-        $connection1 = $this->getMockConnection('tcp://host1:7001');
-        $connection2 = $this->getMockConnection('tcp://host1:7002');
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
 
         $cluster = new PredisCluster();
 
@@ -285,7 +286,7 @@ class PredisClusterTest extends PredisTestCase
 
         $cluster = new PredisCluster();
 
-        $cluster->add($this->getMockConnection());
+        $cluster->add($this->getMockConnection('tcp://127.0.0.1:6379'));
 
         $cluster->getConnectionByCommand($ping);
     }
@@ -323,13 +324,13 @@ class PredisClusterTest extends PredisTestCase
     {
         $command = $this->getCommandFactory()->createCommand('get', array('node01:5431'));
 
-        $connection1 = $this->getMockConnection('tcp://host1:7001');
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
         $connection1
             ->expects($this->once())
             ->method('writeRequest')
             ->with($command);
 
-        $connection2 = $this->getMockConnection('tcp://host1:7002');
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
         $connection2
             ->expects($this->never())
             ->method('writeRequest');
@@ -349,12 +350,12 @@ class PredisClusterTest extends PredisTestCase
     {
         $command = $this->getCommandFactory()->createCommand('get', array('node02:3212'));
 
-        $connection1 = $this->getMockConnection('tcp://host1:7001');
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
         $connection1
             ->expects($this->never())
             ->method('readResponse');
 
-        $connection2 = $this->getMockConnection('tcp://host1:7002');
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
         $connection2
             ->expects($this->once())
             ->method('readResponse')
@@ -375,13 +376,13 @@ class PredisClusterTest extends PredisTestCase
     {
         $command = $this->getCommandFactory()->createCommand('get', array('node01:5431'));
 
-        $connection1 = $this->getMockConnection('tcp://host1:7001');
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
         $connection1
             ->expects($this->once())
             ->method('executeCommand')
             ->with($command);
 
-        $connection2 = $this->getMockConnection('tcp://host1:7002');
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
         $connection2
             ->expects($this->never())
             ->method('executeCommand');
@@ -399,8 +400,8 @@ class PredisClusterTest extends PredisTestCase
      */
     public function testCanBeSerialized()
     {
-        $connection1 = $this->getMockConnection('tcp://host1?alias=first');
-        $connection2 = $this->getMockConnection('tcp://host2?alias=second');
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001?alias=first');
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002?alias=second');
 
         $cluster = new PredisCluster();