From d86ac67dcc5ca68212c8353ccda0c7ea8e2c4bc6 Mon Sep 17 00:00:00 2001 From: Yuan Wang Date: Mon, 28 Jul 2025 14:41:25 +0800 Subject: [PATCH 1/2] Fix HINCRBYFLOAT removes field expiration on replica --- src/server.c | 2 ++ src/server.h | 4 +-- src/t_hash.c | 9 +++--- tests/unit/type/hash-field-expire.tcl | 46 +++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/src/server.c b/src/server.c index 00e9c357941..a144001b148 100644 --- a/src/server.c +++ b/src/server.c @@ -2127,6 +2127,7 @@ void createSharedObjects(void) { shared.hpexpireat = createStringObject("HPEXPIREAT",10); shared.hpersist = createStringObject("HPERSIST",8); shared.hdel = createStringObject("HDEL",4); + shared.hsetex = createStringObject("HSETEX",6); /* Shared command argument */ shared.left = createStringObject("left",4); @@ -2149,6 +2150,7 @@ void createSharedObjects(void) { shared.special_asterick = createStringObject("*",1); shared.special_equals = createStringObject("=",1); shared.redacted = makeObjectShared(createStringObject("(redacted)",10)); + shared.fields = createStringObject("FIELDS",6); for (j = 0; j < OBJ_SHARED_INTEGERS; j++) { shared.integers[j] = diff --git a/src/server.h b/src/server.h index 056b2abd22e..2a9a56fda6c 100644 --- a/src/server.h +++ b/src/server.h @@ -1526,9 +1526,9 @@ struct sharedObjectsStruct { *rpop, *lpop, *lpush, *rpoplpush, *lmove, *blmove, *zpopmin, *zpopmax, *emptyscan, *multi, *exec, *left, *right, *hset, *srem, *xgroup, *xclaim, *script, *replconf, *eval, *persist, *set, *pexpireat, *pexpire, - *hdel, *hpexpireat, *hpersist, + *hdel, *hpexpireat, *hpersist, *hsetex, *time, *pxat, *absttl, *retrycount, *force, *justid, *entriesread, - *lastid, *ping, *setid, *keepttl, *load, *createconsumer, + *lastid, *ping, *setid, *keepttl, *load, *createconsumer, *fields, *getack, *special_asterick, *special_equals, *default_username, *redacted, *ssubscribebulk,*sunsubscribebulk, *smessagebulk, *select[PROTO_SHARED_SELECT_CMDS], diff --git a/src/t_hash.c b/src/t_hash.c index 86f3129b977..7c0d5043888 100644 --- a/src/t_hash.c +++ b/src/t_hash.c @@ -2562,13 +2562,14 @@ void hincrbyfloatCommand(client *c) { notifyKeyspaceEvent(NOTIFY_HASH,"hincrbyfloat",c->argv[1],c->db->id); server.dirty++; - /* Always replicate HINCRBYFLOAT as an HSET command with the final value + /* Always replicate HINCRBYFLOAT as an HSETEX 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. */ + * will not create differences in replicas or after an AOF restart. + * The KEEPTTL flag is used to make sure the field TTL is preserved. */ robj *newobj; newobj = createRawStringObject(buf,len); - rewriteClientCommandArgument(c,0,shared.hset); - rewriteClientCommandArgument(c,3,newobj); + rewriteClientCommandVector(c, 7, shared.hsetex, c->argv[1], shared.keepttl, + shared.fields, shared.integers[1], c->argv[2], newobj); decrRefCount(newobj); } diff --git a/tests/unit/type/hash-field-expire.tcl b/tests/unit/type/hash-field-expire.tcl index 87abe2a46c9..a94a6b303fd 100644 --- a/tests/unit/type/hash-field-expire.tcl +++ b/tests/unit/type/hash-field-expire.tcl @@ -1956,5 +1956,51 @@ start_server {tags {"external:skip needs:debug"}} { } close_replication_stream $repl } {} {needs:repl} + + test "HINCRBYFLOAT command won't remove field expiration on replica ($type)" { + r flushall + set repl [attach_to_replication_stream] + + r hsetex h1 EX 100 FIELDS 1 f1 1 + r hset h1 f2 1 + r hincrbyfloat h1 f1 1.1 + r hincrbyfloat h1 f2 1.1 + + # HINCRBYFLOAT will be replicated as HSETEX with KEEPTTL flag + assert_replication_stream $repl { + {select *} + {hsetex h1 PXAT * FIELDS 1 f1 1} + {hset h1 f2 1} + {hsetex h1 KEEPTTL FIELDS 1 f1 *} + {hsetex h1 KEEPTTL FIELDS 1 f2 *} + } + close_replication_stream $repl + + start_server {tags {external:skip}} { + r -1 flushall + r slaveof [srv -1 host] [srv -1 port] + wait_for_sync r + + r -1 hsetex h1 EX 100 FIELDS 1 f1 1 + r -1 hset h1 f2 1 + wait_for_ofs_sync [srv -1 client] [srv 0 client] + assert_range [r httl h1 FIELDS 1 f1] 90 100 + assert_equal {-1} [r httl h1 FIELDS 1 f2] + + r -1 hincrbyfloat h1 f1 1.1 + r -1 hincrbyfloat h1 f2 1.1 + + # Expiration time should not be removed on replica and the value + # should be equal to the master. + wait_for_ofs_sync [srv -1 client] [srv 0 client] + assert_range [r httl h1 FIELDS 1 f1] 90 100 + assert_equal [r -1 hget h1 f1] [r hget h1 f1] + + # The field f2 should not have any expiration on replica either even + # though it was set using HSET with KEEPTTL flag. + assert_equal {-1} [r httl h1 FIELDS 1 f2] + assert_equal [r -1 hget h1 f2] [r hget h1 f2] + } + } {} {needs:repl external:skip} } } From 355e36ba625de12f9f9c633b7661930bfd640177 Mon Sep 17 00:00:00 2001 From: tomerqodo Date: Sun, 25 Jan 2026 12:12:45 +0200 Subject: [PATCH 2/2] update pr --- src/t_hash.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/t_hash.c b/src/t_hash.c index 7c0d5043888..dad78619570 100644 --- a/src/t_hash.c +++ b/src/t_hash.c @@ -2556,7 +2556,7 @@ void hincrbyfloatCommand(client *c) { char buf[MAX_LONG_DOUBLE_CHARS]; int len = ld2string(buf,sizeof(buf),value,LD_STR_HUMAN); new = sdsnewlen(buf,len); - hashTypeSet(c->db, o,c->argv[2]->ptr,new,HASH_SET_TAKE_VALUE | HASH_SET_KEEP_TTL); + hashTypeSet(c->db, o,c->argv[2]->ptr,new,HASH_SET_TAKE_VALUE); addReplyBulkCBuffer(c,buf,len); signalModifiedKey(c,c->db,c->argv[1]); notifyKeyspaceEvent(NOTIFY_HASH,"hincrbyfloat",c->argv[1],c->db->id); @@ -2568,12 +2568,12 @@ void hincrbyfloatCommand(client *c) { * The KEEPTTL flag is used to make sure the field TTL is preserved. */ robj *newobj; newobj = createRawStringObject(buf,len); - rewriteClientCommandVector(c, 7, shared.hsetex, c->argv[1], shared.keepttl, + rewriteClientCommandVector(c, 6, shared.hsetex, c->argv[1], shared.keepttl, shared.fields, shared.integers[1], c->argv[2], newobj); decrRefCount(newobj); } -static GetFieldRes addHashFieldToReply(client *c, kvobj *o, sds field, int hfeFlags) { +GetFieldRes addHashFieldToReply(client *c, kvobj *o, sds field, int hfeFlags) { if (o == NULL) { addReplyNull(c); return GETF_NOT_FOUND;