summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOran Agra <oran@redislabs.com>2020-07-10 10:02:37 +0300
committerOran Agra <oran@redislabs.com>2020-10-27 08:49:22 +0200
commite9c9e4c2af74cb0571855b1f2cde692e8c436a40 (patch)
tree3a2118fe1c91be8d78b11fce5980d14842636cac
parentee4696b1501c22f3dd79d3f89058ab603981a87a (diff)
downloadredis-e9c9e4c2af74cb0571855b1f2cde692e8c436a40.tar.gz
RESTORE ABSTTL won't store expired keys into the db (#7472)
Similarly to EXPIREAT with TTL in the past, which implicitly deletes the key and return success, RESTORE should not store key that are already expired into the db. When used together with REPLACE it should emit a DEL to keyspace notification and replication stream. (cherry picked from commit 5977a94842a25140520297fe4bfda15e0e4de711) (cherry picked from commit 95ba01b53849d56e5699aebb5db532563f976491)
-rw-r--r--src/cluster.c28
-rw-r--r--src/expire.c18
-rw-r--r--src/server.h1
-rw-r--r--tests/unit/dump.tcl13
4 files changed, 45 insertions, 15 deletions
diff --git a/src/cluster.c b/src/cluster.c
index e33553479..fdb9e8f50 100644
--- a/src/cluster.c
+++ b/src/cluster.c
@@ -4918,7 +4918,8 @@ void restoreCommand(client *c) {
}
/* Make sure this key does not already exist here... */
- if (!replace && lookupKeyWrite(c->db,c->argv[1]) != NULL) {
+ robj *key = c->argv[1];
+ if (!replace && lookupKeyWrite(c->db,key) != NULL) {
addReply(c,shared.busykeyerr);
return;
}
@@ -4940,23 +4941,36 @@ void restoreCommand(client *c) {
rioInitWithBuffer(&payload,c->argv[3]->ptr);
if (((type = rdbLoadObjectType(&payload)) == -1) ||
- ((obj = rdbLoadObject(type,&payload,c->argv[1])) == NULL))
+ ((obj = rdbLoadObject(type,&payload,key)) == NULL))
{
addReplyError(c,"Bad data format");
return;
}
/* Remove the old key if needed. */
- if (replace) dbDelete(c->db,c->argv[1]);
+ int deleted = 0;
+ if (replace)
+ deleted = dbDelete(c->db,key);
+
+ if (ttl && !absttl) ttl+=mstime();
+ if (ttl && checkAlreadyExpired(ttl)) {
+ if (deleted) {
+ rewriteClientCommandVector(c,2,shared.del,key);
+ signalModifiedKey(c->db,key);
+ notifyKeyspaceEvent(NOTIFY_GENERIC,"del",key,c->db->id);
+ server.dirty++;
+ }
+ addReply(c, shared.ok);
+ return;
+ }
/* Create the key and set the TTL if any */
- dbAdd(c->db,c->argv[1],obj);
+ dbAdd(c->db,key,obj);
if (ttl) {
- if (!absttl) ttl+=mstime();
- setExpire(c,c->db,c->argv[1],ttl);
+ setExpire(c,c->db,key,ttl);
}
objectSetLRUOrLFU(obj,lfu_freq,lru_idle,lru_clock);
- signalModifiedKey(c->db,c->argv[1]);
+ signalModifiedKey(c->db,key);
addReply(c,shared.ok);
server.dirty++;
}
diff --git a/src/expire.c b/src/expire.c
index 0b92ee3fe..39241aab7 100644
--- a/src/expire.c
+++ b/src/expire.c
@@ -391,6 +391,16 @@ void flushSlaveKeysWithExpireList(void) {
}
}
+int checkAlreadyExpired(long long when) {
+ /* EXPIRE with negative TTL, or EXPIREAT with a timestamp into the past
+ * should never be executed as a DEL when load the AOF or in the context
+ * of a slave instance.
+ *
+ * Instead we add the already expired key to the database with expire time
+ * (possibly in the past) and wait for an explicit DEL from the master. */
+ return (when <= mstime() && !server.loading && !server.masterhost);
+}
+
/*-----------------------------------------------------------------------------
* Expires Commands
*----------------------------------------------------------------------------*/
@@ -418,13 +428,7 @@ void expireGenericCommand(client *c, long long basetime, int unit) {
return;
}
- /* EXPIRE with negative TTL, or EXPIREAT with a timestamp into the past
- * should never be executed as a DEL when load the AOF or in the context
- * of a slave instance.
- *
- * Instead we take the other branch of the IF statement setting an expire
- * (possibly in the past) and wait for an explicit DEL from the master. */
- if (when <= mstime() && !server.loading && !server.masterhost) {
+ if (checkAlreadyExpired(when)) {
robj *aux;
int deleted = server.lazyfree_lazy_expire ? dbAsyncDelete(c->db,key) :
diff --git a/src/server.h b/src/server.h
index 9ad603dce..a905c0e28 100644
--- a/src/server.h
+++ b/src/server.h
@@ -1834,6 +1834,7 @@ void propagateExpire(redisDb *db, robj *key, int lazy);
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);
diff --git a/tests/unit/dump.tcl b/tests/unit/dump.tcl
index 09768b80e..4c4e5d075 100644
--- a/tests/unit/dump.tcl
+++ b/tests/unit/dump.tcl
@@ -36,7 +36,18 @@ start_server {tags {"dump"}} {
assert {$ttl >= 2900 && $ttl <= 3100}
r get foo
} {bar}
-
+
+ test {RESTORE with ABSTTL in the past} {
+ r set foo bar
+ set encoded [r dump foo]
+ set now [clock milliseconds]
+ r debug set-active-expire 0
+ r restore foo [expr $now-3000] $encoded absttl REPLACE
+ catch {r debug object foo} e
+ r debug set-active-expire 1
+ set e
+ } {ERR no such key}
+
test {RESTORE can set LRU} {
r set foo bar
set encoded [r dump foo]