From e80600e244b8442cb7c86e99b067966cd59bf2ee Mon Sep 17 00:00:00 2001 From: Pavlo Yatsukhnenko Date: Tue, 19 May 2020 04:11:40 +0300 Subject: Issue #548 (#1649) Adds `Redis::SCAN_PREFIX` and `Redis::SCAN_NOPREFIX` as options to SCAN. See #548 --- README.markdown | 5 +++++ common.h | 4 +++- library.c | 4 ++-- redis.c | 13 ++++++++++-- redis_cluster.c | 21 ++++++++++++++++---- redis_commands.c | 13 ++++++++---- tests/RedisClusterTest.php | 49 ++++++++++++++++++++++++++++++++++++++++++++++ tests/RedisTest.php | 34 ++++++++++++++++++++++++++++++++ 8 files changed, 130 insertions(+), 13 deletions(-) diff --git a/README.markdown b/README.markdown index 9edc3dca..1b81f778 100644 --- a/README.markdown +++ b/README.markdown @@ -354,6 +354,11 @@ $redis->setOption(Redis::OPT_PREFIX, 'myAppName:'); // use custom prefix on all */ $redis->setOption(Redis::OPT_SCAN, Redis::SCAN_NORETRY); $redis->setOption(Redis::OPT_SCAN, Redis::SCAN_RETRY); + +/* Scan can also be configured to automatically prepend the currently set PhpRedis + prefix to any MATCH pattern. */ +$redis->setOption(Redis::OPT_SCAN, Redis::SCAN_PREFIX); +$redis->setOption(Redis::OPT_SCAN, Redis::SCAN_NOPREFIX); ~~~ diff --git a/common.h b/common.h index e70a8ca0..1926d641 100644 --- a/common.h +++ b/common.h @@ -103,7 +103,9 @@ typedef enum { /* SCAN options */ #define REDIS_SCAN_NORETRY 0 -#define REDIS_SCAN_RETRY 1 +#define REDIS_SCAN_RETRY 1 +#define REDIS_SCAN_PREFIX 2 +#define REDIS_SCAN_NOPREFIX 3 /* GETBIT/SETBIT offset range limits */ #define BITOP_MIN_OFFSET 0 diff --git a/library.c b/library.c index 565e1cb3..a67f79bb 100644 --- a/library.c +++ b/library.c @@ -722,7 +722,7 @@ int redis_cmd_append_sstr_dbl(smart_string *str, double value) { char tmp[64]; - int len, retval; + int len; /* Convert to string */ len = snprintf(tmp, sizeof(tmp), "%.16g", value); @@ -1785,7 +1785,7 @@ redis_sock_create(char *host, int host_len, int port, redis_sock->err = NULL; - redis_sock->scan = REDIS_SCAN_NORETRY; + redis_sock->scan = 0; redis_sock->readonly = 0; redis_sock->tcp_keepalive = 0; diff --git a/redis.c b/redis.c index 729bcf9b..6c1a7642 100644 --- a/redis.c +++ b/redis.c @@ -723,6 +723,8 @@ static void add_class_constants(zend_class_entry *ce, int is_cluster) { zend_declare_class_constant_long(ce, ZEND_STRL("OPT_SCAN"), REDIS_OPT_SCAN); zend_declare_class_constant_long(ce, ZEND_STRL("SCAN_RETRY"), REDIS_SCAN_RETRY); zend_declare_class_constant_long(ce, ZEND_STRL("SCAN_NORETRY"), REDIS_SCAN_NORETRY); + zend_declare_class_constant_long(ce, ZEND_STRL("SCAN_PREFIX"), REDIS_SCAN_PREFIX); + zend_declare_class_constant_long(ce, ZEND_STRL("SCAN_NOPREFIX"), REDIS_SCAN_NOPREFIX); /* Cluster option to allow for slave failover */ if (is_cluster) { @@ -3462,7 +3464,7 @@ generic_scan_cmd(INTERNAL_FUNCTION_PARAMETERS, REDIS_SCAN_TYPE type) { RedisSock *redis_sock; HashTable *hash; char *pattern = NULL, *cmd, *key = NULL; - int cmd_len, num_elements, key_free = 0; + int cmd_len, num_elements, key_free = 0, pattern_free = 0; size_t key_len = 0, pattern_len = 0; zend_string *match_type = NULL; zend_long count = 0, iter; @@ -3520,6 +3522,10 @@ generic_scan_cmd(INTERNAL_FUNCTION_PARAMETERS, REDIS_SCAN_TYPE type) { key_free = redis_key_prefix(redis_sock, &key, &key_len); } + if (redis_sock->scan & REDIS_SCAN_PREFIX) { + pattern_free = redis_key_prefix(redis_sock, &pattern, &pattern_len); + } + /** * Redis can return to us empty keys, especially in the case where there * are a large number of keys to scan, and we're matching against a @@ -3552,9 +3558,12 @@ generic_scan_cmd(INTERNAL_FUNCTION_PARAMETERS, REDIS_SCAN_TYPE type) { /* Get the number of elements */ hash = Z_ARRVAL_P(return_value); num_elements = zend_hash_num_elements(hash); - } while(redis_sock->scan == REDIS_SCAN_RETRY && iter != 0 && + } while (redis_sock->scan & REDIS_SCAN_RETRY && iter != 0 && num_elements == 0); + /* Free our pattern if it was prefixed */ + if (pattern_free) efree(pattern); + /* Free our key if it was prefixed */ if(key_free) efree(key); diff --git a/redis_cluster.c b/redis_cluster.c index 7f79b5fe..0fc94fe3 100644 --- a/redis_cluster.c +++ b/redis_cluster.c @@ -2411,7 +2411,7 @@ static void cluster_kscan_cmd(INTERNAL_FUNCTION_PARAMETERS, { redisCluster *c = GET_CONTEXT(); char *cmd, *pat = NULL, *key = NULL; - size_t key_len = 0, pat_len = 0; + size_t key_len = 0, pat_len = 0, pat_free = 0; int cmd_len, key_free = 0; short slot; zval *z_it; @@ -2450,6 +2450,10 @@ static void cluster_kscan_cmd(INTERNAL_FUNCTION_PARAMETERS, key_free = redis_key_prefix(c->flags, &key, &key_len); slot = cluster_hash_key(key, key_len); + if (c->flags->scan & REDIS_SCAN_PREFIX) { + pat_free = redis_key_prefix(c->flags, &pat, &pat_len); + } + // If SCAN_RETRY is set, loop until we get a zero iterator or until // we get non-zero elements. Otherwise we just send the command once. do { @@ -2488,7 +2492,10 @@ static void cluster_kscan_cmd(INTERNAL_FUNCTION_PARAMETERS, // Free our command efree(cmd); - } while (c->flags->scan == REDIS_SCAN_RETRY && it != 0 && num_ele == 0); + } while (c->flags->scan & REDIS_SCAN_RETRY && it != 0 && num_ele == 0); + + // Free our pattern + if (pat_free) efree(pat); // Free our key if (key_free) efree(key); @@ -2505,7 +2512,7 @@ PHP_METHOD(RedisCluster, scan) { int cmd_len; short slot; zval *z_it, *z_node; - long it, num_ele; + long it, num_ele, pat_free = 0; zend_long count = 0; /* Treat as read-only */ @@ -2534,6 +2541,10 @@ PHP_METHOD(RedisCluster, scan) { RETURN_FALSE; } + if (c->flags->scan & REDIS_SCAN_PREFIX) { + pat_free = redis_key_prefix(c->flags, &pat, &pat_len); + } + /* With SCAN_RETRY on, loop until we get some keys, otherwise just return * what Redis does, as it does */ do { @@ -2570,7 +2581,9 @@ PHP_METHOD(RedisCluster, scan) { efree(cmd); num_ele = zend_hash_num_elements(Z_ARRVAL_P(return_value)); - } while (c->flags->scan == REDIS_SCAN_RETRY && it != 0 && num_ele == 0); + } while (c->flags->scan & REDIS_SCAN_RETRY && it != 0 && num_ele == 0); + + if (pat_free) efree(pat); Z_LVAL_P(z_it) = it; } diff --git a/redis_commands.c b/redis_commands.c index 6945ed4b..e7cca1a8 100644 --- a/redis_commands.c +++ b/redis_commands.c @@ -4032,11 +4032,16 @@ void redis_setoption_handler(INTERNAL_FUNCTION_PARAMETERS, RETURN_TRUE; case REDIS_OPT_SCAN: val_long = zval_get_long(val); - if (val_long==REDIS_SCAN_NORETRY || val_long==REDIS_SCAN_RETRY) { - redis_sock->scan = val_long; - RETURN_TRUE; + if (val_long == REDIS_SCAN_NORETRY) { + redis_sock->scan &= ~REDIS_SCAN_RETRY; + } else if (val_long == REDIS_SCAN_NOPREFIX) { + redis_sock->scan &= ~REDIS_SCAN_PREFIX; + } else if (val_long == REDIS_SCAN_RETRY || val_long == REDIS_SCAN_PREFIX) { + redis_sock->scan |= val_long; + } else { + break; } - break; + RETURN_TRUE; case REDIS_OPT_FAILOVER: val_long = zval_get_long(val); if (val_long == REDIS_FAILOVER_NONE || diff --git a/tests/RedisClusterTest.php b/tests/RedisClusterTest.php index 5a60cfdc..e5c66cc6 100644 --- a/tests/RedisClusterTest.php +++ b/tests/RedisClusterTest.php @@ -222,6 +222,55 @@ class Redis_Cluster_Test extends Redis_Test { $this->assertEquals($i_scan_count, $i_key_count); } + public function testScanPrefix() { + $arr_prefixes = ['prefix-a:', 'prefix-b:']; + $str_id = uniqid(); + + $arr_keys = []; + foreach ($arr_prefixes as $str_prefix) { + $this->redis->setOption(Redis::OPT_PREFIX, $str_prefix); + $this->redis->set($str_id, "LOLWUT"); + $arr_keys[$str_prefix] = $str_id; + } + + $this->redis->setOption(Redis::OPT_SCAN, Redis::SCAN_RETRY); + $this->redis->setOption(Redis::OPT_SCAN, Redis::SCAN_PREFIX); + + foreach ($arr_prefixes as $str_prefix) { + $arr_prefix_keys = []; + $this->redis->setOption(Redis::OPT_PREFIX, $str_prefix); + + foreach ($this->redis->_masters() as $arr_master) { + $it = NULL; + while ($arr_iter = $this->redis->scan($it, $arr_master, "*$str_id*")) { + foreach ($arr_iter as $str_key) { + $arr_prefix_keys[$str_prefix] = $str_key; + } + } + } + + $this->assertTrue(count($arr_prefix_keys) == 1 && isset($arr_prefix_keys[$str_prefix])); + } + + $this->redis->setOption(Redis::OPT_SCAN, Redis::SCAN_NOPREFIX); + + $arr_scan_keys = []; + + foreach ($this->redis->_masters() as $arr_master) { + $it = NULL; + while ($arr_iter = $this->redis->scan($it, $arr_master, "*$str_id*")) { + foreach ($arr_iter as $str_key) { + $arr_scan_keys[] = $str_key; + } + } + } + + /* We should now have both prefixs' keys */ + foreach ($arr_keys as $str_prefix => $str_id) { + $this->assertTrue(in_array("${str_prefix}${str_id}", $arr_scan_keys)); + } + } + // Run some simple tests against the PUBSUB command. This is problematic, as we // can't be sure what's going on in the instance, but we can do some things. public function testPubSub() { diff --git a/tests/RedisTest.php b/tests/RedisTest.php index 77c3fd85..06b9591a 100644 --- a/tests/RedisTest.php +++ b/tests/RedisTest.php @@ -5008,7 +5008,41 @@ class Redis_Test extends TestSuite } } } + } + + public function testScanPrefix() { + $keyid = uniqid(); + + /* Set some keys with different prefixes */ + $arr_prefixes = ['prefix-a:', 'prefix-b:']; + foreach ($arr_prefixes as $str_prefix) { + $this->redis->setOption(Redis::OPT_PREFIX, $str_prefix); + $this->redis->set("$keyid", "LOLWUT"); + $arr_all_keys["${str_prefix}${keyid}"] = true; + } + + $this->redis->setOption(Redis::OPT_SCAN, Redis::SCAN_RETRY); + $this->redis->setOption(Redis::OPT_SCAN, Redis::SCAN_PREFIX); + + foreach ($arr_prefixes as $str_prefix) { + $this->redis->setOption(Redis::OPT_PREFIX, $str_prefix); + $it = NULL; + $arr_keys = $this->redis->scan($it, "*$keyid*"); + $this->assertEquals($arr_keys, ["${str_prefix}${keyid}"]); + } + + /* Unset the prefix option */ + $this->redis->setOption(Redis::OPT_SCAN, Redis::SCAN_NOPREFIX); + + $it = NULL; + while ($arr_keys = $this->redis->scan($it, "*$keyid*")) { + foreach ($arr_keys as $str_key) { + unset($arr_all_keys[$str_key]); + } + } + /* Should have touched every key */ + $this->assertTrue(count($arr_all_keys) == 0); } public function testHScan() { -- cgit v1.2.3