From 24044f33560e9c34e73d8ffc493ed7c0b6b95dbc Mon Sep 17 00:00:00 2001 From: "zhaozhao.zz" Date: Wed, 18 Dec 2019 14:49:38 +0800 Subject: [PATCH 1/2] add a new SET option KEEPTTL that doesn't remove expire time --- src/bitops.c | 2 +- src/db.c | 4 ++-- src/geo.c | 2 +- src/module.c | 6 +++--- src/server.h | 2 +- src/sort.c | 2 +- src/t_string.c | 25 ++++++++++++++++--------- tests/unit/expire.tcl | 13 +++++++++++++ 8 files changed, 38 insertions(+), 18 deletions(-) diff --git a/src/bitops.c b/src/bitops.c index ee1ce0460..dcd1f27a5 100644 --- a/src/bitops.c +++ b/src/bitops.c @@ -754,7 +754,7 @@ void bitopCommand(client *c) { /* Store the computed value into the target key */ if (maxlen) { o = createObject(OBJ_STRING,res); - setKey(c->db,targetkey,o); + setKey(c->db,targetkey,o,0); notifyKeyspaceEvent(NOTIFY_STRING,"set",targetkey,c->db->id); decrRefCount(o); } else if (dbDelete(c->db,targetkey)) { diff --git a/src/db.c b/src/db.c index 483095a7c..17834f37f 100644 --- a/src/db.c +++ b/src/db.c @@ -219,14 +219,14 @@ void dbOverwrite(redisDb *db, robj *key, robj *val) { * 3) The expire time of the key is reset (the key is made persistent). * * All the new keys in the database should be created via this interface. */ -void setKey(redisDb *db, robj *key, robj *val) { +void setKey(redisDb *db, robj *key, robj *val, int keepttl) { if (lookupKeyWrite(db,key) == NULL) { dbAdd(db,key,val); } else { dbOverwrite(db,key,val); } incrRefCount(val); - removeExpire(db,key); + if (!keepttl) removeExpire(db,key); signalModifiedKey(db,key); } diff --git a/src/geo.c b/src/geo.c index 049335a4f..5d969ad98 100644 --- a/src/geo.c +++ b/src/geo.c @@ -657,7 +657,7 @@ void georadiusGeneric(client *c, int flags) { if (returned_items) { zsetConvertToZiplistIfNeeded(zobj,maxelelen); - setKey(c->db,storekey,zobj); + setKey(c->db,storekey,zobj,0); decrRefCount(zobj); notifyKeyspaceEvent(NOTIFY_ZSET,"georadiusstore",storekey, c->db->id); diff --git a/src/module.c b/src/module.c index 1fda29625..3bfde5cd4 100644 --- a/src/module.c +++ b/src/module.c @@ -2107,7 +2107,7 @@ RedisModuleString *RM_RandomKey(RedisModuleCtx *ctx) { int RM_StringSet(RedisModuleKey *key, RedisModuleString *str) { if (!(key->mode & REDISMODULE_WRITE) || key->iter) return REDISMODULE_ERR; RM_DeleteKey(key); - setKey(key->db,key->key,str); + setKey(key->db,key->key,str,0); key->value = str; return REDISMODULE_OK; } @@ -2187,7 +2187,7 @@ int RM_StringTruncate(RedisModuleKey *key, size_t newlen) { if (key->value == NULL) { /* Empty key: create it with the new size. */ robj *o = createObject(OBJ_STRING,sdsnewlen(NULL, newlen)); - setKey(key->db,key->key,o); + setKey(key->db,key->key,o,0); key->value = o; decrRefCount(o); } else { @@ -3571,7 +3571,7 @@ int RM_ModuleTypeSetValue(RedisModuleKey *key, moduleType *mt, void *value) { if (!(key->mode & REDISMODULE_WRITE) || key->iter) return REDISMODULE_ERR; RM_DeleteKey(key); robj *o = createModuleObject(mt,value); - setKey(key->db,key->key,o); + setKey(key->db,key->key,o,0); decrRefCount(o); key->value = o; return REDISMODULE_OK; diff --git a/src/server.h b/src/server.h index 7a78c884f..655ac4f12 100644 --- a/src/server.h +++ b/src/server.h @@ -2025,7 +2025,7 @@ int objectSetLRUOrLFU(robj *val, long long lfu_freq, long long lru_idle, #define LOOKUP_NOTOUCH (1<<0) void dbAdd(redisDb *db, robj *key, robj *val); void dbOverwrite(redisDb *db, robj *key, robj *val); -void setKey(redisDb *db, robj *key, robj *val); +void setKey(redisDb *db, robj *key, robj *val, int keepttl); int dbExists(redisDb *db, robj *key); robj *dbRandomKey(redisDb *db); int dbSyncDelete(redisDb *db, robj *key); diff --git a/src/sort.c b/src/sort.c index db26da158..c8666d6f5 100644 --- a/src/sort.c +++ b/src/sort.c @@ -570,7 +570,7 @@ void sortCommand(client *c) { } } if (outputlen) { - setKey(c->db,storekey,sobj); + setKey(c->db,storekey,sobj,0); notifyKeyspaceEvent(NOTIFY_LIST,"sortstore",storekey, c->db->id); server.dirty += outputlen; diff --git a/src/t_string.c b/src/t_string.c index 5800c5c0c..a4c4a12cc 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -46,7 +46,7 @@ static int checkStringLength(client *c, long long size) { * options and variants. This function is called in order to implement the * following commands: SET, SETEX, PSETEX, SETNX. * - * 'flags' changes the behavior of the command (NX or XX, see belove). + * 'flags' changes the behavior of the command (NX or XX, see below). * * 'expire' represents an expire to set in form of a Redis object as passed * by the user. It is interpreted according to the specified 'unit'. @@ -59,10 +59,11 @@ static int checkStringLength(client *c, long long size) { * If abort_reply is NULL, "$-1" is used. */ #define OBJ_SET_NO_FLAGS 0 -#define OBJ_SET_NX (1<<0) /* Set if key not exists. */ -#define OBJ_SET_XX (1<<1) /* Set if key exists. */ -#define OBJ_SET_EX (1<<2) /* Set if time in seconds is given */ -#define OBJ_SET_PX (1<<3) /* Set if time in ms in given */ +#define OBJ_SET_NX (1<<0) /* Set if key not exists. */ +#define OBJ_SET_XX (1<<1) /* Set if key exists. */ +#define OBJ_SET_EX (1<<2) /* Set if time in seconds is given */ +#define OBJ_SET_PX (1<<3) /* Set if time in ms in given */ +#define OBJ_SET_KEEPTTL (1<<4) /* Set and keep the ttl */ void setGenericCommand(client *c, int flags, robj *key, robj *val, robj *expire, int unit, robj *ok_reply, robj *abort_reply) { long long milliseconds = 0; /* initialized to avoid any harmness warning */ @@ -83,7 +84,7 @@ void setGenericCommand(client *c, int flags, robj *key, robj *val, robj *expire, addReply(c, abort_reply ? abort_reply : shared.null[c->resp]); return; } - setKey(c->db,key,val); + setKey(c->db,key,val,flags & OBJ_SET_KEEPTTL); server.dirty++; if (expire) setExpire(c,c->db,key,mstime()+milliseconds); notifyKeyspaceEvent(NOTIFY_STRING,"set",key,c->db->id); @@ -92,7 +93,7 @@ void setGenericCommand(client *c, int flags, robj *key, robj *val, robj *expire, addReply(c, ok_reply ? ok_reply : shared.ok); } -/* SET key value [NX] [XX] [EX ] [PX ] */ +/* SET key value [NX] [XX] [KEEPTTL] [EX ] [PX ] */ void setCommand(client *c) { int j; robj *expire = NULL; @@ -113,8 +114,13 @@ void setCommand(client *c) { !(flags & OBJ_SET_NX)) { flags |= OBJ_SET_XX; + } else if (!strcasecmp(c->argv[j]->ptr,"KEEPTTL") && + !(flags & OBJ_SET_EX) && !(flags & OBJ_SET_PX)) + { + flags |= OBJ_SET_KEEPTTL; } else if ((a[0] == 'e' || a[0] == 'E') && (a[1] == 'x' || a[1] == 'X') && a[2] == '\0' && + !(flags & OBJ_SET_KEEPTTL) && !(flags & OBJ_SET_PX) && next) { flags |= OBJ_SET_EX; @@ -123,6 +129,7 @@ void setCommand(client *c) { j++; } else if ((a[0] == 'p' || a[0] == 'P') && (a[1] == 'x' || a[1] == 'X') && a[2] == '\0' && + !(flags & OBJ_SET_KEEPTTL) && !(flags & OBJ_SET_EX) && next) { flags |= OBJ_SET_PX; @@ -176,7 +183,7 @@ void getCommand(client *c) { void getsetCommand(client *c) { if (getGenericCommand(c) == C_ERR) return; c->argv[2] = tryObjectEncoding(c->argv[2]); - setKey(c->db,c->argv[1],c->argv[2]); + setKey(c->db,c->argv[1],c->argv[2],0); notifyKeyspaceEvent(NOTIFY_STRING,"set",c->argv[1],c->db->id); server.dirty++; } @@ -321,7 +328,7 @@ void msetGenericCommand(client *c, int nx) { for (j = 1; j < c->argc; j += 2) { c->argv[j+1] = tryObjectEncoding(c->argv[j+1]); - setKey(c->db,c->argv[j],c->argv[j+1]); + setKey(c->db,c->argv[j],c->argv[j+1],0); notifyKeyspaceEvent(NOTIFY_STRING,"set",c->argv[j],c->db->id); } server.dirty += (c->argc-1)/2; diff --git a/tests/unit/expire.tcl b/tests/unit/expire.tcl index de24eabed..11fb82ef0 100644 --- a/tests/unit/expire.tcl +++ b/tests/unit/expire.tcl @@ -219,4 +219,17 @@ start_server {tags {"expire"}} { set ttl [r ttl foo] assert {$ttl <= 98 && $ttl > 90} } + + test {SET command will remove expire} { + r set foo bar EX 100 + r set foo bar + r ttl foo + } {-1} + + test {SET - use KEEPTTL option, TTL should not be removed} { + r set foo bar EX 100 + r set foo bar KEEPTTL + set ttl [r ttl foo] + assert {$ttl <= 100 && $ttl > 90} + } } From 58554396d6d49b2c80fcf61ce5c2ccfdc550c7c2 Mon Sep 17 00:00:00 2001 From: "zhaozhao.zz" Date: Wed, 18 Dec 2019 15:44:51 +0800 Subject: [PATCH 2/2] incrbyfloat: fix issue #5256 ttl lost after propagate --- src/t_string.c | 11 +++++++---- tests/integration/replication.tcl | 7 +++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/t_string.c b/src/t_string.c index a4c4a12cc..4ceb02fe3 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -405,7 +405,7 @@ void decrbyCommand(client *c) { void incrbyfloatCommand(client *c) { long double incr, value; - robj *o, *new, *aux; + robj *o, *new, *aux1, *aux2; o = lookupKeyWrite(c->db,c->argv[1]); if (o != NULL && checkType(c,o,OBJ_STRING)) return; @@ -431,10 +431,13 @@ void incrbyfloatCommand(client *c) { /* Always replicate INCRBYFLOAT as a SET command with the final value * in order to make sure that differences in float precision or formatting * will not create differences in replicas or after an AOF restart. */ - aux = createStringObject("SET",3); - rewriteClientCommandArgument(c,0,aux); - decrRefCount(aux); + aux1 = createStringObject("SET",3); + rewriteClientCommandArgument(c,0,aux1); + decrRefCount(aux1); rewriteClientCommandArgument(c,2,new); + aux2 = createStringObject("KEEPTTL",7); + rewriteClientCommandArgument(c,3,aux2); + decrRefCount(aux2); } void appendCommand(client *c) { diff --git a/tests/integration/replication.tcl b/tests/integration/replication.tcl index 4bd1f47f7..f69002b36 100644 --- a/tests/integration/replication.tcl +++ b/tests/integration/replication.tcl @@ -70,6 +70,13 @@ start_server {tags {"repl"}} { } } + test {INCRBYFLOAT replication, should not remove expire} { + r set test 1 EX 100 + r incrbyfloat test 0.1 + after 1000 + assert_equal [$A debug digest] [$B debug digest] + } + test {BRPOPLPUSH replication, when blocking against empty list} { set rd [redis_deferring_client] $rd brpoplpush a b 5