diff --git a/src/cluster.c b/src/cluster.c index 566ac5a19..edca39585 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -262,6 +262,8 @@ void restoreCommand(client *c) { if (ttl && !absttl) ttl += commandTimeSnapshot(); if (ttl && checkAlreadyExpired(ttl)) { if (deleted) { + /* Here we don't use deleteExpiredKeyFromOverwriteAndPropagate because + * strictly speaking, the `delete` is triggered by the `replace`. */ robj *aux = server.lazyfree_lazy_server_del ? shared.unlink : shared.del; rewriteClientCommandVector(c, 2, aux, key); signalModifiedKey(c, c->db, key); diff --git a/src/db.c b/src/db.c index 4a2c0a495..0e8661f02 100644 --- a/src/db.c +++ b/src/db.c @@ -1712,6 +1712,19 @@ void deleteExpiredKeyAndPropagate(serverDb *db, robj *keyobj) { server.stat_expiredkeys++; } +/* Delete the specified expired key from overwriting and propagate the DEL or UNLINK. */ +void deleteExpiredKeyFromOverwriteAndPropagate(client *c, robj *keyobj) { + int deleted = dbGenericDelete(c->db, keyobj, server.lazyfree_lazy_expire, DB_FLAG_KEY_EXPIRED); + serverAssertWithInfo(c, keyobj, deleted); + server.dirty++; + + /* Replicate/AOF this as an explicit DEL or UNLINK. */ + robj *aux = server.lazyfree_lazy_expire ? shared.unlink : shared.del; + rewriteClientCommandVector(c, 2, aux, keyobj); + signalModifiedKey(c, c->db, keyobj); + notifyKeyspaceEvent(NOTIFY_GENERIC, "del", keyobj, c->db->id); +} + /* Propagate an implicit key deletion into replicas and the AOF file. * When a key was deleted in the primary by eviction, expiration or a similar * mechanism a DEL/UNLINK operation for this key is sent diff --git a/src/expire.c b/src/expire.c index a9842ae12..97426e9e1 100644 --- a/src/expire.c +++ b/src/expire.c @@ -668,17 +668,7 @@ void expireGenericCommand(client *c, long long basetime, int unit) { } if (checkAlreadyExpired(when)) { - robj *aux; - - int deleted = dbGenericDelete(c->db, key, server.lazyfree_lazy_expire, DB_FLAG_KEY_EXPIRED); - serverAssertWithInfo(c, key, deleted); - server.dirty++; - - /* Replicate/AOF this as an explicit DEL or UNLINK. */ - aux = server.lazyfree_lazy_expire ? shared.unlink : shared.del; - rewriteClientCommandVector(c, 2, aux, key); - signalModifiedKey(c, c->db, key); - notifyKeyspaceEvent(NOTIFY_GENERIC, "del", key, c->db->id); + deleteExpiredKeyFromOverwriteAndPropagate(c, key); addReply(c, shared.cone); return; } else { diff --git a/src/server.h b/src/server.h index fc806968d..e34fc680e 100644 --- a/src/server.h +++ b/src/server.h @@ -3505,6 +3505,7 @@ int setModuleNumericConfig(ModuleConfig *config, long long val, const char **err /* db.c -- Keyspace access API */ int removeExpire(serverDb *db, robj *key); void deleteExpiredKeyAndPropagate(serverDb *db, robj *keyobj); +void deleteExpiredKeyFromOverwriteAndPropagate(client *c, robj *keyobj); void propagateDeletion(serverDb *db, robj *key, int lazy); int keyIsExpired(serverDb *db, robj *key); long long getExpire(serverDb *db, robj *key); diff --git a/src/t_string.c b/src/t_string.c index 6e233e095..429fdfac4 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -109,6 +109,15 @@ void setGenericCommand(client *c, return; } + /* If the `milliseconds` have expired, then we don't need to set it into the + * database, and then wait for the active expire to delete it, it is wasteful. + * If the key already exists, delete it. */ + if (expire && checkAlreadyExpired(milliseconds)) { + if (found) deleteExpiredKeyFromOverwriteAndPropagate(c, key); + if (!(flags & OBJ_SET_GET)) addReply(c, shared.ok); + return; + } + /* When expire is not NULL, we avoid deleting the TTL so it can be updated later instead of being deleted and then * created again. */ setkey_flags |= ((flags & OBJ_KEEPTTL) || expire) ? SETKEY_KEEPTTL : 0; @@ -395,13 +404,7 @@ void getexCommand(client *c) { if (((flags & OBJ_PXAT) || (flags & OBJ_EXAT)) && checkAlreadyExpired(milliseconds)) { /* When PXAT/EXAT absolute timestamp is specified, there can be a chance that timestamp * has already elapsed so delete the key in that case. */ - int deleted = dbGenericDelete(c->db, c->argv[1], server.lazyfree_lazy_expire, DB_FLAG_KEY_EXPIRED); - serverAssert(deleted); - robj *aux = server.lazyfree_lazy_expire ? shared.unlink : shared.del; - rewriteClientCommandVector(c,2,aux,c->argv[1]); - signalModifiedKey(c, c->db, c->argv[1]); - notifyKeyspaceEvent(NOTIFY_GENERIC, "del", c->argv[1], c->db->id); - server.dirty++; + deleteExpiredKeyFromOverwriteAndPropagate(c, c->argv[1]); } else if (expire) { setExpire(c,c->db,c->argv[1],milliseconds); /* Propagate as PXEXPIREAT millisecond-timestamp if there is diff --git a/tests/unit/type/string.tcl b/tests/unit/type/string.tcl index 381cc4a69..6d273498a 100644 --- a/tests/unit/type/string.tcl +++ b/tests/unit/type/string.tcl @@ -607,6 +607,40 @@ if {[string match {*jemalloc*} [s mem_allocator]]} { r set foo bar pxat [expr [clock milliseconds] + 10000] assert_range [r ttl foo] 5 10 } + + test "SET EXAT / PXAT Expiration time is expired" { + r debug set-active-expire 0 + set repl [attach_to_replication_stream] + + # Key exists. + r set foo bar + r set foo bar exat [expr [clock seconds] - 100] + assert_error {ERR no such key} {r debug object foo} + r set foo bar + r set foo bar pxat [expr [clock milliseconds] - 10000] + assert_error {ERR no such key} {r debug object foo} + + # Key does not exist. + r del foo + r set foo bar exat [expr [clock seconds] - 100] + assert_error {ERR no such key} {r debug object foo} + r set foo bar pxat [expr [clock milliseconds] - 10000] + assert_error {ERR no such key} {r debug object foo} + + r incr foo + assert_replication_stream $repl { + {select *} + {set foo bar} + {del foo} + {set foo bar} + {del foo} + {incr foo} + } + + r debug set-active-expire 1 + close_replication_stream $repl + } {} {needs:debug needs:repl} + test {Extended SET using multiple options at once} { r set foo val assert {[r set foo bar xx px 10000] eq {OK}}