HFE to support AOF and replicas #13285

moticless · 2024-05-21T12:09:09Z

For replica sake, rewrite commands H*EXPIRE* , HSETF, HGETF to have absolute unix time in msec.
On active-expiration of field, propagate HDEL to replica (propagateHashFieldDeletion())
On lazy-expiration, propagate HDEL to replica (hashTypeGetValue() now calls hashTypeDelete(). It also takes care to call propagateHashFieldDeletion()).
Fix H*EXPIRE* command such that if it gets flag LT and it doesn’t have any expiration on the field then it will considered as valid condition.

Note, replicas doesn’t make any active expiration, and should avoid lazy expiration. On hashTypeGetValue() it doesn't check expiration (As long as the master didn’t request to delete the field, it is valid)

TODO:

Attach dbid to HASH metadata. See here

src/aof.c

src/server.h

src/t_hash.c

sundb · 2024-05-23T05:08:54Z

@moticless should the TODO in https://github.com/redis/redis/blob/a25b15392c3f50909ab2dafbcbdb3794ada29620/src/rdb.c#L2312:L2315 be handled here?

src/t_hash.c

tests/unit/type/hash-field-expire.tcl

src/t_hash.c

tests/support/util.tcl

src/t_hash.c

moticless · 2024-05-23T11:54:15Z

@moticless should the TODO in https://github.com/redis/redis/blob/a25b15392c3f50909ab2dafbcbdb3794ada29620/src/rdb.c#L2312:L2315 be handled here?

I will handle it on the next commit.

src/module.c

src/t_hash.c

src/aof.c

tests/unit/type/hash-field-expire.tcl

src/t_hash.c

src/aof.c

src/t_hash.c

src/rdb.c

src/aof.c

tezc · 2024-05-27T14:40:46Z

src/t_hash.c

-    /* Delete all the expired fields in one go */
-    if (r.expired > 0)
-        lpt->lp = lpDeleteRange(lpt->lp, 0, r.expired * 3);
+        ptr = lpNext(lpt->lp, ptr);


Just a note to all of us, maybe we can implement this logic with lpFindCb() in future to avoid performance cost of lpNext() calls.

sundb · 2024-05-28T02:48:18Z

src/module.c

+            int isHashDeleted;
+            int exists = hashTypeExists(key->db, key->value, field->ptr, &isHashDeleted);
+            /* hash-field-expiration is not exposed to modules */
+            serverAssert(isHashDeleted == 0);


if i use hexpire command to create a hash with TTL fields, then operate the hash with RM_HashSet, will thia assertion be triggered?

Regarding this matter, I am seeking input from those experienced with modules:

We likely need to pass the mode/flags of the opened key and respond appropriately.

Should we update the opened key if the key was deleted due to last field expiration?

@MeirShpilraien, perhaps you can assist. Thanks.

src/t_hash.c

tezc · 2024-05-28T08:11:28Z

src/t_hash.c

+        shared.hdel,
+        createStringObject((char*) key, sdslen(key)),
+        createStringObject(field, fieldLen)
+    };


Note: Later, maybe we can add a check to avoid creating these objects if we are not going to propagate anything.

tests/integration/psync2-master-restart.tcl

HFE to support AOF and replicas

26d050a

moticless requested review from tezc, sundb and oranagra May 21, 2024 12:13

fix flaky test

9b5bd8f

sundb reviewed May 21, 2024

View reviewed changes

src/aof.c Outdated Show resolved Hide resolved

sundb reviewed May 22, 2024

View reviewed changes

src/server.h Outdated Show resolved Hide resolved

src/t_hash.c Outdated Show resolved Hide resolved

src/t_hash.c Show resolved Hide resolved

Merge branch 'hash-field-expiry-integ' into hfe-integ-aof-and-replica

ef1b9e9