Fail EXEC command in case a watched key is expired (#9194)

There are two issues fixed in this commit: 
1. we want to fail the EXEC command in case there is a watched key that's logically
   expired but not yet deleted by active expire or lazy expire.
2. we saw that currently cache time is update in every `call()` (including nested calls),
   this time is being also being use for the isKeyExpired comparison, we want to update
   the cache time only in the first call (execCommand)

Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit ac8b1df8850cc80fbf9ce8c2fbde0c1d3a1b4e91)
This commit is contained in:
perryitay 2021-07-11 13:17:23 +03:00 committed by Oran Agra
parent 970302c00c
commit 6ae3673b5e
4 changed files with 46 additions and 3 deletions

View File

@ -168,6 +168,11 @@ void execCommand(client *c) {
return;
}
/* EXEC with expired watched key is disallowed*/
if (isWatchedKeyExpired(c)) {
c->flags |= (CLIENT_DIRTY_CAS);
}
/* Check if we need to abort the EXEC because:
* 1) Some WATCHed key was touched.
* 2) There was a previous error while queueing commands.
@ -342,6 +347,22 @@ void unwatchAllKeys(client *c) {
}
}
/* iterates over the watched_keys list and
* look for an expired key . */
int isWatchedKeyExpired(client *c) {
listIter li;
listNode *ln;
watchedKey *wk;
if (listLength(c->watched_keys) == 0) return 0;
listRewind(c->watched_keys,&li);
while ((ln = listNext(&li))) {
wk = listNodeValue(ln);
if (keyIsExpired(wk->db, wk->key)) return 1;
}
return 0;
}
/* "Touch" a key, so that if this key is being WATCHed by some client the
* next EXEC will fail. */
void touchWatchedKey(redisDb *db, robj *key) {

View File

@ -3697,8 +3697,6 @@ void call(client *c, int flags) {
struct redisCommand *real_cmd = c->cmd;
static long long prev_err_count;
server.fixed_time_expire++;
/* Initialization: clear the flags that must be set by the command on
* demand, and initialize the array for additional commands propagation. */
c->flags &= ~(CLIENT_FORCE_AOF|CLIENT_FORCE_REPL|CLIENT_PREVENT_PROP);
@ -3708,7 +3706,13 @@ void call(client *c, int flags) {
/* Call the command. */
dirty = server.dirty;
prev_err_count = server.stat_total_error_replies;
updateCachedTime(0);
/* Update cache time, in case we have nested calls we want to
* update only on the first call*/
if (server.fixed_time_expire++ == 0) {
updateCachedTime(0);
}
elapsedStart(&call_timer);
c->cmd->proc(c);
const long duration = elapsedUs(call_timer);

View File

@ -1945,6 +1945,7 @@ void initClientMultiState(client *c);
void freeClientMultiState(client *c);
void queueMultiCommand(client *c);
void touchWatchedKey(redisDb *db, robj *key);
int isWatchedKeyExpired(client *c);
void touchAllWatchedKeysInDb(redisDb *emptied, redisDb *replaced_with);
void discardTransaction(client *c);
void flagTransaction(client *c);
@ -2318,6 +2319,7 @@ void initConfigValues();
/* db.c -- Keyspace access API */
int removeExpire(redisDb *db, robj *key);
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);

View File

@ -121,6 +121,22 @@ start_server {tags {"multi"}} {
r exec
} {}
test {EXEC fail on lazy expired WATCHed key} {
r flushall
r debug set-active-expire 0
r del key
r set key 1 px 2
r watch key
after 100
r multi
r incr key
assert_equal [r exec] {}
r debug set-active-expire 1
} {OK} {needs:debug}
test {After successful EXEC key is no longer watched} {
r set x 30
r watch x