From 89e662dc896b4550be98bdaa1acf56d56f9459cd Mon Sep 17 00:00:00 2001 From: Daniele Alessandri Date: Wed, 16 Jul 2014 18:11:17 +0200 Subject: [PATCH] Use same strategy for both client-side sharding and redis-cluster. This change is possible because, after a few changes in redis-cluster, our default cluster strategy used for client side sharding and the one used for redis-cluster turned out to be exactly the same, except for the hashing function used to calculate distribution. Actually some checks used to enforce correctness are redundant in the context of redis-cluster (e.g. the one used to make sure that keys in requests performing cross-keys operations will hash to the same slot, which is performed by the server) so we could also add a more dumb and permissive cluster strategy that relies on checks performed by Redis. Differently to v0.8, the strategy for client-side sharding now uses the same rules for extracting hash tags from keys especially when empty tags are found in the string. --- src/Cluster/PredisStrategy.php | 4 +- src/Cluster/RedisStrategy.php | 298 +----------------- tests/Predis/Cluster/RedisStrategyTest.php | 13 + .../Aggregate/PredisClusterTest.php | 25 ++ .../Connection/Aggregate/RedisClusterTest.php | 6 +- 5 files changed, 46 insertions(+), 300 deletions(-) diff --git a/src/Cluster/PredisStrategy.php b/src/Cluster/PredisStrategy.php index 21833307..e5fa5582 100644 --- a/src/Cluster/PredisStrategy.php +++ b/src/Cluster/PredisStrategy.php @@ -24,8 +24,8 @@ use Predis\Command\ScriptCommand; */ class PredisStrategy implements StrategyInterface { - private $commands; - private $hashGenerator; + protected $commands; + protected $hashGenerator; /** * @param HashGeneratorInterface $hashGenerator Hash generator instance. diff --git a/src/Cluster/RedisStrategy.php b/src/Cluster/RedisStrategy.php index 84e79d40..4a0d3e45 100644 --- a/src/Cluster/RedisStrategy.php +++ b/src/Cluster/RedisStrategy.php @@ -21,305 +21,13 @@ use Predis\Command\ScriptCommand; * * @author Daniele Alessandri */ -class RedisStrategy implements StrategyInterface +class RedisStrategy extends PredisStrategy { - private $commands; - private $hashGenerator; - /** * */ - public function __construct() + public function __construct(HashGeneratorInterface $hashGenerator = null) { - $this->commands = $this->getDefaultCommands(); - $this->hashGenerator = new Hash\CRC16(); - } - - /** - * Returns the default map of supported commands with their handlers. - * - * @return array - */ - protected function getDefaultCommands() - { - $getKeyFromFirstArgument = array($this, 'getKeyFromFirstArgument'); - $getKeyFromAllArguments = array($this, 'getKeyFromAllArguments'); - - return array( - /* commands operating on the key space */ - 'EXISTS' => $getKeyFromFirstArgument, - 'DEL' => $getKeyFromAllArguments, - 'TYPE' => $getKeyFromFirstArgument, - 'EXPIRE' => $getKeyFromFirstArgument, - 'EXPIREAT' => $getKeyFromFirstArgument, - 'PERSIST' => $getKeyFromFirstArgument, - 'PEXPIRE' => $getKeyFromFirstArgument, - 'PEXPIREAT' => $getKeyFromFirstArgument, - 'TTL' => $getKeyFromFirstArgument, - 'PTTL' => $getKeyFromFirstArgument, - 'SORT' => $getKeyFromFirstArgument, // TODO - - /* commands operating on string values */ - 'APPEND' => $getKeyFromFirstArgument, - 'DECR' => $getKeyFromFirstArgument, - 'DECRBY' => $getKeyFromFirstArgument, - 'GET' => $getKeyFromFirstArgument, - 'GETBIT' => $getKeyFromFirstArgument, - 'MGET' => $getKeyFromAllArguments, - 'SET' => $getKeyFromFirstArgument, - 'GETRANGE' => $getKeyFromFirstArgument, - 'GETSET' => $getKeyFromFirstArgument, - 'INCR' => $getKeyFromFirstArgument, - 'INCRBY' => $getKeyFromFirstArgument, - 'INCRBYFLOAT' => $getKeyFromFirstArgument, - 'SETBIT' => $getKeyFromFirstArgument, - 'SETEX' => $getKeyFromFirstArgument, - 'MSET' => array($this, 'getKeyFromInterleavedArguments'), - 'MSETNX' => array($this, 'getKeyFromInterleavedArguments'), - 'SETNX' => $getKeyFromFirstArgument, - 'SETRANGE' => $getKeyFromFirstArgument, - 'STRLEN' => $getKeyFromFirstArgument, - 'SUBSTR' => $getKeyFromFirstArgument, - 'BITCOUNT' => $getKeyFromFirstArgument, - - /* commands operating on lists */ - 'LINSERT' => $getKeyFromFirstArgument, - 'LINDEX' => $getKeyFromFirstArgument, - 'LLEN' => $getKeyFromFirstArgument, - 'LPOP' => $getKeyFromFirstArgument, - 'RPOP' => $getKeyFromFirstArgument, - 'BLPOP' => array($this, 'getKeyFromBlockingListCommands'), - 'BRPOP' => array($this, 'getKeyFromBlockingListCommands'), - 'LPUSH' => $getKeyFromFirstArgument, - 'LPUSHX' => $getKeyFromFirstArgument, - 'RPUSH' => $getKeyFromFirstArgument, - 'RPUSHX' => $getKeyFromFirstArgument, - 'LRANGE' => $getKeyFromFirstArgument, - 'LREM' => $getKeyFromFirstArgument, - 'LSET' => $getKeyFromFirstArgument, - 'LTRIM' => $getKeyFromFirstArgument, - - /* commands operating on sets */ - 'SADD' => $getKeyFromFirstArgument, - 'SCARD' => $getKeyFromFirstArgument, - 'SISMEMBER' => $getKeyFromFirstArgument, - 'SMEMBERS' => $getKeyFromFirstArgument, - 'SSCAN' => $getKeyFromFirstArgument, - 'SPOP' => $getKeyFromFirstArgument, - 'SRANDMEMBER' => $getKeyFromFirstArgument, - 'SREM' => $getKeyFromFirstArgument, - - /* commands operating on sorted sets */ - 'ZADD' => $getKeyFromFirstArgument, - 'ZCARD' => $getKeyFromFirstArgument, - 'ZCOUNT' => $getKeyFromFirstArgument, - 'ZINCRBY' => $getKeyFromFirstArgument, - 'ZRANGE' => $getKeyFromFirstArgument, - 'ZRANGEBYSCORE' => $getKeyFromFirstArgument, - 'ZRANK' => $getKeyFromFirstArgument, - 'ZREM' => $getKeyFromFirstArgument, - 'ZREMRANGEBYRANK' => $getKeyFromFirstArgument, - 'ZREMRANGEBYSCORE' => $getKeyFromFirstArgument, - 'ZREVRANGE' => $getKeyFromFirstArgument, - 'ZREVRANGEBYSCORE' => $getKeyFromFirstArgument, - 'ZREVRANK' => $getKeyFromFirstArgument, - 'ZSCORE' => $getKeyFromFirstArgument, - 'ZSCAN' => $getKeyFromFirstArgument, - 'ZLEXCOUNT' => $getKeyFromFirstArgument, - 'ZRANGEBYLEX' => $getKeyFromFirstArgument, - 'ZREMRANGEBYLEX' => $getKeyFromFirstArgument, - - /* commands operating on hashes */ - 'HDEL' => $getKeyFromFirstArgument, - 'HEXISTS' => $getKeyFromFirstArgument, - 'HGET' => $getKeyFromFirstArgument, - 'HGETALL' => $getKeyFromFirstArgument, - 'HMGET' => $getKeyFromFirstArgument, - 'HMSET' => $getKeyFromFirstArgument, - 'HINCRBY' => $getKeyFromFirstArgument, - 'HINCRBYFLOAT' => $getKeyFromFirstArgument, - 'HKEYS' => $getKeyFromFirstArgument, - 'HLEN' => $getKeyFromFirstArgument, - 'HSET' => $getKeyFromFirstArgument, - 'HSETNX' => $getKeyFromFirstArgument, - 'HVALS' => $getKeyFromFirstArgument, - 'HSCAN' => $getKeyFromFirstArgument, - - /* commands operating on HyperLogLog */ - 'PFADD' => $getKeyFromFirstArgument, - 'PFCOUNT' => $getKeyFromAllArguments, - 'PFMERGE' => $getKeyFromAllArguments, - - /* scripting */ - 'EVAL' => array($this, 'getKeyFromScriptingCommands'), - 'EVALSHA' => array($this, 'getKeyFromScriptingCommands'), - ); - } - - /** - * Returns the list of IDs for the supported commands. - * - * @return array - */ - public function getSupportedCommands() - { - return array_keys($this->commands); - } - - /** - * Sets an handler for the specified command ID. - * - * The signature of the callback must have a single parameter of type - * Predis\Command\CommandInterface. - * - * When the callback argument is omitted or NULL, the previously associated - * handler for the specified command ID is removed. - * - * @param string $commandID Command ID. - * @param mixed $callback A valid callable object, or NULL to unset the handler. - */ - public function setCommandHandler($commandID, $callback = null) - { - $commandID = strtoupper($commandID); - - if (!isset($callback)) { - unset($this->commands[$commandID]); - - return; - } - - if (!is_callable($callback)) { - throw new InvalidArgumentException( - "The argument must be a callable object or NULL." - ); - } - - $this->commands[$commandID] = $callback; - } - - /** - * Extracts the key from the first argument of a command instance. - * - * @param CommandInterface $command Command instance. - * @return string - */ - protected function getKeyFromFirstArgument(CommandInterface $command) - { - return $command->getArgument(0); - } - - /** - * Extracts the key from a command that can accept multiple keys ensuring - * that only one key is actually specified to comply with redis-cluster. - * - * @param CommandInterface $command Command instance. - * @return string - */ - protected function getKeyFromAllArguments(CommandInterface $command) - { - $arguments = $command->getArguments(); - - if (count($arguments) === 1) { - return $arguments[0]; - } - } - - /** - * Extracts the key from a command that can accept multiple keys ensuring - * that only one key is actually specified to comply with redis-cluster. - * - * @param CommandInterface $command Command instance. - * @return string - */ - protected function getKeyFromInterleavedArguments(CommandInterface $command) - { - $arguments = $command->getArguments(); - - if (count($arguments) === 2) { - return $arguments[0]; - } - } - - /** - * Extracts the key from BLPOP and BRPOP commands ensuring that only one key - * is actually specified to comply with redis-cluster. - * - * @param CommandInterface $command Command instance. - * @return string - */ - protected function getKeyFromBlockingListCommands(CommandInterface $command) - { - $arguments = $command->getArguments(); - - if (count($arguments) === 2) { - return $arguments[0]; - } - } - - /** - * Extracts the key from EVAL and EVALSHA commands. - * - * @param CommandInterface $command Command instance. - * @return string - */ - protected function getKeyFromScriptingCommands(CommandInterface $command) - { - if ($command instanceof ScriptCommand) { - $keys = $command->getKeys(); - } else { - $keys = array_slice($args = $command->getArguments(), 2, $args[1]); - } - - if (count($keys) === 1) { - return $keys[0]; - } - } - - /** - * {@inheritdoc} - */ - public function getHash(CommandInterface $command) - { - $hash = $command->getHash(); - - if (!isset($hash) && isset($this->commands[$cmdID = $command->getId()])) { - $key = call_user_func($this->commands[$cmdID], $command); - - if (isset($key)) { - $hash = $this->hashGenerator->hash($key); - $command->setHash($hash); - } - } - - return $hash; - } - - /** - * {@inheritdoc} - */ - public function getKeyHash($key) - { - $key = $this->extractKeyTag($key); - $hash = $this->hashGenerator->hash($key); - - return $hash; - } - - /** - * Returns only the hashable part of a key (delimited by "{...}"), or the - * whole key if a key tag is not found in the string. - * - * @param string $key A key. - * @return string - */ - protected function extractKeyTag($key) - { - if (false !== $start = strpos($key, '{')) { - if (false !== ($end = strpos($key, '}', $start)) && $end !== ++$start) { - $key = substr($key, $start, $end - $start); - } - } - - return $key; + parent::__construct($hashGenerator ?: new Hash\CRC16()); } } diff --git a/tests/Predis/Cluster/RedisStrategyTest.php b/tests/Predis/Cluster/RedisStrategyTest.php index 7035a59a..6190e4ee 100644 --- a/tests/Predis/Cluster/RedisStrategyTest.php +++ b/tests/Predis/Cluster/RedisStrategyTest.php @@ -275,6 +275,8 @@ class RedisStrategyTest extends PredisTestCase 'TTL' => 'keys-first', 'PTTL' => 'keys-first', 'SORT' => 'keys-first', // TODO + 'DUMP' => 'keys-first', + 'RESTORE' => 'keys-first', /* commands operating on string values */ 'APPEND' => 'keys-first', @@ -297,6 +299,7 @@ class RedisStrategyTest extends PredisTestCase 'SETRANGE' => 'keys-first', 'STRLEN' => 'keys-first', 'SUBSTR' => 'keys-first', + 'BITOP' => 'keys-bitop', 'BITCOUNT' => 'keys-first', /* commands operating on lists */ @@ -305,8 +308,10 @@ class RedisStrategyTest extends PredisTestCase 'LLEN' => 'keys-first', 'LPOP' => 'keys-first', 'RPOP' => 'keys-first', + 'RPOPLPUSH' => 'keys-all', 'BLPOP' => 'keys-blockinglist', 'BRPOP' => 'keys-blockinglist', + 'BRPOPLPUSH' => 'keys-blockinglist', 'LPUSH' => 'keys-first', 'LPUSHX' => 'keys-first', 'RPUSH' => 'keys-first', @@ -319,6 +324,12 @@ class RedisStrategyTest extends PredisTestCase /* commands operating on sets */ 'SADD' => 'keys-first', 'SCARD' => 'keys-first', + 'SDIFF' => 'keys-all', + 'SDIFFSTORE' => 'keys-all', + 'SINTER' => 'keys-all', + 'SINTERSTORE' => 'keys-all', + 'SUNION' => 'keys-all', + 'SUNIONSTORE' => 'keys-all', 'SISMEMBER' => 'keys-first', 'SMEMBERS' => 'keys-first', 'SSCAN' => 'keys-first', @@ -331,6 +342,7 @@ class RedisStrategyTest extends PredisTestCase 'ZCARD' => 'keys-first', 'ZCOUNT' => 'keys-first', 'ZINCRBY' => 'keys-first', + 'ZINTERSTORE' => 'keys-zaggregated', 'ZRANGE' => 'keys-first', 'ZRANGEBYSCORE' => 'keys-first', 'ZRANK' => 'keys-first', @@ -341,6 +353,7 @@ class RedisStrategyTest extends PredisTestCase 'ZREVRANGEBYSCORE' => 'keys-first', 'ZREVRANK' => 'keys-first', 'ZSCORE' => 'keys-first', + 'ZUNIONSTORE' => 'keys-zaggregated', 'ZSCAN' => 'keys-first', 'ZLEXCOUNT' => 'keys-first', 'ZRANGEBYLEX' => 'keys-first', diff --git a/tests/Predis/Connection/Aggregate/PredisClusterTest.php b/tests/Predis/Connection/Aggregate/PredisClusterTest.php index 5d4db552..f1142992 100644 --- a/tests/Predis/Connection/Aggregate/PredisClusterTest.php +++ b/tests/Predis/Connection/Aggregate/PredisClusterTest.php @@ -269,6 +269,31 @@ class PredisClusterTest extends PredisTestCase $cluster->getConnection($ping); } + /** + * @group disconnected + */ + public function testSupportsKeyHashTags() + { + $profile = Profile\Factory::getDefault(); + + $connection1 = $this->getMockConnection('tcp://127.0.0.1:6379'); + $connection2 = $this->getMockConnection('tcp://127.0.0.1:6380'); + + $cluster = new RedisCluster(); + $cluster->add($connection1); + $cluster->add($connection2); + + $set = $profile->createCommand('set', array('{node:1001}:foo', 'foobar')); + $get = $profile->createCommand('get', array('{node:1001}:foo')); + $this->assertSame($connection1, $cluster->getConnection($set)); + $this->assertSame($connection1, $cluster->getConnection($get)); + + $set = $profile->createCommand('set', array('{node:1001}:bar', 'foobar')); + $get = $profile->createCommand('get', array('{node:1001}:bar')); + $this->assertSame($connection1, $cluster->getConnection($set)); + $this->assertSame($connection1, $cluster->getConnection($get)); + } + /** * @group disconnected */ diff --git a/tests/Predis/Connection/Aggregate/RedisClusterTest.php b/tests/Predis/Connection/Aggregate/RedisClusterTest.php index 8315e2d2..bf77ee3d 100644 --- a/tests/Predis/Connection/Aggregate/RedisClusterTest.php +++ b/tests/Predis/Connection/Aggregate/RedisClusterTest.php @@ -428,7 +428,7 @@ class RedisClusterTest extends PredisTestCase /** * @group disconnected */ - public function testDoesNotSupportKeyTags() + public function testSupportsKeyHashTags() { $profile = Profile\Factory::getDefault(); @@ -446,8 +446,8 @@ class RedisClusterTest extends PredisTestCase $set = $profile->createCommand('set', array('{node:1001}:bar', 'foobar')); $get = $profile->createCommand('get', array('{node:1001}:bar')); - $this->assertSame($connection2, $cluster->getConnection($set)); - $this->assertSame($connection2, $cluster->getConnection($get)); + $this->assertSame($connection1, $cluster->getConnection($set)); + $this->assertSame($connection1, $cluster->getConnection($get)); } /**