diff --git a/src/aof.c b/src/aof.c index a85399ab0..4b9900ab2 100644 --- a/src/aof.c +++ b/src/aof.c @@ -737,7 +737,8 @@ int loadAppendOnlyFile(char *filename) { * to the same file we're about to read. */ server.aof_state = AOF_OFF; - fakeClient = createAOFClient(); + client *old_client = server.current_client; + fakeClient = server.current_client = createAOFClient(); startLoadingFile(fp, filename, RDBFLAGS_AOF_PREAMBLE); /* Check if this AOF file has an RDB preamble. In that case we need to @@ -928,6 +929,7 @@ fmterr: /* Format error. */ cleanup: if (fakeClient) freeClient(fakeClient); + server.current_client = old_client; fclose(fp); stopLoading(ret == AOF_OK); return ret; diff --git a/src/blocked.c b/src/blocked.c index 3723b4fbf..a554e863b 100644 --- a/src/blocked.c +++ b/src/blocked.c @@ -591,8 +591,7 @@ void handleClientsBlockedOnKeys(void) { updateCachedTime(0); /* Serve clients blocked on the key. */ - robj *o = lookupKeyWrite(rl->db,rl->key); - + robj *o = lookupKeyReadWithFlags(rl->db, rl->key, LOOKUP_NONOTIFY | LOOKUP_NOSTATS); if (o != NULL) { if (o->type == OBJ_LIST) serveClientsBlockedOnListKey(o,rl); diff --git a/src/cluster.c b/src/cluster.c index aaad17358..215137505 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -6012,8 +6012,9 @@ clusterNode *getNodeByQuery(client *c, struct redisCommand *cmd, robj **argv, in } /* Migrating / Importing slot? Count keys we don't have. */ + int flags = LOOKUP_NOTOUCH | LOOKUP_NOSTATS | LOOKUP_NONOTIFY; if ((migrating_slot || importing_slot) && - lookupKeyRead(&server.db[0],thiskey) == NULL) + lookupKeyReadWithFlags(&server.db[0], thiskey, flags) == NULL) { missing_keys++; } diff --git a/src/db.c b/src/db.c index 1dfe00f58..b98310cf3 100644 --- a/src/db.c +++ b/src/db.c @@ -39,6 +39,7 @@ * C-level DB API *----------------------------------------------------------------------------*/ +int expireIfNeeded(redisDb *db, robj *key, int force_delete_expired); int keyIsExpired(redisDb *db, robj *key); /* Update LFU when an object is accessed. @@ -50,14 +51,53 @@ void updateLFU(robj *val) { val->lru = (LFUGetTimeInMinutes()<<8) | counter; } -/* Low level key lookup API, not actually called directly from commands - * implementations that should instead rely on lookupKeyRead(), - * lookupKeyWrite() and lookupKeyReadWithFlags(). */ +/* Lookup a key for read or write operations, or return NULL if the key is not + * found in the specified DB. This function implements the functionality of + * lookupKeyRead(), lookupKeyWrite() and their ...WithFlags() variants. + * + * Side-effects of calling this function: + * + * 1. A key gets expired if it reached it's TTL. + * 2. The key's last access time is updated. + * 3. The global keys hits/misses stats are updated (reported in INFO). + * 4. If keyspace notifications are enabled, a "keymiss" notification is fired. + * + * Flags change the behavior of this command: + * + * LOOKUP_NONE (or zero): No special flags are passed. + * LOOKUP_NOTOUCH: Don't alter the last access time of the key. + * LOOKUP_NONOTIFY: Don't trigger keyspace event on key miss. + * LOOKUP_NOSTATS: Don't increment key hits/misses couters. + * LOOKUP_WRITE: Prepare the key for writing (delete expired keys even on + * replicas, use separate keyspace stats and events (TODO)). + * + * Note: this function also returns NULL if the key is logically expired but + * still existing, in case this is a replica and the LOOKUP_WRITE is not set. + * Even if the key expiry is master-driven, we can correctly report a key is + * expired on replicas even if the master is lagging expiring our key via DELs + * in the replication link. */ robj *lookupKey(redisDb *db, robj *key, int flags) { dictEntry *de = dictFind(db->dict,key->ptr); + robj *val = NULL; if (de) { - robj *val = dictGetVal(de); + val = dictGetVal(de); + int force_delete_expired = flags & LOOKUP_WRITE; + if (force_delete_expired) { + /* Forcing deletion of expired keys on a replica makes the replica + * inconsistent with the master. The reason it's allowed for write + * commands is to make writable replicas behave consistently. It + * shall not be used in readonly commands. Modules are accepted so + * that we don't break old modules. */ + client *c = server.in_eval ? server.lua_client : server.current_client; + serverAssert(!c || !c->cmd || (c->cmd->flags & (CMD_WRITE|CMD_MODULE))); + } + if (expireIfNeeded(db, key, force_delete_expired)) { + /* The key is no longer valid. */ + val = NULL; + } + } + if (val) { /* Update the access time for the ageing algorithm. * Don't do it if we have a saving child, as this will trigger * a copy on write madness. */ @@ -68,75 +108,33 @@ robj *lookupKey(redisDb *db, robj *key, int flags) { val->lru = LRU_CLOCK(); } } - return val; + + if (!(flags & (LOOKUP_NOSTATS | LOOKUP_WRITE))) + server.stat_keyspace_hits++; + /* TODO: Use separate hits stats for WRITE */ } else { - return NULL; + if (!(flags & (LOOKUP_NONOTIFY | LOOKUP_WRITE))) + notifyKeyspaceEvent(NOTIFY_KEY_MISS, "keymiss", key, db->id); + if (!(flags & (LOOKUP_NOSTATS | LOOKUP_WRITE))) + server.stat_keyspace_misses++; + /* TODO: Use separate misses stats and notify event for WRITE */ } + + return val; } /* Lookup a key for read operations, or return NULL if the key is not found * in the specified DB. * - * As a side effect of calling this function: - * 1. A key gets expired if it reached it's TTL. - * 2. The key last access time is updated. - * 3. The global keys hits/misses stats are updated (reported in INFO). - * 4. If keyspace notifications are enabled, a "keymiss" notification is fired. - * * This API should not be used when we write to the key after obtaining * the object linked to the key, but only for read only operations. * - * Flags change the behavior of this command: - * - * LOOKUP_NONE (or zero): no special flags are passed. - * LOOKUP_NOTOUCH: don't alter the last access time of the key. - * - * Note: this function also returns NULL if the key is logically expired - * but still existing, in case this is a slave, since this API is called only - * for read operations. Even if the key expiry is master-driven, we can - * correctly report a key is expired on slaves even if the master is lagging - * expiring our key via DELs in the replication link. */ + * This function is equivalent to lookupKey(). The point of using this function + * rather than lookupKey() directly is to indicate that the purpose is to read + * the key. */ robj *lookupKeyReadWithFlags(redisDb *db, robj *key, int flags) { - robj *val; - - if (expireIfNeeded(db,key) == 1) { - /* If we are in the context of a master, expireIfNeeded() returns 1 - * when the key is no longer valid, so we can return NULL ASAP. */ - if (server.masterhost == NULL) - goto keymiss; - - /* However if we are in the context of a slave, expireIfNeeded() will - * not really try to expire the key, it only returns information - * about the "logical" status of the key: key expiring is up to the - * master in order to have a consistent view of master's data set. - * - * However, if the command caller is not the master, and as additional - * safety measure, the command invoked is a read-only command, we can - * safely return NULL here, and provide a more consistent behavior - * to clients accessing expired values in a read-only fashion, that - * will say the key as non existing. - * - * Notably this covers GETs when slaves are used to scale reads. */ - if (server.current_client && - server.current_client != server.master && - server.current_client->cmd && - server.current_client->cmd->flags & CMD_READONLY) - { - goto keymiss; - } - } - val = lookupKey(db,key,flags); - if (val == NULL) - goto keymiss; - server.stat_keyspace_hits++; - return val; - -keymiss: - if (!(flags & LOOKUP_NONOTIFY)) { - notifyKeyspaceEvent(NOTIFY_KEY_MISS, "keymiss", key, db->id); - } - server.stat_keyspace_misses++; - return NULL; + serverAssert(!(flags & LOOKUP_WRITE)); + return lookupKey(db, key, flags); } /* Like lookupKeyReadWithFlags(), but does not use any flag, which is the @@ -146,13 +144,13 @@ robj *lookupKeyRead(redisDb *db, robj *key) { } /* Lookup a key for write operations, and as a side effect, if needed, expires - * the key if its TTL is reached. + * the key if its TTL is reached. It's equivalent to lookupKey() with the + * LOOKUP_WRITE flag added. * * Returns the linked value object if the key exists or NULL if the key * does not exist in the specified DB. */ robj *lookupKeyWriteWithFlags(redisDb *db, robj *key, int flags) { - expireIfNeeded(db,key); - return lookupKey(db,key,flags); + return lookupKey(db, key, flags | LOOKUP_WRITE); } robj *lookupKeyWrite(redisDb *db, robj *key) { @@ -292,7 +290,7 @@ robj *dbRandomKey(redisDb *db) { * return a key name that may be already expired. */ return keyobj; } - if (expireIfNeeded(db,keyobj)) { + if (expireIfNeeded(db,keyobj,0)) { decrRefCount(keyobj); continue; /* search for another key. This expired. */ } @@ -641,7 +639,7 @@ void delGenericCommand(client *c, int lazy) { int numdel = 0, j; for (j = 1; j < c->argc; j++) { - expireIfNeeded(c->db,c->argv[j]); + expireIfNeeded(c->db,c->argv[j],0); int deleted = lazy ? dbAsyncDelete(c->db,c->argv[j]) : dbSyncDelete(c->db,c->argv[j]); if (deleted) { @@ -939,7 +937,7 @@ void scanGenericCommand(client *c, robj *o, unsigned long cursor) { } /* Filter element if it is an expired key. */ - if (!filter && o == NULL && expireIfNeeded(c->db, kobj)) filter = 1; + if (!filter && o == NULL && expireIfNeeded(c->db, kobj, 0)) filter = 1; /* Remove the element and its associated value if needed. */ if (filter) { @@ -1206,7 +1204,7 @@ void copyCommand(client *c) { } /* Check if the element exists and get a reference */ - o = lookupKeyWrite(c->db, key); + o = lookupKeyRead(c->db, key); if (!o) { addReply(c,shared.czero); return; @@ -1266,8 +1264,11 @@ void scanDatabaseForReadyLists(redisDb *db) { dictIterator *di = dictGetSafeIterator(db->blocking_keys); while((de = dictNext(di)) != NULL) { robj *key = dictGetKey(de); - robj *value = lookupKey(db,key,LOOKUP_NOTOUCH); - if (value) signalKeyAsReady(db, key, value->type); + dictEntry *kde = dictFind(db->dict,key->ptr); + if (kde) { + robj *value = dictGetVal(kde); + signalKeyAsReady(db, key, value->type); + } } dictReleaseIterator(di); } @@ -1529,10 +1530,10 @@ int keyIsExpired(redisDb *db, robj *key) { * is via lookupKey*() family of functions. * * The behavior of the function depends on the replication role of the - * instance, because slave instances do not expire keys, they wait - * for DELs from the master for consistency matters. However even - * slaves will try to have a coherent return value for the function, - * so that read commands executed in the slave side will be able to + * instance, because by default replicas do not delete expired keys. They + * wait for DELs from the master for consistency matters. However even + * replicas will try to have a coherent return value for the function, + * so that read commands executed in the replica side will be able to * behave like if the key is expired even if still present (because the * master has yet to propagate the DEL). * @@ -1540,20 +1541,34 @@ int keyIsExpired(redisDb *db, robj *key) { * key will be evicted from the database. Also this may trigger the * propagation of a DEL/UNLINK command in AOF / replication stream. * + * On replicas, this function does not delete expired keys by default, but + * it still returns 1 if the key is logically expired. To force deletion + * of logically expired keys even on replicas, set force_delete_expired to + * a non-zero value. Note though that if the current client is executing + * replicated commands from the master, keys are never considered expired. + * * The return value of the function is 0 if the key is still valid, * otherwise the function returns 1 if the key is expired. */ -int expireIfNeeded(redisDb *db, robj *key) { +int expireIfNeeded(redisDb *db, robj *key, int force_delete_expired) { if (!keyIsExpired(db,key)) return 0; - /* If we are running in the context of a slave, instead of + /* If we are running in the context of a replica, instead of * evicting the expired key from the database, we return ASAP: - * the slave key expiration is controlled by the master that will - * send us synthesized DEL operations for expired keys. + * the replica key expiration is controlled by the master that will + * send us synthesized DEL operations for expired keys. The + * exception is when write operations are performed on writable + * replicas. * * Still we try to return the right information to the caller, * that is, 0 if we think the key should be still valid, 1 if - * we think the key is expired at this time. */ - if (server.masterhost != NULL) return 1; + * we think the key is expired at this time. + * + * When replicating commands from the master, keys are never considered + * expired. */ + if (server.masterhost != NULL) { + if (server.current_client == server.master) return 0; + if (!force_delete_expired) return 1; + } /* If clients are paused, we keep the current dataset constant, * but return to the client what we believe is the right state. Typically, diff --git a/src/hyperloglog.c b/src/hyperloglog.c index 82c43d44c..5c50fd694 100644 --- a/src/hyperloglog.c +++ b/src/hyperloglog.c @@ -1258,8 +1258,15 @@ void pfcountCommand(client *c) { /* Case 2: cardinality of the single HLL. * * The user specified a single key. Either return the cached value - * or compute one and update the cache. */ - o = lookupKeyWrite(c->db,c->argv[1]); + * or compute one and update the cache. + * + * Since a HLL is a regular Redis string type value, updating the cache does + * modify the value. We do a lookupKeyRead anyway since this is flagged as a + * read-only command. The difference is that with lookupKeyWrite, a + * logically expired key on a replica is deleted, while with lookupKeyRead + * it isn't, but the lookup returns NULL either way if the key is logically + * expired, which is what matters here. */ + o = lookupKeyRead(c->db,c->argv[1]); if (o == NULL) { /* No key? Cardinality is zero since no element was added, otherwise * we would have a key as HLLADD creates it as a side effect. */ @@ -1296,8 +1303,7 @@ void pfcountCommand(client *c) { hdr->card[5] = (card >> 40) & 0xff; hdr->card[6] = (card >> 48) & 0xff; hdr->card[7] = (card >> 56) & 0xff; - /* This is not considered a read-only command even if the - * data structure is not modified, since the cached value + /* This is considered a read-only command even if the cached value * may be modified and given that the HLL is a Redis string * we need to propagate the change. */ signalModifiedKey(c,c->db,c->argv[1]); diff --git a/src/server.h b/src/server.h index a24696f6d..4e300aa9e 100644 --- a/src/server.h +++ b/src/server.h @@ -2623,11 +2623,9 @@ int removeExpire(redisDb *db, robj *key); void deleteExpiredKeyAndPropagate(redisDb *db, robj *keyobj); void propagateExpire(redisDb *db, robj *key, int lazy); int keyIsExpired(redisDb *db, robj *key); -int expireIfNeeded(redisDb *db, robj *key); long long getExpire(redisDb *db, robj *key); void setExpire(client *c, redisDb *db, robj *key, long long when); int checkAlreadyExpired(long long when); -robj *lookupKey(redisDb *db, robj *key, int flags); robj *lookupKeyRead(redisDb *db, robj *key); robj *lookupKeyWrite(redisDb *db, robj *key); robj *lookupKeyReadOrReply(client *c, robj *key, robj *reply); @@ -2639,8 +2637,11 @@ robj *objectCommandLookupOrReply(client *c, robj *key, robj *reply); int objectSetLRUOrLFU(robj *val, long long lfu_freq, long long lru_idle, long long lru_clock, int lru_multiplier); #define LOOKUP_NONE 0 -#define LOOKUP_NOTOUCH (1<<0) -#define LOOKUP_NONOTIFY (1<<1) +#define LOOKUP_NOTOUCH (1<<0) /* Don't update LRU. */ +#define LOOKUP_NONOTIFY (1<<1) /* Don't trigger keyspace event on key misses. */ +#define LOOKUP_NOSTATS (1<<2) /* Don't update keyspace hits/misses couters. */ +#define LOOKUP_WRITE (1<<3) /* Delete expired keys even in replicas. */ + void dbAdd(redisDb *db, robj *key, robj *val); int dbAddRDBLoad(redisDb *db, sds key, robj *val); void dbOverwrite(redisDb *db, robj *key, robj *val); diff --git a/src/sort.c b/src/sort.c index 436c7fb0e..fe0d55fea 100644 --- a/src/sort.c +++ b/src/sort.c @@ -58,7 +58,7 @@ redisSortOperation *createSortOperation(int type, robj *pattern) { * * The returned object will always have its refcount increased by 1 * when it is non-NULL. */ -robj *lookupKeyByPattern(redisDb *db, robj *pattern, robj *subst, int writeflag) { +robj *lookupKeyByPattern(redisDb *db, robj *pattern, robj *subst) { char *p, *f, *k; sds spat, ssub; robj *keyobj, *fieldobj = NULL, *o; @@ -106,10 +106,7 @@ robj *lookupKeyByPattern(redisDb *db, robj *pattern, robj *subst, int writeflag) decrRefCount(subst); /* Incremented by decodeObject() */ /* Lookup substituted key */ - if (!writeflag) - o = lookupKeyRead(db,keyobj); - else - o = lookupKeyWrite(db,keyobj); + o = lookupKeyRead(db, keyobj); if (o == NULL) goto noobj; if (fieldobj) { @@ -270,10 +267,7 @@ void sortCommandGeneric(client *c, int readonly) { } /* Lookup the key to sort. It must be of the right types */ - if (!storekey) - sortval = lookupKeyRead(c->db,c->argv[1]); - else - sortval = lookupKeyWrite(c->db,c->argv[1]); + sortval = lookupKeyRead(c->db, c->argv[1]); if (sortval && sortval->type != OBJ_SET && sortval->type != OBJ_LIST && sortval->type != OBJ_ZSET) @@ -459,7 +453,7 @@ void sortCommandGeneric(client *c, int readonly) { robj *byval; if (sortby) { /* lookup value to sort by */ - byval = lookupKeyByPattern(c->db,sortby,vector[j].obj,storekey!=NULL); + byval = lookupKeyByPattern(c->db,sortby,vector[j].obj); if (!byval) continue; } else { /* use object itself to sort by */ @@ -522,7 +516,7 @@ void sortCommandGeneric(client *c, int readonly) { while((ln = listNext(&li))) { redisSortOperation *sop = ln->value; robj *val = lookupKeyByPattern(c->db,sop->pattern, - vector[j].obj,storekey!=NULL); + vector[j].obj); if (sop->type == SORT_OP_GET) { if (!val) { @@ -552,7 +546,7 @@ void sortCommandGeneric(client *c, int readonly) { while((ln = listNext(&li))) { redisSortOperation *sop = ln->value; robj *val = lookupKeyByPattern(c->db,sop->pattern, - vector[j].obj,storekey!=NULL); + vector[j].obj); if (sop->type == SORT_OP_GET) { if (!val) val = createStringObject("",0); diff --git a/src/t_set.c b/src/t_set.c index ec9749f13..3c2de9908 100644 --- a/src/t_set.c +++ b/src/t_set.c @@ -871,9 +871,7 @@ void sinterGenericCommand(client *c, robj **setkeys, int encoding, empty = 0; for (j = 0; j < setnum; j++) { - robj *setobj = dstkey ? - lookupKeyWrite(c->db,setkeys[j]) : - lookupKeyRead(c->db,setkeys[j]); + robj *setobj = lookupKeyRead(c->db, setkeys[j]); if (!setobj) { /* A NULL is considered an empty set */ empty += 1; @@ -1059,9 +1057,7 @@ void sunionDiffGenericCommand(client *c, robj **setkeys, int setnum, int diff_algo = 1; for (j = 0; j < setnum; j++) { - robj *setobj = dstkey ? - lookupKeyWrite(c->db,setkeys[j]) : - lookupKeyRead(c->db,setkeys[j]); + robj *setobj = lookupKeyRead(c->db, setkeys[j]); if (!setobj) { sets[j] = NULL; continue; diff --git a/src/t_zset.c b/src/t_zset.c index 483951535..7470f785d 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -2551,9 +2551,7 @@ void zunionInterDiffGenericCommand(client *c, robj *dstkey, int numkeysIndex, in /* read keys to be used for input */ src = zcalloc(sizeof(zsetopsrc) * setnum); for (i = 0, j = numkeysIndex+1; i < setnum; i++, j++) { - robj *obj = dstkey ? - lookupKeyWrite(c->db,c->argv[j]) : - lookupKeyRead(c->db,c->argv[j]); + robj *obj = lookupKeyRead(c->db, c->argv[j]); if (obj != NULL) { if (obj->type != OBJ_ZSET && obj->type != OBJ_SET) { zfree(src); @@ -3646,9 +3644,7 @@ void zrangeGenericCommand(zrange_result_handler *handler, int argc_start, int st } /* Step 3: Lookup the key and get the range. */ - zobj = handler->dstkey ? - lookupKeyWrite(c->db,key) : - lookupKeyRead(c->db,key); + zobj = lookupKeyRead(c->db, key); if (zobj == NULL) { if (store) { handler->beginResultEmission(handler); diff --git a/tests/integration/replication-3.tcl b/tests/integration/replication-3.tcl index 32ec1a932..4546e414e 100644 --- a/tests/integration/replication-3.tcl +++ b/tests/integration/replication-3.tcl @@ -64,6 +64,68 @@ start_server {tags {"repl external:skip"}} { after 6000 r -1 dbsize } {0} + + test {Writable replica doesn't return expired keys} { + r select 5 + assert {[r dbsize] == 0} + r debug set-active-expire 0 + r set key1 5 px 10 + r set key2 5 px 10 + r -1 select 5 + wait_for_condition 50 100 { + [r -1 dbsize] == 2 && [r -1 exists key1 key2] == 0 + } else { + fail "Keys didn't replicate or didn't expire." + } + r -1 config set slave-read-only no + assert_equal 2 [r -1 dbsize] ; # active expire is off + assert_equal 1 [r -1 incr key1] ; # incr expires and re-creates key1 + assert_equal -1 [r -1 ttl key1] ; # incr created key1 without TTL + assert_equal {} [r -1 get key2] ; # key2 expired but not deleted + assert_equal 2 [r -1 dbsize] + # cleanup + r debug set-active-expire 1 + r -1 del key1 key2 + r -1 config set slave-read-only yes + r del key1 key2 + } + + test {PFCOUNT updates cache on readonly replica} { + r select 5 + assert {[r dbsize] == 0} + r pfadd key a b c d e f g h i j k l m n o p q + set strval [r get key] + r -1 select 5 + wait_for_condition 50 100 { + [r -1 dbsize] == 1 + } else { + fail "Replication timeout." + } + assert {$strval == [r -1 get key]} + assert_equal 17 [r -1 pfcount key] + assert {$strval != [r -1 get key]}; # cache updated + # cleanup + r del key + } + + test {PFCOUNT doesn't use expired key on readonly replica} { + r select 5 + assert {[r dbsize] == 0} + r debug set-active-expire 0 + r pfadd key a b c d e f g h i j k l m n o p q + r pexpire key 10 + r -1 select 5 + wait_for_condition 50 100 { + [r -1 dbsize] == 1 && [r -1 exists key] == 0 + } else { + fail "Key didn't replicate or didn't expire." + } + assert_equal [r -1 pfcount key] 0 ; # expired key not used + assert_equal [r -1 dbsize] 1 ; # but it's also not deleted + # cleanup + r debug set-active-expire 1 + r del key + } } } diff --git a/tests/integration/replication-4.tcl b/tests/integration/replication-4.tcl index 29e444b82..00c5d8ae0 100644 --- a/tests/integration/replication-4.tcl +++ b/tests/integration/replication-4.tcl @@ -85,6 +85,40 @@ start_server {tags {"repl external:skip"}} { } } +start_server {tags {"repl external:skip"}} { + start_server {} { + set master [srv -1 client] + set master_host [srv -1 host] + set master_port [srv -1 port] + set slave [srv 0 client] + + test {First server should have role slave after SLAVEOF} { + $slave slaveof $master_host $master_port + wait_for_condition 50 100 { + [s 0 master_link_status] eq {up} + } else { + fail "Replication not started." + } + } + + test {Replication of an expired key does not delete the expired key} { + $master debug set-active-expire 0 + $master set k 1 ex 1 + wait_for_ofs_sync $master $slave + exec kill -SIGSTOP [srv 0 pid] + $master incr k + after 1000 + # Stopping the replica for one second to makes sure the INCR arrives + # to the replica after the key is logically expired. + exec kill -SIGCONT [srv 0 pid] + wait_for_ofs_sync $master $slave + # Check that k is locigally expired but is present in the replica. + assert_equal 0 [$slave exists k] + $slave debug object k ; # Raises exception if k is gone. + } + } +} + start_server {tags {"repl external:skip"}} { start_server {} { set master [srv -1 client] diff --git a/tests/modules/aclcheck.c b/tests/modules/aclcheck.c index 739890132..e307744fd 100644 --- a/tests/modules/aclcheck.c +++ b/tests/modules/aclcheck.c @@ -157,7 +157,7 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) if (RedisModule_Init(ctx,"aclcheck",1,REDISMODULE_APIVER_1)== REDISMODULE_ERR) return REDISMODULE_ERR; - if (RedisModule_CreateCommand(ctx,"aclcheck.set.check.key", set_aclcheck_key,"",0,0,0) == REDISMODULE_ERR) + if (RedisModule_CreateCommand(ctx,"aclcheck.set.check.key", set_aclcheck_key,"write",0,0,0) == REDISMODULE_ERR) return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx,"aclcheck.publish.check.channel", publish_aclcheck_channel,"",0,0,0) == REDISMODULE_ERR) @@ -169,7 +169,8 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) if (RedisModule_CreateCommand(ctx,"aclcheck.rm_call.check.cmd.module.user", rm_call_aclcheck_cmd_module_user,"",0,0,0) == REDISMODULE_ERR) return REDISMODULE_ERR; - if (RedisModule_CreateCommand(ctx,"aclcheck.rm_call", rm_call_aclcheck,"",0,0,0) == REDISMODULE_ERR) + if (RedisModule_CreateCommand(ctx,"aclcheck.rm_call", rm_call_aclcheck, + "write",0,0,0) == REDISMODULE_ERR) return REDISMODULE_ERR; return REDISMODULE_OK; diff --git a/tests/modules/basics.c b/tests/modules/basics.c index 7f4383759..dd2e0e165 100644 --- a/tests/modules/basics.c +++ b/tests/modules/basics.c @@ -936,12 +936,12 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx,"test.basics", - TestBasics,"readonly",1,1,1) == REDISMODULE_ERR) + TestBasics,"write",1,1,1) == REDISMODULE_ERR) return REDISMODULE_ERR; /* the following commands are used by an external test and should not be added to TestBasics */ if (RedisModule_CreateCommand(ctx,"test.rmcallautomode", - TestCallRespAutoMode,"readonly",1,1,1) == REDISMODULE_ERR) + TestCallRespAutoMode,"write",1,1,1) == REDISMODULE_ERR) return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx,"test.getresp", diff --git a/tests/modules/blockedclient.c b/tests/modules/blockedclient.c index ebe89ecb2..5df51f63d 100644 --- a/tests/modules/blockedclient.c +++ b/tests/modules/blockedclient.c @@ -214,7 +214,8 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) if (RedisModule_CreateCommand(ctx, "acquire_gil", acquire_gil, "", 0, 0, 0) == REDISMODULE_ERR) return REDISMODULE_ERR; - if (RedisModule_CreateCommand(ctx, "do_rm_call", do_rm_call, "", 0, 0, 0) == REDISMODULE_ERR) + if (RedisModule_CreateCommand(ctx, "do_rm_call", do_rm_call, + "write", 0, 0, 0) == REDISMODULE_ERR) return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx, "do_bg_rm_call", do_bg_rm_call, "", 0, 0, 0) == REDISMODULE_ERR) diff --git a/tests/modules/blockonkeys.c b/tests/modules/blockonkeys.c index 48b408d59..72ccbcf38 100644 --- a/tests/modules/blockonkeys.c +++ b/tests/modules/blockonkeys.c @@ -470,35 +470,35 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) if (fsltype == NULL) return REDISMODULE_ERR; - if (RedisModule_CreateCommand(ctx,"fsl.push",fsl_push,"",1,1,1) == REDISMODULE_ERR) + if (RedisModule_CreateCommand(ctx,"fsl.push",fsl_push,"write",1,1,1) == REDISMODULE_ERR) return REDISMODULE_ERR; - if (RedisModule_CreateCommand(ctx,"fsl.bpop",fsl_bpop,"",1,1,1) == REDISMODULE_ERR) + if (RedisModule_CreateCommand(ctx,"fsl.bpop",fsl_bpop,"write",1,1,1) == REDISMODULE_ERR) return REDISMODULE_ERR; - if (RedisModule_CreateCommand(ctx,"fsl.bpopgt",fsl_bpopgt,"",1,1,1) == REDISMODULE_ERR) + if (RedisModule_CreateCommand(ctx,"fsl.bpopgt",fsl_bpopgt,"write",1,1,1) == REDISMODULE_ERR) return REDISMODULE_ERR; - if (RedisModule_CreateCommand(ctx,"fsl.bpoppush",fsl_bpoppush,"",1,2,1) == REDISMODULE_ERR) + if (RedisModule_CreateCommand(ctx,"fsl.bpoppush",fsl_bpoppush,"write",1,2,1) == REDISMODULE_ERR) return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx,"fsl.getall",fsl_getall,"",1,1,1) == REDISMODULE_ERR) return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx, "blockonkeys.popall", blockonkeys_popall, - "", 1, 1, 1) == REDISMODULE_ERR) + "write", 1, 1, 1) == REDISMODULE_ERR) return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx, "blockonkeys.lpush", blockonkeys_lpush, - "", 1, 1, 1) == REDISMODULE_ERR) + "write", 1, 1, 1) == REDISMODULE_ERR) return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx, "blockonkeys.lpush_unblock", blockonkeys_lpush, - "", 1, 1, 1) == REDISMODULE_ERR) + "write", 1, 1, 1) == REDISMODULE_ERR) return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx, "blockonkeys.blpopn", blockonkeys_blpopn, - "", 1, 1, 1) == REDISMODULE_ERR) + "write", 1, 1, 1) == REDISMODULE_ERR) return REDISMODULE_ERR; return REDISMODULE_OK; diff --git a/tests/modules/datatype.c b/tests/modules/datatype.c index 8a70d4be1..d763a2412 100644 --- a/tests/modules/datatype.c +++ b/tests/modules/datatype.c @@ -206,19 +206,22 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) if (datatype == NULL) return REDISMODULE_ERR; - if (RedisModule_CreateCommand(ctx,"datatype.set", datatype_set,"deny-oom",1,1,1) == REDISMODULE_ERR) + if (RedisModule_CreateCommand(ctx,"datatype.set", datatype_set, + "write deny-oom", 1, 1, 1) == REDISMODULE_ERR) return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx,"datatype.get", datatype_get,"",1,1,1) == REDISMODULE_ERR) return REDISMODULE_ERR; - if (RedisModule_CreateCommand(ctx,"datatype.restore", datatype_restore,"deny-oom",1,1,1) == REDISMODULE_ERR) + if (RedisModule_CreateCommand(ctx,"datatype.restore", datatype_restore, + "write deny-oom", 1, 1, 1) == REDISMODULE_ERR) return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx,"datatype.dump", datatype_dump,"",1,1,1) == REDISMODULE_ERR) return REDISMODULE_ERR; - if (RedisModule_CreateCommand(ctx,"datatype.swap", datatype_swap,"",1,1,1) == REDISMODULE_ERR) + if (RedisModule_CreateCommand(ctx, "datatype.swap", datatype_swap, + "write", 1, 1, 1) == REDISMODULE_ERR) return REDISMODULE_ERR; return REDISMODULE_OK; diff --git a/tests/modules/hash.c b/tests/modules/hash.c index 05ab03800..001a34e49 100644 --- a/tests/modules/hash.c +++ b/tests/modules/hash.c @@ -81,7 +81,7 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) REDISMODULE_NOT_USED(argc); if (RedisModule_Init(ctx, "hash", 1, REDISMODULE_APIVER_1) == REDISMODULE_OK && - RedisModule_CreateCommand(ctx, "hash.set", hash_set, "", + RedisModule_CreateCommand(ctx, "hash.set", hash_set, "write", 1, 1, 1) == REDISMODULE_OK) { return REDISMODULE_OK; } else { diff --git a/tests/modules/keyspace_events.c b/tests/modules/keyspace_events.c index 8a55e0f44..1cfeae998 100644 --- a/tests/modules/keyspace_events.c +++ b/tests/modules/keyspace_events.c @@ -250,19 +250,23 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) return REDISMODULE_ERR; } - if (RedisModule_CreateCommand(ctx,"keyspace.del_key_copy", cmdDelKeyCopy,"",0,0,0) == REDISMODULE_ERR){ + if (RedisModule_CreateCommand(ctx, "keyspace.del_key_copy", cmdDelKeyCopy, + "write", 0, 0, 0) == REDISMODULE_ERR){ return REDISMODULE_ERR; } - if (RedisModule_CreateCommand(ctx,"keyspace.incr_case1", cmdIncrCase1,"",0,0,0) == REDISMODULE_ERR){ + if (RedisModule_CreateCommand(ctx, "keyspace.incr_case1", cmdIncrCase1, + "write", 0, 0, 0) == REDISMODULE_ERR){ return REDISMODULE_ERR; } - if (RedisModule_CreateCommand(ctx,"keyspace.incr_case2", cmdIncrCase2,"",0,0,0) == REDISMODULE_ERR){ + if (RedisModule_CreateCommand(ctx, "keyspace.incr_case2", cmdIncrCase2, + "write", 0, 0, 0) == REDISMODULE_ERR){ return REDISMODULE_ERR; } - if (RedisModule_CreateCommand(ctx,"keyspace.incr_case3", cmdIncrCase3,"",0,0,0) == REDISMODULE_ERR){ + if (RedisModule_CreateCommand(ctx, "keyspace.incr_case3", cmdIncrCase3, + "write", 0, 0, 0) == REDISMODULE_ERR){ return REDISMODULE_ERR; } diff --git a/tests/modules/list.c b/tests/modules/list.c index fcbd446ce..727b05d6f 100644 --- a/tests/modules/list.c +++ b/tests/modules/list.c @@ -219,15 +219,15 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) if (RedisModule_Init(ctx, "list", 1, REDISMODULE_APIVER_1) == REDISMODULE_OK && RedisModule_CreateCommand(ctx, "list.getall", list_getall, "", 1, 1, 1) == REDISMODULE_OK && - RedisModule_CreateCommand(ctx, "list.edit", list_edit, "", + RedisModule_CreateCommand(ctx, "list.edit", list_edit, "write", 1, 1, 1) == REDISMODULE_OK && - RedisModule_CreateCommand(ctx, "list.get", list_get, "", + RedisModule_CreateCommand(ctx, "list.get", list_get, "write", 1, 1, 1) == REDISMODULE_OK && - RedisModule_CreateCommand(ctx, "list.set", list_set, "", + RedisModule_CreateCommand(ctx, "list.set", list_set, "write", 1, 1, 1) == REDISMODULE_OK && - RedisModule_CreateCommand(ctx, "list.insert", list_insert, "", + RedisModule_CreateCommand(ctx, "list.insert", list_insert, "write", 1, 1, 1) == REDISMODULE_OK && - RedisModule_CreateCommand(ctx, "list.delete", list_delete, "", + RedisModule_CreateCommand(ctx, "list.delete", list_delete, "write", 1, 1, 1) == REDISMODULE_OK) { return REDISMODULE_OK; } else { diff --git a/tests/modules/propagate.c b/tests/modules/propagate.c index 766c61ea5..b9852c158 100644 --- a/tests/modules/propagate.c +++ b/tests/modules/propagate.c @@ -239,17 +239,17 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) if (RedisModule_CreateCommand(ctx,"propagate-test.mixed", propagateTestMixedCommand, - "",1,1,1) == REDISMODULE_ERR) + "write",1,1,1) == REDISMODULE_ERR) return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx,"propagate-test.nested", propagateTestNestedCommand, - "",1,1,1) == REDISMODULE_ERR) + "write",1,1,1) == REDISMODULE_ERR) return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx,"propagate-test.incr", propagateTestIncr, - "",1,1,1) == REDISMODULE_ERR) + "write",1,1,1) == REDISMODULE_ERR) return REDISMODULE_ERR; return REDISMODULE_OK; diff --git a/tests/modules/stream.c b/tests/modules/stream.c index b82568b94..65762a399 100644 --- a/tests/modules/stream.c +++ b/tests/modules/stream.c @@ -238,19 +238,19 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) if (RedisModule_Init(ctx, "stream", 1, REDISMODULE_APIVER_1) == REDISMODULE_ERR) return REDISMODULE_ERR; - if (RedisModule_CreateCommand(ctx, "stream.add", stream_add, "", + if (RedisModule_CreateCommand(ctx, "stream.add", stream_add, "write", 1, 1, 1) == REDISMODULE_ERR) return REDISMODULE_ERR; - if (RedisModule_CreateCommand(ctx, "stream.addn", stream_addn, "", + if (RedisModule_CreateCommand(ctx, "stream.addn", stream_addn, "write", 1, 1, 1) == REDISMODULE_ERR) return REDISMODULE_ERR; - if (RedisModule_CreateCommand(ctx, "stream.delete", stream_delete, "", + if (RedisModule_CreateCommand(ctx, "stream.delete", stream_delete, "write", 1, 1, 1) == REDISMODULE_ERR) return REDISMODULE_ERR; - if (RedisModule_CreateCommand(ctx, "stream.range", stream_range, "", + if (RedisModule_CreateCommand(ctx, "stream.range", stream_range, "write", 1, 1, 1) == REDISMODULE_ERR) return REDISMODULE_ERR; - if (RedisModule_CreateCommand(ctx, "stream.trim", stream_trim, "", + if (RedisModule_CreateCommand(ctx, "stream.trim", stream_trim, "write", 1, 1, 1) == REDISMODULE_ERR) return REDISMODULE_ERR; diff --git a/tests/modules/test_lazyfree.c b/tests/modules/test_lazyfree.c index 144dab9b3..7ba213ff8 100644 --- a/tests/modules/test_lazyfree.c +++ b/tests/modules/test_lazyfree.c @@ -98,7 +98,7 @@ int LazyFreeLinkLen_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, if (argc != 2) return RedisModule_WrongArity(ctx); RedisModuleKey *key = RedisModule_OpenKey(ctx,argv[1], - REDISMODULE_READ|REDISMODULE_WRITE); + REDISMODULE_READ); int type = RedisModule_KeyType(key); if (type != REDISMODULE_KEYTYPE_EMPTY && RedisModule_ModuleTypeGetType(key) != LazyFreeLinkType) diff --git a/tests/modules/testrdb.c b/tests/modules/testrdb.c index 6786fdf2e..6f2ca2a14 100644 --- a/tests/modules/testrdb.c +++ b/tests/modules/testrdb.c @@ -262,7 +262,7 @@ int testrdb_get_key(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) return REDISMODULE_OK; } - RedisModuleKey *key = RedisModule_OpenKey(ctx, argv[1], REDISMODULE_WRITE); + RedisModuleKey *key = RedisModule_OpenKey(ctx, argv[1], REDISMODULE_READ); RedisModuleString *str = RedisModule_ModuleTypeGetValue(key); RedisModule_CloseKey(key); RedisModule_ReplyWithString(ctx, str); diff --git a/tests/modules/zset.c b/tests/modules/zset.c index 4806f6549..91791f907 100644 --- a/tests/modules/zset.c +++ b/tests/modules/zset.c @@ -22,7 +22,7 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) REDISMODULE_NOT_USED(argc); if (RedisModule_Init(ctx, "zset", 1, REDISMODULE_APIVER_1) == REDISMODULE_OK && - RedisModule_CreateCommand(ctx, "zset.rem", zset_rem, "", + RedisModule_CreateCommand(ctx, "zset.rem", zset_rem, "write", 1, 1, 1) == REDISMODULE_OK) return REDISMODULE_OK; else diff --git a/tests/unit/type/list.tcl b/tests/unit/type/list.tcl index 44f201276..a6a0060a0 100644 --- a/tests/unit/type/list.tcl +++ b/tests/unit/type/list.tcl @@ -1032,6 +1032,44 @@ foreach {pop} {BLPOP BLMPOP_LEFT} { set _ $res } {} + test {SWAPDB awakes blocked client} { + r flushall + r select 1 + r rpush k hello + r select 9 + set rd [redis_deferring_client] + $rd brpop k 5 + wait_for_blocked_clients_count 1 + r swapdb 1 9 + $rd read + } {k hello} {singledb:skip} + + test {SWAPDB awakes blocked client, but the key already expired} { + r flushall + r debug set-active-expire 0 + r select 1 + r rpush k hello + r pexpire k 100 + set rd [redis_deferring_client] + $rd select 9 + assert_equal {OK} [$rd read] + $rd client id + set id [$rd read] + $rd brpop k 1 + wait_for_blocked_clients_count 1 + after 101 + r swapdb 1 9 + # The SWAPDB command tries to awake the blocked client, but it remains + # blocked because the key is expired. Check that the deferred client is + # still blocked. Then unblock it. + assert_match "*flags=b*" [r client list id $id] + r client unblock $id + assert_equal {} [$rd read] + # Restore server and client state + r debug set-active-expire 1 + r select 9 + } {OK} {singledb:skip} + foreach {pop} {BLPOP BLMPOP_LEFT} { test "$pop when new key is moved into place" { set rd [redis_deferring_client]