summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMatt Stancliff <matt@genges.com>2014-04-07 21:22:30 -0400
committerantirez <antirez@gmail.com>2014-06-09 11:39:44 +0200
commitb4f9761d85ff222aea8b07d3cd66352e5069dd28 (patch)
tree1ab16705d85927ac7771aee97aa42edb1528bb18
parent6ab4775a2a69188dc6f6594c20bf9137c7671f28 (diff)
downloadredis-b4f9761d85ff222aea8b07d3cd66352e5069dd28.tar.gz
Fix blocking operations from missing new lists
Behrad Zari discovered [1] and Josiah reported [2]: if you block and wait for a list to exist, but the list creates from a non-push command, the blocked client never gets notified. This commit adds notification of blocked clients into the DB layer and away from individual commands. Lists can be created by [LR]PUSH, SORT..STORE, RENAME, MOVE, and RESTORE. Previously, blocked client notifications were only triggered by [LR]PUSH. Your client would never get notified if a list were created by SORT..STORE or RENAME or a RESTORE, etc. Blocked client notification now happens in one unified place: - dbAdd() triggers notification when adding a list to the DB Two new tests are added that fail prior to this commit. All test pass. Fixes #1668 [1]: https://groups.google.com/forum/#!topic/redis-db/k4oWfMkN1NU [2]: #1668
-rw-r--r--src/db.c1
-rw-r--r--src/redis.h1
-rw-r--r--src/t_list.c16
-rw-r--r--tests/unit/type/list.tcl23
4 files changed, 30 insertions, 11 deletions
diff --git a/src/db.c b/src/db.c
index 654ca43c5..35fc81eb5 100644
--- a/src/db.c
+++ b/src/db.c
@@ -93,6 +93,7 @@ void dbAdd(redisDb *db, robj *key, robj *val) {
int retval = dictAdd(db->dict, copy, val);
redisAssertWithInfo(NULL,key,retval == REDIS_OK);
+ if (val->type == REDIS_LIST) signalListAsReady(db, key);
}
/* Overwrite an existing key with a new value. Incrementing the reference
diff --git a/src/redis.h b/src/redis.h
index 87061cc87..f6deb7975 100644
--- a/src/redis.h
+++ b/src/redis.h
@@ -968,6 +968,7 @@ void listTypeConvert(robj *subject, int enc);
void unblockClientWaitingData(redisClient *c);
void handleClientsBlockedOnLists(void);
void popGenericCommand(redisClient *c, int where);
+void signalListAsReady(redisDb *db, robj *key);
/* MULTI/EXEC/WATCH... */
void unwatchAllKeys(redisClient *c);
diff --git a/src/t_list.c b/src/t_list.c
index 24febe51a..ea18dd450 100644
--- a/src/t_list.c
+++ b/src/t_list.c
@@ -29,8 +29,6 @@
#include "redis.h"
-void signalListAsReady(redisClient *c, robj *key);
-
/*-----------------------------------------------------------------------------
* List API
*----------------------------------------------------------------------------*/
@@ -297,15 +295,12 @@ void listTypeConvert(robj *subject, int enc) {
void pushGenericCommand(redisClient *c, int where) {
int j, waiting = 0, pushed = 0;
robj *lobj = lookupKeyWrite(c->db,c->argv[1]);
- int may_have_waiting_clients = (lobj == NULL);
if (lobj && lobj->type != REDIS_LIST) {
addReply(c,shared.wrongtypeerr);
return;
}
- if (may_have_waiting_clients) signalListAsReady(c,c->argv[1]);
-
for (j = 2; j < c->argc; j++) {
c->argv[j] = tryObjectEncoding(c->argv[j]);
if (!lobj) {
@@ -709,7 +704,6 @@ void rpoplpushHandlePush(redisClient *c, robj *dstkey, robj *dstobj, robj *value
if (!dstobj) {
dstobj = createZiplistObject();
dbAdd(c->db,dstkey,dstobj);
- signalListAsReady(c,dstkey);
}
signalModifiedKey(c->db,dstkey);
listTypePush(dstobj,value,REDIS_HEAD);
@@ -855,19 +849,19 @@ void unblockClientWaitingData(redisClient *c) {
* made by a script or in the context of MULTI/EXEC.
*
* The list will be finally processed by handleClientsBlockedOnLists() */
-void signalListAsReady(redisClient *c, robj *key) {
+void signalListAsReady(redisDb *db, robj *key) {
readyList *rl;
/* No clients blocking for this key? No need to queue it. */
- if (dictFind(c->db->blocking_keys,key) == NULL) return;
+ if (dictFind(db->blocking_keys,key) == NULL) return;
/* Key was already signaled? No need to queue it again. */
- if (dictFind(c->db->ready_keys,key) != NULL) return;
+ if (dictFind(db->ready_keys,key) != NULL) return;
/* Ok, we need to queue this key into server.ready_keys. */
rl = zmalloc(sizeof(*rl));
rl->key = key;
- rl->db = c->db;
+ rl->db = db;
incrRefCount(key);
listAddNodeTail(server.ready_keys,rl);
@@ -875,7 +869,7 @@ void signalListAsReady(redisClient *c, robj *key) {
* to avoid adding it multiple times into a list with a simple O(1)
* check. */
incrRefCount(key);
- redisAssert(dictAdd(c->db->ready_keys,key,NULL) == DICT_OK);
+ redisAssert(dictAdd(db->ready_keys,key,NULL) == DICT_OK);
}
/* This is a helper function for handleClientsBlockedOnLists(). It's work
diff --git a/tests/unit/type/list.tcl b/tests/unit/type/list.tcl
index e665afc0a..c8e26602b 100644
--- a/tests/unit/type/list.tcl
+++ b/tests/unit/type/list.tcl
@@ -408,6 +408,29 @@ start_server {
$rd read
} {}
+ test "BLPOP when new key is moved into place" {
+ set rd [redis_deferring_client]
+
+ $rd blpop foo 5
+ r lpush bob abc def hij
+ r rename bob foo
+ $rd read
+ } {foo hij}
+
+ test "BLPOP when result key is created by SORT..STORE" {
+ set rd [redis_deferring_client]
+
+ # zero out list from previous test without explicit delete
+ r lpop foo
+ r lpop foo
+ r lpop foo
+
+ $rd blpop foo 5
+ r lpush notfoo hello hola aguacate konichiwa zanzibar
+ r sort notfoo ALPHA store foo
+ $rd read
+ } {foo aguacate}
+
foreach {pop} {BLPOP BRPOP} {
test "$pop: with single empty list argument" {
set rd [redis_deferring_client]