Skip to content

Commit

Permalink
Issue #548 (#1649)
Browse files Browse the repository at this point in the history 
Adds `Redis::SCAN_PREFIX` and `Redis::SCAN_NOPREFIX` as options to SCAN.

See #548
  • Loading branch information
@yatsukhnenko
yatsukhnenko committed May 19, 2020
1 parent 508fef6 commit e80600e
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 13 deletions.
5 changes: 5 additions & 0 deletions README.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
~~~


Expand Down
4 changes: 3 additions & 1 deletion common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions library.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
13 changes: 11 additions & 2 deletions redis.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down
21 changes: 17 additions & 4 deletions redis_cluster.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand All @@ -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 */
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand Down
13 changes: 9 additions & 4 deletions redis_commands.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand Down
49 changes: 49 additions & 0 deletions tests/RedisClusterTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,55 @@ public function testScan() {
$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() {
Expand Down
34 changes: 34 additions & 0 deletions tests/RedisTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5008,7 +5008,41 @@ public function testScan() {
}
}
}
}

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() {
Expand Down

0 comments on commit e80600e

Please sign in to comment.