From ce121b92619d87284b5bc654f86605cb802f0b8e Mon Sep 17 00:00:00 2001
From: Moti Cohen <moticless@gmail.com>
Date: Mon, 10 Jun 2024 11:24:26 +0300
Subject: [PATCH 01/14] HFE - Avoid lazy expire if called by modules + cleanup
 (#13326)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Need to be carefull if called by modules since modules API allow to open
and close key handler. We don't want to invalidate the handler
underneath.

* hashTypeExists(), hashTypeGetValueObject() - will return the logical
state of the field. A flag will indicate noExpire.
* RM_HashGet() - Will get NULL if the field expired. Fields won’t be
deleted.
* RM_ScanKey() - might return 0 items if all fields got expired. Fields
won’t be deleted.
* RM_HashSet() - If set, then override expired field. If delete, we can
either delete or leave it to active-expiration. XX/NX - logically
correct (Verify with tests).

Nice to have (not implemented):
* RedisModule_CloseKey() - We can local active-expire up-to 100 items.

Note:
Length will be wrong to modules just like redis (Count expired fields).
---
 src/module.c                          |  77 ++--
 src/server.h                          |   9 +-
 src/sort.c                            |   2 +-
 src/t_hash.c                          | 594 ++++++++++++--------------
 tests/unit/moduleapi/hash.tcl         |  39 ++
 tests/unit/moduleapi/scan.tcl         |  22 +-
 tests/unit/type/hash-field-expire.tcl |   8 +-
 7 files changed, 391 insertions(+), 360 deletions(-)

diff --git a/src/module.c b/src/module.c
index 81f726602..a279d4029 100644
--- a/src/module.c
+++ b/src/module.c
@@ -5271,10 +5271,21 @@ int RM_HashSet(RedisModuleKey *key, int flags, ...) {
 
         /* Handle XX and NX */
         if (flags & (REDISMODULE_HASH_XX|REDISMODULE_HASH_NX)) {
-            int isHashDeleted;
-            int exists = hashTypeExists(key->db, key->value, field->ptr, &isHashDeleted);
-            /* hash-field-expiration is not exposed to modules */
-            serverAssert(isHashDeleted == 0);
+            int hfeFlags = HFE_LAZY_AVOID_HASH_DEL; /* Avoid invalidate the key */
+
+            /*
+             * The hash might contain expired fields. If we lazily delete expired
+             * field and the command was sent with XX flag, the operation could
+             * fail and leave the hash empty, which the caller might not expect.
+             * To prevent unexpected behavior, we avoid lazy deletion in this case
+             * yet let the operation fail. Note that moduleDelKeyIfEmpty()
+             * below won't delete the hash if it left with single expired key
+             * because hash counts blindly expired fields as well.
+             */
+            if (flags & REDISMODULE_HASH_XX)
+                hfeFlags |= HFE_LAZY_AVOID_FIELD_DEL;
+
+            int exists = hashTypeExists(key->db, key->value, field->ptr, hfeFlags, NULL);
             if (((flags & REDISMODULE_HASH_XX) && !exists) ||
                 ((flags & REDISMODULE_HASH_NX) && exists))
             {
@@ -5357,6 +5368,7 @@ int RM_HashSet(RedisModuleKey *key, int flags, ...) {
  * RedisModule_FreeString(), or by enabling automatic memory management.
  */
 int RM_HashGet(RedisModuleKey *key, int flags, ...) {
+    int hfeFlags = HFE_LAZY_AVOID_FIELD_DEL | HFE_LAZY_AVOID_HASH_DEL;
     va_list ap;
     if (key->value && key->value->type != OBJ_HASH) return REDISMODULE_ERR;
 
@@ -5378,21 +5390,16 @@ int RM_HashGet(RedisModuleKey *key, int flags, ...) {
         if (flags & REDISMODULE_HASH_EXISTS) {
             existsptr = va_arg(ap,int*);
             if (key->value) {
-                int isHashDeleted;
-                *existsptr = hashTypeExists(key->db, key->value, field->ptr, &isHashDeleted);
-                /* hash-field-expiration is not exposed to modules */
-                serverAssert(isHashDeleted == 0);
+                *existsptr = hashTypeExists(key->db, key->value, field->ptr, hfeFlags, NULL);
             } else {
                 *existsptr = 0;
             }
         } else {
-            int isHashDeleted;
             valueptr = va_arg(ap,RedisModuleString**);
             if (key->value) {
-                *valueptr = hashTypeGetValueObject(key->db,key->value,field->ptr, &isHashDeleted);
+                *valueptr = hashTypeGetValueObject(key->db, key->value, field->ptr,
+                                                   hfeFlags, NULL);
 
-                /* Currently hash-field-expiration is not exposed to modules */
-                serverAssert(isHashDeleted == 0);
                 if (*valueptr) {
                     robj *decoded = getDecodedObject(*valueptr);
                     decrRefCount(*valueptr);
@@ -11080,6 +11087,11 @@ static void moduleScanKeyCallback(void *privdata, const dictEntry *de) {
         value = NULL;
     } else if (o->type == OBJ_HASH) {
         sds val = dictGetVal(de);
+
+        /* If field is expired, then ignore */
+        if (hfieldIsExpired(key))
+            return;
+
         field = createStringObject(key, hfieldlen(key));
         value = createStringObject(val, sdslen(val));
     } else if (o->type == OBJ_ZSET) {
@@ -11189,9 +11201,8 @@ int RM_ScanKey(RedisModuleKey *key, RedisModuleScanCursor *cursor, RedisModuleSc
         ret = 0;
     } else if (o->type == OBJ_ZSET || o->type == OBJ_HASH) {
         unsigned char *lp, *p;
-        unsigned char *vstr;
-        unsigned int vlen;
-        long long vll;
+        /* is hash with expiry on fields, then lp tuples are [field][value][expire] */
+        int hfe = o->type == OBJ_HASH && o->encoding == OBJ_ENCODING_LISTPACK_EX;
 
         if (o->type == OBJ_HASH)
             lp = hashTypeListpackGetLp(o);
@@ -11200,19 +11211,32 @@ int RM_ScanKey(RedisModuleKey *key, RedisModuleScanCursor *cursor, RedisModuleSc
 
         p = lpSeek(lp,0);
         while(p) {
-            vstr = lpGetValue(p,&vlen,&vll);
-            robj *field = (vstr != NULL) ?
-                createStringObject((char*)vstr,vlen) :
-                createStringObjectFromLongLongWithSds(vll);
+            long long vllField, vllValue, vllExpire;
+            unsigned int lenField, lenValue;
+            unsigned char *pField, *pValue;
+
+            pField = lpGetValue(p,&lenField,&vllField);
             p = lpNext(lp,p);
-            vstr = lpGetValue(p,&vlen,&vll);
-            robj *value = (vstr != NULL) ?
-                createStringObject((char*)vstr,vlen) :
-                createStringObjectFromLongLongWithSds(vll);
+            pValue = lpGetValue(p,&lenValue,&vllValue);
+            p = lpNext(lp,p);
+
+            if (hfe) {
+                serverAssert(lpGetIntegerValue(p, &vllExpire));
+                p = lpNext(lp, p);
+
+                /* Skip expired fields */
+                if (hashTypeIsExpired(o, vllExpire))
+                    continue;
+            }
+
+            robj *value = (pValue != NULL) ?
+                          createStringObject((char*)pValue,lenValue) :
+                          createStringObjectFromLongLongWithSds(vllValue);
+
+            robj *field = (pField != NULL) ?
+                          createStringObject((char*)pField,lenField) :
+                          createStringObjectFromLongLongWithSds(vllField);
             fn(key, field, value, privdata);
-            p = lpNext(lp,p);
-            if (o->type == OBJ_HASH && o->encoding == OBJ_ENCODING_LISTPACK_EX)
-                p = lpNext(lp, p); /* Skip expire time */
 
             decrRefCount(field);
             decrRefCount(value);
@@ -11225,7 +11249,6 @@ int RM_ScanKey(RedisModuleKey *key, RedisModuleScanCursor *cursor, RedisModuleSc
     return ret;
 }
 
-
 /* --------------------------------------------------------------------------
  * ## Module fork API
  * -------------------------------------------------------------------------- */
diff --git a/src/server.h b/src/server.h
index 39975ddb0..59bad41ab 100644
--- a/src/server.h
+++ b/src/server.h
@@ -3191,9 +3191,14 @@ typedef struct dictExpireMetadata {
 #define HASH_SET_TAKE_VALUE (1<<1)
 #define HASH_SET_COPY 0
 
+/* Hash field lazy expiration flags. Used by core hashTypeGetValue() and its callers */
+#define HFE_LAZY_EXPIRE           (0) /* Delete expired field, and if last field also the hash */
+#define HFE_LAZY_AVOID_FIELD_DEL  (1<<0) /* Avoid deleting expired field */
+#define HFE_LAZY_AVOID_HASH_DEL   (1<<1) /* Avoid deleting hash if the field is the last one */
+
 void hashTypeConvert(robj *o, int enc, ebuckets *hexpires);
 void hashTypeTryConversion(redisDb *db, robj *subject, robj **argv, int start, int end);
-int hashTypeExists(redisDb *db, robj *o, sds key, int *isHashDeleted);
+int hashTypeExists(redisDb *db, robj *o, sds key, int hfeFlags, int *isHashDeleted);
 int hashTypeDelete(robj *o, void *key, int isSdsField);
 unsigned long hashTypeLength(const robj *o, int subtractExpiredFields);
 hashTypeIterator *hashTypeInitIterator(robj *subject);
@@ -3210,7 +3215,7 @@ void hashTypeCurrentObject(hashTypeIterator *hi, int what, unsigned char **vstr,
                            unsigned int *vlen, long long *vll, uint64_t *expireTime);
 sds hashTypeCurrentObjectNewSds(hashTypeIterator *hi, int what);
 hfield hashTypeCurrentObjectNewHfield(hashTypeIterator *hi);
-robj *hashTypeGetValueObject(redisDb *db, robj *o, sds field, int *isHashDeleted);
+robj *hashTypeGetValueObject(redisDb *db, robj *o, sds field, int hfeFlags, int *isHashDeleted);
 int hashTypeSet(redisDb *db, robj *o, sds field, sds value, int flags);
 robj *hashTypeDup(robj *o, sds newkey, uint64_t *minHashExpire);
 uint64_t hashTypeRemoveFromExpires(ebuckets *hexpires, robj *o);
diff --git a/src/sort.c b/src/sort.c
index d45c380ac..2dcea1754 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -95,7 +95,7 @@ robj *lookupKeyByPattern(redisDb *db, robj *pattern, robj *subst) {
         /* Retrieve value from hash by the field name. The returned object
          * is a new object with refcount already incremented. */
         int isHashDeleted;
-        o = hashTypeGetValueObject(db, o, fieldobj->ptr, &isHashDeleted);
+        o = hashTypeGetValueObject(db, o, fieldobj->ptr, HFE_LAZY_EXPIRE, &isHashDeleted);
 
         if (isHashDeleted)
             goto noobj;
diff --git a/src/t_hash.c b/src/t_hash.c
index efb489265..afab548d7 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -21,7 +21,7 @@ typedef enum GetFieldRes {
     GETF_NOT_FOUND,         /* The field was not found. */
 
     /* used only by hashTypeGetValue() */
-    GETF_EXPIRED,           /* Logically expired but not yet deleted. */
+    GETF_EXPIRED,           /* Logically expired (Might be lazy deleted or not) */
     GETF_EXPIRED_HASH,      /* Delete hash since retrieved field was expired and
                              * it was the last field in the hash. */
 } GetFieldRes;
@@ -152,21 +152,18 @@ static inline int isDictWithMetaHFE(dict *d) {
 }
 
 /*-----------------------------------------------------------------------------
- * setex* - Set field OR field's expiration
+ * setex* - Set field's expiration
  *
- * Whereas setting plain fields is rather straightforward, setting expiration
- * time to fields might be time-consuming and complex since each update of
- * expiration time, not only updates `ebuckets` of corresponding hash, but also
- * might update `ebuckets` of global HFE DS. It is required to opt sequence of
- * field updates with expirartion for a given hash, such that only once done,
- * the global HFE DS will get updated.
+ * Setting expiration time to fields might be time-consuming and complex since
+ * each update of expiration time, not only updates `ebuckets` of corresponding
+ * hash, but also might update `ebuckets` of global HFE DS. It is required to opt
+ * sequence of field updates with expirartion for a given hash, such that only
+ * once done, the global HFE DS will get updated.
  *
  * To do so, follow the scheme:
  * 1. Call hashTypeSetExInit() to initialize the HashTypeSetEx struct.
  * 2. Call hashTypeSetEx() one time or more, for each field/expiration update.
  * 3. Call hashTypeSetExDone() for notification and update of global HFE.
- *
- * If expiration is not required, then avoid this API and use instead hashTypeSet()
  *----------------------------------------------------------------------------*/
 
 /* Returned value of hashTypeSetEx() */
@@ -178,10 +175,6 @@ typedef enum SetExRes {
     HSETEX_NO_FIELD =         -2,   /* No such hash-field */
     HSETEX_NO_CONDITION_MET =  0,   /* Specified NX | XX | GT | LT condition not met */
     HSETEX_DELETED =           2,   /* Field deleted because the specified time is in the past */
-
-    /* If not provided HashTypeSetEx struct to hashTypeSetEx() (plain HSET) */
-    HSET_UPDATE =              4,   /* Update of the field without expiration time */
-
 } SetExRes;
 
 /* Used by httlGenericCommand() */
@@ -190,14 +183,6 @@ typedef enum GetExpireTimeRes {
     HFE_GET_NO_TTL =            -1, /* No TTL attached to the field */
 } GetExpireTimeRes;
 
-/* on fail return HSETEX_NO_CONDITION_MET */
-typedef enum FieldSetCond {
-    FIELD_CREATE_OR_OVRWRT = 0,
-    FIELD_DONT_CREATE      = 1,
-    FIELD_DONT_CREATE2     = 2,     /* on fail return HSETEX_NO_FIELD */
-    FIELD_DONT_OVRWRT      = 3
-} FieldSetCond;
-
 typedef enum FieldGet { /* TBD */
     FIELD_GET_NONE = 0,
     FIELD_GET_NEW  = 1,
@@ -220,7 +205,6 @@ typedef struct HashTypeSet {
 typedef struct HashTypeSetEx {
 
     /*** config ***/
-    FieldSetCond fieldSetCond;          /* [DCF | DOF] */
     ExpireSetCond expireSetCond;        /* [XX | NX | GT | LT] */
 
     /*** metadata ***/
@@ -239,14 +223,10 @@ typedef struct HashTypeSetEx {
     const char *cmd;
 } HashTypeSetEx;
 
-static SetExRes hashTypeSetExListpack(redisDb *db, robj *o, sds field, HashTypeSet *setParams,
-                                      uint64_t expireAt, HashTypeSetEx *exParams);
-
 int hashTypeSetExInit(robj *key, robj *o, client *c, redisDb *db, const char *cmd,
-                      FieldSetCond fieldSetCond, ExpireSetCond expireSetCond, HashTypeSetEx *ex);
+                      ExpireSetCond expireSetCond, HashTypeSetEx *ex);
 
-SetExRes hashTypeSetEx(redisDb *db, robj *o, sds field, HashTypeSet *setKeyVal,
-                       uint64_t expireAt, HashTypeSetEx *exInfo);
+SetExRes hashTypeSetEx(robj *o, sds field, uint64_t expireAt, HashTypeSetEx *exInfo);
 
 void hashTypeSetExDone(HashTypeSetEx *e);
 
@@ -712,23 +692,21 @@ GetFieldRes hashTypeGetFromHashTable(robj *o, sds field, sds *value, uint64_t *e
 
 /* Higher level function of hashTypeGet*() that returns the hash value
  * associated with the specified field.
+ * Arguments:
+ * hfeFlags      - Lookup for HFE_LAZY_* flags
  *
  * Returned:
- * - GetFieldRes: OK: Return    Field's valid value
- *                NOT_FOUND:    Field was not found.
- *                EXPIRED:      Field is expired and Lazy deleted
- *                EXPIRED_HASH: Returned only if the field is the last one in the
- *                              hash and the hash is deleted.
- * - vstr, vlen : if string, ref in either *vstr and *vlen if it's
+ * GetFieldRes  - Result of get operation
+ * vstr, vlen   - if string, ref in either *vstr and *vlen if it's
  *                returned in string form,
- * - vll        : or stored in *vll if it's returned as a number.
+ * vll          - or stored in *vll if it's returned as a number.
  *                If *vll is populated *vstr is set to NULL, so the caller can
  *                always check the function return by checking the return value
  *                for GETF_OK and checking if vll (or vstr) is NULL.
  *
  */
 GetFieldRes hashTypeGetValue(redisDb *db, robj *o, sds field, unsigned char **vstr,
-                             unsigned int *vlen, long long *vll) {
+                             unsigned int *vlen, long long *vll, int hfeFlags) {
     uint64_t expiredAt;
     sds key;
     GetFieldRes res;
@@ -760,7 +738,12 @@ GetFieldRes hashTypeGetValue(redisDb *db, robj *o, sds field, unsigned char **vs
          (expiredAt >= (uint64_t) commandTimeSnapshot()) )
         return GETF_OK;
 
-    /* Got expired. Extract attached key from LISTPACK_EX/HT */
+    /* Field is expired */
+
+    /* If indicated to avoid deleting expired field */
+    if (hfeFlags & HFE_LAZY_AVOID_FIELD_DEL)
+        return GETF_EXPIRED;
+
     if (o->encoding == OBJ_ENCODING_LISTPACK_EX)
         key = ((listpackEx *) o->ptr)->key;
     else
@@ -771,10 +754,11 @@ GetFieldRes hashTypeGetValue(redisDb *db, robj *o, sds field, unsigned char **vs
     propagateHashFieldDeletion(db, key, field, sdslen(field));
 
     /* If the field is the last one in the hash, then the hash will be deleted */
-    if (hashTypeLength(o, 0) == 0) {
+    if ((hashTypeLength(o, 0) == 0) && (!(hfeFlags & HFE_LAZY_AVOID_HASH_DEL))) {
         robj *keyObj = createStringObject(key, sdslen(key));
         notifyKeyspaceEvent(NOTIFY_GENERIC, "del", keyObj, db->id);
         dbDelete(db,keyObj);
+        signalModifiedKey(NULL, db, keyObj);
         decrRefCount(keyObj);
         return GETF_EXPIRED_HASH;
     }
@@ -787,24 +771,25 @@ GetFieldRes hashTypeGetValue(redisDb *db, robj *o, sds field, unsigned char **vs
  * The function returns NULL if the field is not found in the hash. Otherwise
  * a newly allocated string object with the value is returned.
  *
+ * hfeFlags      - Lookup HFE_LAZY_* flags
  * isHashDeleted - If attempted to access expired field and it's the last field
  *                 in the hash, then the hash will as well be deleted. In this case,
  *                 isHashDeleted will be set to 1.
  */
-robj *hashTypeGetValueObject(redisDb *db, robj *o, sds field, int *isHashDeleted) {
+robj *hashTypeGetValueObject(redisDb *db, robj *o, sds field, int hfeFlags, int *isHashDeleted) {
     unsigned char *vstr;
     unsigned int vlen;
     long long vll;
 
-    *isHashDeleted = 0; /*default*/
-    GetFieldRes res = hashTypeGetValue(db,o,field,&vstr,&vlen,&vll);
+    if (isHashDeleted) *isHashDeleted = 0;
+    GetFieldRes res = hashTypeGetValue(db,o,field,&vstr,&vlen,&vll, hfeFlags);
 
     if (res == GETF_OK) {
         if (vstr) return createStringObject((char*)vstr,vlen);
         else return createStringObjectFromLongLong(vll);
     }
 
-    if (res == GETF_EXPIRED_HASH)
+    if ((res == GETF_EXPIRED_HASH) && (isHashDeleted))
         *isHashDeleted = 1;
 
     /* GETF_EXPIRED_HASH, GETF_EXPIRED, GETF_NOT_FOUND */
@@ -814,19 +799,21 @@ robj *hashTypeGetValueObject(redisDb *db, robj *o, sds field, int *isHashDeleted
 /* Test if the specified field exists in the given hash. If the field is
  * expired (HFE), then it will be lazy deleted
  *
- * Returns 1 if the field exists, and 0 when it doesn't.
- *
+ * hfeFlags      - Lookup HFE_LAZY_* flags
  * isHashDeleted - If attempted to access expired field and it is the last field
  *                 in the hash, then the hash will as well be deleted. In this case,
  *                 isHashDeleted will be set to 1.
+ *
+ * Returns 1 if the field exists, and 0 when it doesn't.
  */
-int hashTypeExists(redisDb *db, robj *o, sds field, int *isHashDeleted) {
+int hashTypeExists(redisDb *db, robj *o, sds field, int hfeFlags, int *isHashDeleted) {
     unsigned char *vstr = NULL;
     unsigned int vlen = UINT_MAX;
     long long vll = LLONG_MAX;
 
-    GetFieldRes res = hashTypeGetValue(db, o, field, &vstr, &vlen, &vll);
-    *isHashDeleted = (res == GETF_EXPIRED_HASH) ? 1 : 0;
+    GetFieldRes res = hashTypeGetValue(db, o, field, &vstr, &vlen, &vll, hfeFlags);
+    if (isHashDeleted)
+        *isHashDeleted = (res == GETF_EXPIRED_HASH) ? 1 : 0;
     return (res == GETF_OK) ? 1 : 0;
 }
 
@@ -839,7 +826,7 @@ int hashTypeExists(redisDb *db, robj *o, sds field, int *isHashDeleted) {
  *
  * HASH_SET_TAKE_FIELD  -- The SDS field ownership passes to the function.
  * HASH_SET_TAKE_VALUE  -- The SDS value ownership passes to the function.
- * HASH_SET_KEEP_FIELD --  keep original field along with TTL if already exists
+ * HASH_SET_KEEP_TTL --  keep original TTL if field already exists
  *
  * When the flags are used the caller does not need to release the passed
  * SDS string(s). It's up to the function to use the string to create a new
@@ -851,164 +838,92 @@ int hashTypeExists(redisDb *db, robj *o, sds field, int *isHashDeleted) {
  */
 #define HASH_SET_TAKE_FIELD  (1<<0)
 #define HASH_SET_TAKE_VALUE  (1<<1)
-#define HASH_SET_KEEP_FIELD (1<<2)
+#define HASH_SET_KEEP_TTL (1<<2)
 #define HASH_SET_COPY 0
 int hashTypeSet(redisDb *db, robj *o, sds field, sds value, int flags) {
-    HashTypeSet set = {value, flags};
-    return (hashTypeSetEx(db, o, field, &set, 0, NULL) == HSET_UPDATE) ? 1 : 0;
-}
-
-SetExRes hashTypeSetExpiry(HashTypeSetEx *ex, sds field, uint64_t expireAt, dictEntry **de) {
-    dict *ht = ex->hashObj->ptr;
-    dictEntry *newEntry = NULL, *existingEntry = NULL;
-
-    /* New field with expiration metadata */
-    hfield hfNew = hfieldNew(field, sdslen(field), 1 /*withExpireMeta*/);
-
-    if ((ex->fieldSetCond == FIELD_DONT_CREATE) || (ex->fieldSetCond == FIELD_DONT_CREATE2)) {
-        if ((existingEntry = dictFind(ht, field)) == NULL) {
-            hfieldFree(hfNew);
-            return (ex->fieldSetCond == FIELD_DONT_CREATE) ?
-                   HSETEX_NO_CONDITION_MET : HSETEX_NO_FIELD;
-        }
-    } else {
-        dictUseStoredKeyApi(ht, 1);
-        newEntry = dictAddRaw(ht, hfNew, &existingEntry);
-        dictUseStoredKeyApi(ht, 0);
-    }
-
-    if (newEntry) {
-        *de = newEntry;
-
-        if (ex->expireSetCond & (HFE_XX | HFE_LT | HFE_GT)) {
-            dictDelete(ht, field);
-            return HSETEX_NO_CONDITION_MET;
-        }
-    } else { /* field exist */
-        *de = existingEntry;
-
-        if (ex->fieldSetCond == FIELD_DONT_OVRWRT) {
-            hfieldFree(hfNew);
-            return HSETEX_NO_CONDITION_MET;
-        }
-
-        hfield hfOld = dictGetKey(existingEntry);
-
-        /* If field doesn't have expiry metadata attached */
-        if (!hfieldIsExpireAttached(hfOld)) {
-
-            /* For fields without expiry, LT condition is considered valid */
-            if (ex->expireSetCond & (HFE_XX | HFE_GT)) {
-                hfieldFree(hfNew);
-                return HSETEX_NO_CONDITION_MET;
-            }
-
-            /* Delete old field. Below goanna dictSetKey(..,hfNew) */
-            hfieldFree(hfOld);
-
-        } else { /* field has ExpireMeta struct attached */
-
-            /* No need for hfNew (Just modify expire-time of existing field) */
-            hfieldFree(hfNew);
-
-            uint64_t prevExpire = hfieldGetExpireTime(hfOld);
-
-            /* If field has valid expiration time, then check GT|LT|NX */
-            if (prevExpire != EB_EXPIRE_TIME_INVALID) {
-                if (((ex->expireSetCond == HFE_GT) && (prevExpire >= expireAt)) ||
-                    ((ex->expireSetCond == HFE_LT) && (prevExpire <= expireAt)) ||
-                    (ex->expireSetCond == HFE_NX) )
-                    return HSETEX_NO_CONDITION_MET;
-
-                /* remove old expiry time from hash's private ebuckets */
-                dictExpireMetadata *dm = (dictExpireMetadata *) dictMetadata(ht);
-                ebRemove(&dm->hfe, &hashFieldExpireBucketsType, hfOld);
-
-                /* Track of minimum expiration time (only later update global HFE DS) */
-                if (ex->minExpireFields > prevExpire)
-                    ex->minExpireFields = prevExpire;
-
-            } else {
-                /* field has invalid expiry. No need to ebRemove() */
-
-                /* Check XX|LT|GT */
-                if (ex->expireSetCond & (HFE_XX | HFE_GT))
-                    return HSETEX_NO_CONDITION_MET;
-            }
-
-            /* Reuse hfOld as hfNew and rewrite its expiry with ebAdd() */
-            hfNew = hfOld;
-        }
-
-        dictSetKey(ht, existingEntry, hfNew);
-    }
-
-    /* if expiration time is in the past */
-    if (unlikely(checkAlreadyExpired(expireAt))) {
-        hashTypeDelete(ex->hashObj, field, 1);
-        ex->fieldDeleted++;
-        return HSETEX_DELETED;
-    }
-
-    if (ex->minExpireFields > expireAt)
-        ex->minExpireFields = expireAt;
-
-    dictExpireMetadata *dm = (dictExpireMetadata *) dictMetadata(ht);
-    ebAdd(&dm->hfe, &hashFieldExpireBucketsType, hfNew, expireAt);
-    ex->fieldUpdated++;
-    return HSETEX_OK;
-}
-
-/*
- * Set fields OR field's expiration (See also `setex*` comment above)
- *
- * Take care to call first hashTypeSetExInit() and then call this function.
- * Finally, call hashTypeSetExDone() to notify and update global HFE DS.
- *
- * NOTE: this functions is also called during RDB load to set dict-encoded
- *       fields with and without expiration.
- */
-SetExRes hashTypeSetEx(redisDb *db, robj *o, sds field, HashTypeSet *setKeyVal,
-                       uint64_t expireAt, HashTypeSetEx *exInfo)
-{
-    SetExRes res = HSETEX_OK;
-    int isSetKeyValue  = (setKeyVal) ? 1 : 0;
-    int isSetExpire = (exInfo) ? 1 : 0;
-    int flags = (setKeyVal) ? setKeyVal->flags : 0;
+    int update = 0;
 
     /* Check if the field is too long for listpack, and convert before adding the item.
      * This is needed for HINCRBY* case since in other commands this is handled early by
      * hashTypeTryConversion, so this check will be a NOP. */
-    if (o->encoding == OBJ_ENCODING_LISTPACK ||
-        o->encoding == OBJ_ENCODING_LISTPACK_EX)
-    {
-        if ( (isSetKeyValue) &&
-             (sdslen(field) > server.hash_max_listpack_value ||
-              sdslen(setKeyVal->value) > server.hash_max_listpack_value) )
-        {
+    if (o->encoding == OBJ_ENCODING_LISTPACK  ||
+        o->encoding == OBJ_ENCODING_LISTPACK_EX) {
+        if (sdslen(field) > server.hash_max_listpack_value || sdslen(value) > server.hash_max_listpack_value)
             hashTypeConvert(o, OBJ_ENCODING_HT, &db->hexpires);
-        } else {
-            res = hashTypeSetExListpack(db, o, field, setKeyVal, expireAt, exInfo);
-            goto SetExDone; /*done*/
-        }
     }
 
-    if (o->encoding != OBJ_ENCODING_HT)
-        serverPanic("Unknown hash encoding");
+    if (o->encoding == OBJ_ENCODING_LISTPACK) {
+        unsigned char *zl, *fptr, *vptr;
 
-    /*** now deal with HT ***/
-    hfield newField;
-    dict *ht = o->ptr;
-    dictEntry *de;
+        zl = o->ptr;
+        fptr = lpFirst(zl);
+        if (fptr != NULL) {
+            fptr = lpFind(zl, fptr, (unsigned char*)field, sdslen(field), 1);
+            if (fptr != NULL) {
+                /* Grab pointer to the value (fptr points to the field) */
+                vptr = lpNext(zl, fptr);
+                serverAssert(vptr != NULL);
 
-    /* If needed to set the field along with expiry */
-    if (isSetExpire) {
-        res = hashTypeSetExpiry(exInfo, field, expireAt, &de);
-        if (res != HSETEX_OK) goto SetExDone;
-    } else {
-        dictEntry *existing;
-        /* Cannot leverage HASH_SET_TAKE_FIELD since hfield is not of type sds */
-        newField = hfieldNew(field, sdslen(field), 0);
+                /* Replace value */
+                zl = lpReplace(zl, &vptr, (unsigned char*)value, sdslen(value));
+                update = 1;
+            }
+        }
+
+        if (!update) {
+            /* Push new field/value pair onto the tail of the listpack */
+            zl = lpAppend(zl, (unsigned char*)field, sdslen(field));
+            zl = lpAppend(zl, (unsigned char*)value, sdslen(value));
+        }
+        o->ptr = zl;
+
+        /* Check if the listpack needs to be converted to a hash table */
+        if (hashTypeLength(o, 0) > server.hash_max_listpack_entries)
+            hashTypeConvert(o, OBJ_ENCODING_HT, &db->hexpires);
+    } else if (o->encoding == OBJ_ENCODING_LISTPACK_EX) {
+        unsigned char *fptr = NULL, *vptr = NULL, *tptr = NULL;
+        listpackEx *lpt = o->ptr;
+        long long expireTime = HASH_LP_NO_TTL;
+
+        fptr = lpFirst(lpt->lp);
+        if (fptr != NULL) {
+            fptr = lpFind(lpt->lp, fptr, (unsigned char*)field, sdslen(field), 2);
+            if (fptr != NULL) {
+                /* Grab pointer to the value (fptr points to the field) */
+                vptr = lpNext(lpt->lp, fptr);
+                serverAssert(vptr != NULL);
+
+                /* Replace value */
+                lpt->lp = lpReplace(lpt->lp, &vptr, (unsigned char *) value, sdslen(value));
+                update = 1;
+
+                fptr = lpPrev(lpt->lp, vptr);
+                serverAssert(fptr != NULL);
+
+                tptr = lpNext(lpt->lp, vptr);
+                serverAssert(tptr && lpGetIntegerValue(tptr, &expireTime));
+
+                if (flags & HASH_SET_KEEP_TTL) {
+                    /* keep old field along with TTL */
+                } else if (expireTime != HASH_LP_NO_TTL) {
+                    /* re-insert field and override TTL */
+                    listpackExUpdateExpiry(o, field, fptr, vptr, HASH_LP_NO_TTL);
+                }
+            }
+        }
+
+        if (!update)
+            listpackExAddNew(o, field, sdslen(field), value, sdslen(value),
+                             HASH_LP_NO_TTL);
+
+        /* Check if the listpack needs to be converted to a hash table */
+        if (hashTypeLength(o, 0) > server.hash_max_listpack_entries)
+            hashTypeConvert(o, OBJ_ENCODING_HT, &db->hexpires);
+
+    } else if (o->encoding == OBJ_ENCODING_HT) {
+        hfield newField = hfieldNew(field, sdslen(field), 0);
+        dict *ht = o->ptr;
+        dictEntry *de, *existing;
 
         /* stored key is different than lookup key */
         dictUseStoredKeyApi(ht, 1);
@@ -1017,8 +932,8 @@ SetExRes hashTypeSetEx(redisDb *db, robj *o, sds field, HashTypeSet *setKeyVal,
 
         /* If field already exists, then update "field". "Value" will be set afterward */
         if (de == NULL) {
-            if (flags & HASH_SET_KEEP_FIELD) {
-                /* Not keep old field along with TTL */
+            if (flags & HASH_SET_KEEP_TTL) {
+                /* keep old field along with TTL */
                 hfieldFree(newField);
             } else {
                 /* If attached TTL to the old field, then remove it from hash's private ebuckets */
@@ -1028,27 +943,146 @@ SetExRes hashTypeSetEx(redisDb *db, robj *o, sds field, HashTypeSet *setKeyVal,
                 dictSetKey(ht, existing, newField);
             }
             sdsfree(dictGetVal(existing));
-            res = HSET_UPDATE;
+            update = 1;
             de = existing;
         }
-    }
 
-    /* If need to set value */
-    if (isSetKeyValue) {
         if (flags & HASH_SET_TAKE_VALUE) {
-            dictSetVal(ht, de, setKeyVal->value);
+            dictSetVal(ht, de, value);
             flags &= ~HASH_SET_TAKE_VALUE;
         } else {
-            dictSetVal(ht, de, sdsdup(setKeyVal->value));
+            dictSetVal(ht, de, sdsdup(value));
         }
+    } else {
+        serverPanic("Unknown hash encoding");
     }
 
-SetExDone:
     /* Free SDS strings we did not referenced elsewhere if the flags
      * want this function to be responsible. */
     if (flags & HASH_SET_TAKE_FIELD && field) sdsfree(field);
-    if (flags & HASH_SET_TAKE_VALUE && setKeyVal->value) sdsfree(setKeyVal->value);
-    return res;
+    if (flags & HASH_SET_TAKE_VALUE && value) sdsfree(value);
+    return update;
+}
+
+SetExRes hashTypeSetExpiryHT(HashTypeSetEx *exInfo, sds field, uint64_t expireAt) {
+    dict *ht = exInfo->hashObj->ptr;
+    dictEntry *existingEntry = NULL;
+
+    /* New field with expiration metadata */
+    hfield hfNew = hfieldNew(field, sdslen(field), 1 /*withExpireMeta*/);
+
+    if ((existingEntry = dictFind(ht, field)) == NULL) {
+        hfieldFree(hfNew);
+        return HSETEX_NO_FIELD;
+    }
+
+    hfield hfOld = dictGetKey(existingEntry);
+
+    /* If field doesn't have expiry metadata attached */
+    if (!hfieldIsExpireAttached(hfOld)) {
+
+        /* For fields without expiry, LT condition is considered valid */
+        if (exInfo->expireSetCond & (HFE_XX | HFE_GT)) {
+            hfieldFree(hfNew);
+            return HSETEX_NO_CONDITION_MET;
+        }
+
+        /* Delete old field. Below goanna dictSetKey(..,hfNew) */
+        hfieldFree(hfOld);
+
+    } else { /* field has ExpireMeta struct attached */
+
+        /* No need for hfNew (Just modify expire-time of existing field) */
+        hfieldFree(hfNew);
+
+        uint64_t prevExpire = hfieldGetExpireTime(hfOld);
+
+        /* If field has valid expiration time, then check GT|LT|NX */
+        if (prevExpire != EB_EXPIRE_TIME_INVALID) {
+            if (((exInfo->expireSetCond == HFE_GT) && (prevExpire >= expireAt)) ||
+                ((exInfo->expireSetCond == HFE_LT) && (prevExpire <= expireAt)) ||
+                (exInfo->expireSetCond == HFE_NX) )
+                return HSETEX_NO_CONDITION_MET;
+
+            /* remove old expiry time from hash's private ebuckets */
+            dictExpireMetadata *dm = (dictExpireMetadata *) dictMetadata(ht);
+            ebRemove(&dm->hfe, &hashFieldExpireBucketsType, hfOld);
+
+            /* Track of minimum expiration time (only later update global HFE DS) */
+            if (exInfo->minExpireFields > prevExpire)
+                exInfo->minExpireFields = prevExpire;
+
+        } else {
+            /* field has invalid expiry. No need to ebRemove() */
+
+            /* Check XX|LT|GT */
+            if (exInfo->expireSetCond & (HFE_XX | HFE_GT))
+                return HSETEX_NO_CONDITION_MET;
+        }
+
+        /* Reuse hfOld as hfNew and rewrite its expiry with ebAdd() */
+        hfNew = hfOld;
+    }
+
+    dictSetKey(ht, existingEntry, hfNew);
+
+
+    /* if expiration time is in the past */
+    if (unlikely(checkAlreadyExpired(expireAt))) {
+        hashTypeDelete(exInfo->hashObj, field, 1);
+        exInfo->fieldDeleted++;
+        return HSETEX_DELETED;
+    }
+
+    if (exInfo->minExpireFields > expireAt)
+        exInfo->minExpireFields = expireAt;
+
+    dictExpireMetadata *dm = (dictExpireMetadata *) dictMetadata(ht);
+    ebAdd(&dm->hfe, &hashFieldExpireBucketsType, hfNew, expireAt);
+    exInfo->fieldUpdated++;
+    return HSETEX_OK;
+}
+
+/*
+ * Set field expiration
+ *
+ * Take care to call first hashTypeSetExInit() and then call this function.
+ * Finally, call hashTypeSetExDone() to notify and update global HFE DS.
+ */
+SetExRes hashTypeSetEx(robj *o, sds field, uint64_t expireAt, HashTypeSetEx *exInfo)
+{
+    if (o->encoding == OBJ_ENCODING_LISTPACK_EX)
+    {
+        unsigned char *fptr = NULL, *vptr = NULL, *tptr = NULL;
+
+        listpackEx *lpt = o->ptr;
+        long long expireTime = HASH_LP_NO_TTL;
+
+        if ((fptr = lpFirst(lpt->lp)) == NULL)
+            return HSETEX_NO_FIELD;
+
+        fptr = lpFind(lpt->lp, fptr, (unsigned char*)field, sdslen(field), 2);
+
+        if (!fptr)
+            return HSETEX_NO_FIELD;
+
+        /* Grab pointer to the value (fptr points to the field) */
+        vptr = lpNext(lpt->lp, fptr);
+        serverAssert(vptr != NULL);
+
+        tptr = lpNext(lpt->lp, vptr);
+        serverAssert(tptr && lpGetIntegerValue(tptr, &expireTime));
+
+        /* update TTL */
+        return hashTypeSetExpiryListpack(exInfo, field, fptr, vptr, tptr, expireAt);
+    } else if (o->encoding == OBJ_ENCODING_HT) {
+        /* If needed to set the field along with expiry */
+        return hashTypeSetExpiryHT(exInfo, field, expireAt);
+    } else {
+        serverPanic("Unknown hash encoding");
+    }
+
+    return HSETEX_OK; /* never reach here */
 }
 
 void initDictExpireMetadata(sds key, robj *o) {
@@ -1066,12 +1100,10 @@ void initDictExpireMetadata(sds key, robj *o) {
  * Don't have to provide client and "cmd". If provided, then notification once
  * done by function hashTypeSetExDone().
  */
-int hashTypeSetExInit(robj *key, robj *o, client *c, redisDb *db, const char *cmd, FieldSetCond fieldSetCond,
+int hashTypeSetExInit(robj *key, robj *o, client *c, redisDb *db, const char *cmd,
                       ExpireSetCond expireSetCond, HashTypeSetEx *ex)
 {
     dict *ht = o->ptr;
-
-    ex->fieldSetCond = fieldSetCond;
     ex->expireSetCond = expireSetCond;
     ex->minExpire = EB_EXPIRE_TIME_INVALID;
     ex->c = c;
@@ -1123,15 +1155,15 @@ void hashTypeSetExDone(HashTypeSetEx *ex) {
     /* Notify keyspace event, update dirty count and update global HFE DS */
     if (ex->fieldDeleted + ex->fieldUpdated > 0) {
 
-        if (ex->c) {
-            server.dirty += ex->fieldDeleted + ex->fieldUpdated;
-            signalModifiedKey(ex->c, ex->db, ex->key);
-            notifyKeyspaceEvent(NOTIFY_HASH, "hexpire", ex->key, ex->db->id);
-        }
+        server.dirty += ex->fieldDeleted + ex->fieldUpdated;
         if (ex->fieldDeleted && hashTypeLength(ex->hashObj, 0) == 0) {
             dbDelete(ex->db,ex->key);
-            if (ex->c) notifyKeyspaceEvent(NOTIFY_GENERIC,"del",ex->key, ex->db->id);
+            signalModifiedKey(ex->c, ex->db, ex->key);
+            notifyKeyspaceEvent(NOTIFY_GENERIC,"del",ex->key, ex->db->id);
         } else {
+            signalModifiedKey(ex->c, ex->db, ex->key);
+            notifyKeyspaceEvent(NOTIFY_HASH, "hexpire", ex->key, ex->db->id);
+
             /* If minimum HFE of the hash is smaller than expiration time of the
              * specified fields in the command as well as it is smaller or equal
              * than expiration time provided in the command, then the minimum
@@ -1161,99 +1193,6 @@ void hashTypeSetExDone(HashTypeSetEx *ex) {
     }
 }
 
-/* Check if the field is too long for listpack, and convert before adding the item.
- * This is needed for HINCRBY* case since in other commands this is handled early by
- * hashTypeTryConversion, so this check will be a NOP. */
-static SetExRes hashTypeSetExListpack(redisDb *db, robj *o, sds field, HashTypeSet *setParams,
-                                      uint64_t expireAt, HashTypeSetEx *exParams)
-{
-    int res = HSETEX_OK;
-    unsigned char *fptr = NULL, *vptr = NULL, *tptr = NULL;
-
-    if (o->encoding == OBJ_ENCODING_LISTPACK) {
-        /* If reached here, then no need to set expiration. Otherwise, as precond
-         * listpack is converted to listpackex by hashTypeSetExInit() */
-
-        unsigned char *zl = o->ptr;
-        fptr = lpFirst(zl);
-        if (fptr != NULL) {
-            fptr = lpFind(zl, fptr, (unsigned char*)field, sdslen(field), 1);
-            if (fptr != NULL) {
-                /* Grab pointer to the value (fptr points to the field) */
-                vptr = lpNext(zl, fptr);
-                serverAssert(vptr != NULL);
-                res = HSET_UPDATE;
-
-                /* Replace value */
-                zl = lpReplace(zl, &vptr, (unsigned char *) setParams->value, sdslen(setParams->value));
-            }
-        }
-
-        if (res != HSET_UPDATE) {
-            /* Push new field/value pair onto the tail of the listpack */
-            zl = lpAppend(zl, (unsigned char*)field, sdslen(field));
-            zl = lpAppend(zl, (unsigned char*)setParams->value, sdslen(setParams->value));
-        }
-        o->ptr = zl;
-        goto out;
-    } else if (o->encoding == OBJ_ENCODING_LISTPACK_EX) {
-        listpackEx *lpt = o->ptr;
-        long long expireTime = HASH_LP_NO_TTL;
-
-        fptr = lpFirst(lpt->lp);
-        if (fptr != NULL) {
-            fptr = lpFind(lpt->lp, fptr, (unsigned char*)field, sdslen(field), 2);
-            if (fptr != NULL) {
-                /* Grab pointer to the value (fptr points to the field) */
-                vptr = lpNext(lpt->lp, fptr);
-                serverAssert(vptr != NULL);
-
-                if (setParams) {
-                    /* Replace value */
-                    lpt->lp = lpReplace(lpt->lp, &vptr,
-                                        (unsigned char *) setParams->value,
-                                        sdslen(setParams->value));
-
-                    fptr = lpPrev(lpt->lp, vptr);
-                    serverAssert(fptr != NULL);
-                    res = HSET_UPDATE;
-                }
-                tptr = lpNext(lpt->lp, vptr);
-                serverAssert(tptr && lpGetIntegerValue(tptr, &expireTime));
-
-                /* Keep, update or clear TTL */
-                if (setParams && setParams->flags & HASH_SET_KEEP_FIELD) {
-                    /* keep old field along with TTL */
-                } else if (exParams) {
-                    res = hashTypeSetExpiryListpack(exParams, field, fptr, vptr, tptr,
-                                                    expireAt);
-                    if (res != HSETEX_OK)
-                        goto out;
-                } else if (res == HSET_UPDATE && expireTime != HASH_LP_NO_TTL) {
-                    /* Clear TTL */
-                    listpackExUpdateExpiry(o, field, fptr, vptr, HASH_LP_NO_TTL);
-                }
-            }
-        }
-
-        if (!fptr) {
-            if (setParams) {
-                listpackExAddNew(o, field, sdslen(field),
-                                 setParams->value, sdslen(setParams->value),
-                                 exParams ? expireAt : HASH_LP_NO_TTL);
-            } else {
-                res = HSETEX_NO_FIELD;
-            }
-        }
-    }
-out:
-    /* Check if the listpack needs to be converted to a hash table */
-    if (hashTypeLength(o, 0) > server.hash_max_listpack_entries)
-        hashTypeConvert(o, OBJ_ENCODING_HT, &db->hexpires);
-
-    return res;
-}
-
 /* Delete an element from a hash.
  *
  * Return 1 on deleted and 0 on not found.
@@ -2079,7 +2018,7 @@ void hsetnxCommand(client *c) {
     robj *o;
     if ((o = hashTypeLookupWriteOrCreate(c,c->argv[1])) == NULL) return;
 
-    if (hashTypeExists(c->db, o, c->argv[2]->ptr, &isHashDeleted)) {
+    if (hashTypeExists(c->db, o, c->argv[2]->ptr, HFE_LAZY_EXPIRE, &isHashDeleted)) {
         addReply(c, shared.czero);
         return;
     }
@@ -2137,7 +2076,8 @@ void hincrbyCommand(client *c) {
     if (getLongLongFromObjectOrReply(c,c->argv[3],&incr,NULL) != C_OK) return;
     if ((o = hashTypeLookupWriteOrCreate(c,c->argv[1])) == NULL) return;
 
-    GetFieldRes res = hashTypeGetValue(c->db,o,c->argv[2]->ptr,&vstr,&vlen,&value);
+    GetFieldRes res = hashTypeGetValue(c->db,o,c->argv[2]->ptr,&vstr,&vlen,&value,
+                                       HFE_LAZY_EXPIRE);
     if (res == GETF_OK) {
         if (vstr) {
             if (string2ll((char*)vstr,vlen,&value) == 0) {
@@ -2162,7 +2102,7 @@ void hincrbyCommand(client *c) {
     }
     value += incr;
     new = sdsfromlonglong(value);
-    hashTypeSet(c->db, o,c->argv[2]->ptr,new,HASH_SET_TAKE_VALUE | HASH_SET_KEEP_FIELD);
+    hashTypeSet(c->db, o,c->argv[2]->ptr,new,HASH_SET_TAKE_VALUE | HASH_SET_KEEP_TTL);
     addReplyLongLong(c,value);
     signalModifiedKey(c,c->db,c->argv[1]);
     notifyKeyspaceEvent(NOTIFY_HASH,"hincrby",c->argv[1],c->db->id);
@@ -2183,7 +2123,8 @@ void hincrbyfloatCommand(client *c) {
         return;
     }
     if ((o = hashTypeLookupWriteOrCreate(c,c->argv[1])) == NULL) return;
-    GetFieldRes res = hashTypeGetValue(c->db, o,c->argv[2]->ptr,&vstr,&vlen,&ll);
+    GetFieldRes res = hashTypeGetValue(c->db, o,c->argv[2]->ptr,&vstr,&vlen,&ll,
+                                       HFE_LAZY_EXPIRE);
     if (res == GETF_OK) {
         if (vstr) {
             if (string2ld((char*)vstr,vlen,&value) == 0) {
@@ -2211,7 +2152,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_FIELD);
+    hashTypeSet(c->db, o,c->argv[2]->ptr,new,HASH_SET_TAKE_VALUE | HASH_SET_KEEP_TTL);
     addReplyBulkCBuffer(c,buf,len);
     signalModifiedKey(c,c->db,c->argv[1]);
     notifyKeyspaceEvent(NOTIFY_HASH,"hincrbyfloat",c->argv[1],c->db->id);
@@ -2237,7 +2178,8 @@ static GetFieldRes addHashFieldToReply(client *c, robj *o, sds field) {
     unsigned int vlen = UINT_MAX;
     long long vll = LLONG_MAX;
 
-    GetFieldRes res = hashTypeGetValue(c->db, o, field, &vstr, &vlen, &vll);
+    GetFieldRes res = hashTypeGetValue(c->db, o, field, &vstr, &vlen, &vll,
+                                       HFE_LAZY_EXPIRE);
     if (res == GETF_OK) {
         if (vstr) {
             addReplyBulkCBuffer(c, vstr, vlen);
@@ -2330,7 +2272,8 @@ void hstrlenCommand(client *c) {
     if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.czero)) == NULL ||
         checkType(c,o,OBJ_HASH)) return;
 
-    GetFieldRes res = hashTypeGetValue(c->db, o, c->argv[2]->ptr, &vstr, &vlen, &vll);
+    GetFieldRes res = hashTypeGetValue(c->db, o, c->argv[2]->ptr, &vstr, &vlen, &vll,
+                                       HFE_LAZY_EXPIRE);
 
     if (res == GETF_NOT_FOUND || res == GETF_EXPIRED || res == GETF_EXPIRED_HASH) {
         addReply(c, shared.czero);
@@ -2421,11 +2364,11 @@ void hgetallCommand(client *c) {
 
 void hexistsCommand(client *c) {
     robj *o;
-    int isHashDeleted;
     if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.czero)) == NULL ||
         checkType(c,o,OBJ_HASH)) return;
 
-    addReply(c,hashTypeExists(c->db,o,c->argv[2]->ptr,&isHashDeleted) ? shared.cone : shared.czero);
+    addReply(c,hashTypeExists(c->db,o,c->argv[2]->ptr,HFE_LAZY_EXPIRE, NULL) ?
+                                shared.cone : shared.czero);
 }
 
 void hscanCommand(client *c) {
@@ -2847,6 +2790,7 @@ static ExpireMeta *hashGetExpireMeta(const eItem hash) {
         serverPanic("Unknown encoding: %d", hashObj->encoding);
     }
 }
+
 /* HTTL key <FIELDS count field [field ...]>  */
 static void httlGenericCommand(client *c, const char *cmd, long long basetime, int unit) {
     UNUSED(cmd);
@@ -3032,16 +2976,12 @@ static void hexpireGenericCommand(client *c, const char *cmd, long long basetime
     }
 
     HashTypeSetEx exCtx;
-    hashTypeSetExInit(keyArg, hashObj, c, c->db, cmd,
-                      FIELD_DONT_CREATE2,
-                      expireSetCond,
-                      &exCtx);
-
+    hashTypeSetExInit(keyArg, hashObj, c, c->db, cmd, expireSetCond, &exCtx);
     addReplyArrayLen(c, numFields);
 
     for (int i = 0 ; i < numFields ; i++) {
         sds field = c->argv[numFieldsAt+i+1]->ptr;
-        SetExRes res = hashTypeSetEx(c->db, hashObj, field, NULL, expire, &exCtx);
+        SetExRes res = hashTypeSetEx(hashObj, field, expire, &exCtx);
         addReplyLongLong(c,res);
     }
     hashTypeSetExDone(&exCtx);
@@ -3216,5 +3156,9 @@ void hpersistCommand(client *c) {
 
     /* Generates a hpersist event if the expiry time associated with any field
      * has been successfully deleted. */
-    if (changed) notifyKeyspaceEvent(NOTIFY_HASH,"hpersist",c->argv[1],c->db->id);
+    if (changed) {
+        notifyKeyspaceEvent(NOTIFY_HASH, "hpersist", c->argv[1], c->db->id);
+        signalModifiedKey(c, c->db, c->argv[1]);
+        server.dirty++;
+    }
 }
diff --git a/tests/unit/moduleapi/hash.tcl b/tests/unit/moduleapi/hash.tcl
index 116b1c512..8cd919b3a 100644
--- a/tests/unit/moduleapi/hash.tcl
+++ b/tests/unit/moduleapi/hash.tcl
@@ -21,6 +21,45 @@ start_server {tags {"modules"}} {
         r hgetall k
     } {squirrel ofcourse banana no what nothing something nice}
 
+    test {Module hash - set (override) NX expired field successfully} {
+        r debug set-active-expire 0
+        r del H1 H2
+        r hash.set H1 "n" f1 v1
+        r hpexpire H1 1 FIELDS 1 f1
+        r hash.set H2 "n" f1 v1 f2 v2
+        r hpexpire H2 1 FIELDS 1 f1
+        after 5
+        assert_equal 0 [r hash.set H1 "n" f1 xx]
+        assert_equal "f1 xx" [r hgetall H1]
+        assert_equal 0 [r hash.set H2 "n" f1 yy]
+        assert_equal "f1 f2 v2 yy" [lsort [r hgetall H2]]
+        r debug set-active-expire 1
+    } {OK} {needs:debug}
+
+    test {Module hash - set XX of expired field gets failed as expected} {
+        r debug set-active-expire 0
+        r del H1 H2
+        r hash.set H1 "n" f1 v1
+        r hpexpire H1 1 FIELDS 1 f1
+        r hash.set H2 "n" f1 v1 f2 v2
+        r hpexpire H2 1 FIELDS 1 f1
+        after 5
+
+        # expected to fail on condition XX. hgetall should return empty list
+        r hash.set H1 "x" f1 xx
+        assert_equal "" [lsort [r hgetall H1]]
+        # But expired field was not lazy deleted
+        assert_equal 1 [r hlen H1]
+
+        # expected to fail on condition XX. hgetall should return list without expired f1
+        r hash.set H2 "x" f1 yy
+        assert_equal "f2 v2" [lsort [r hgetall H2]]
+        # But expired field was not lazy deleted
+        assert_equal 2 [r hlen H2]
+
+        r debug set-active-expire 1
+    } {OK} {needs:debug}
+
     test "Unload the module - hash" {
         assert_equal {OK} [r module unload hash]
     }
diff --git a/tests/unit/moduleapi/scan.tcl b/tests/unit/moduleapi/scan.tcl
index 7cf8e60af..2f0127267 100644
--- a/tests/unit/moduleapi/scan.tcl
+++ b/tests/unit/moduleapi/scan.tcl
@@ -25,12 +25,16 @@ start_server {tags {"modules"}} {
     } {{f1 1}}
 
     test {Module scan hash listpack with hexpire} {
-        r hmset hh f1 v1 f2 v2
+        r debug set-active-expire 0
+        r hmset hh f1 v1 f2 v2 f3 v3
         r hexpire hh 100000 fields 1 f1
+        r hpexpire hh 1 fields 1 f3
+        after 10
         assert_range [r httl hh fields 1 f1] 10000 100000
         assert_encoding listpackex hh
+        r debug set-active-expire 1
         lsort [r scan.scan_key hh]
-    } {{f1 v1} {f2 v2}}
+    } {{f1 v1} {f2 v2}} {needs:debug}
 
     test {Module scan hash dict} {
         r config set hash-max-ziplist-entries 2
@@ -44,10 +48,22 @@ start_server {tags {"modules"}} {
         r del hh
         r hmset hh f1 v1 f2 v2 f3 v3
         r hexpire hh 100000 fields 1 f2
+        r hpexpire hh 5 fields 1 f3
         assert_range [r httl hh fields 1 f2] 10000 100000
         assert_encoding hashtable hh
+        after 10
         lsort [r scan.scan_key hh]
-    } {{f1 v1} {f2 v2} {f3 v3}}
+    } {{f1 v1} {f2 v2}}
+
+    test {Module scan hash with hexpire can return no items} {
+        r del hh
+        r debug set-active-expire 0
+        r hmset hh f1 v1 f2 v2 f3 v3
+        r hpexpire hh 1 fields 3 f1 f2 f3
+        after 10
+        r debug set-active-expire 1
+        lsort [r scan.scan_key hh]
+    } {} {needs:debug}
 
     test {Module scan zset listpack} {
         r zadd zz 1 f1 2 f2
diff --git a/tests/unit/type/hash-field-expire.tcl b/tests/unit/type/hash-field-expire.tcl
index 1c9084861..557bd6eaf 100644
--- a/tests/unit/type/hash-field-expire.tcl
+++ b/tests/unit/type/hash-field-expire.tcl
@@ -1103,8 +1103,11 @@ start_server {tags {"external:skip needs:debug"}} {
             r hexpireat h1 [expr [clock seconds]+100] NX FIELDS 1 f1
             r hset h2 f2 v2
             r hpexpireat h2 [expr [clock seconds]*1000+100000] NX FIELDS 1 f2
-            r hset h3 f3 v3 f4 v4
+            r hset h3 f3 v3 f4 v4 f5 v5
+            # hpersist does nothing here. Verify it is not propagated.
+            r hpersist h3 FIELDS 1 f5
             r hexpire h3 100 FIELDS 3 f3 f4 non_exists_field
+            r hpersist h3 FIELDS 1 f3
 
             assert_replication_stream $repl {
                 {select *}
@@ -1112,8 +1115,9 @@ start_server {tags {"external:skip needs:debug"}} {
                 {hpexpireat h1 * NX FIELDS 1 f1}
                 {hset h2 f2 v2}
                 {hpexpireat h2 * NX FIELDS 1 f2}
-                {hset h3 f3 v3 f4 v4}
+                {hset h3 f3 v3 f4 v4 f5 v5}
                 {hpexpireat h3 * FIELDS 3 f3 f4 non_exists_field}
+                {hpersist h3 FIELDS 1 f3}
             }
             close_replication_stream $repl
         } {} {needs:repl}

From f01fdc3960f51c1bc7f0d52afff8b0b3f9865c8b Mon Sep 17 00:00:00 2001
From: Moti Cohen <moticless@gmail.com>
Date: Mon, 10 Jun 2024 16:57:26 +0300
Subject: [PATCH 02/14] Reserve 2 bits out of EB_EXPIRE_TIME_MAX for possible
 future use (#13331)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Reserve 2 bits out of hash-field expiration time (`EB_EXPIRE_TIME_MAX`)
for possible future lightweight indexing/categorizing of fields. It can
be achieved by hacking HFE as follows:
```
HPEXPIREAT key [ 2^47 + USER_INDEX ] FIELDS numfields field [field …]
```

Redis will also need to expose kind of `HEXPIRESCAN` and `HEXPIRECOUNT`
for this idea. Yet to be better defined.

`HFE_MAX_ABS_TIME_MSEC` constraint must be enforced only at API level.
Internally, the expiration time can be up to `EB_EXPIRE_TIME_MAX` for
future readiness.
---
 src/t_hash.c                          | 36 ++++++++++++---------------
 tests/integration/rdb.tcl             |  6 ++---
 tests/unit/type/hash-field-expire.tcl |  6 ++---
 3 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/src/t_hash.c b/src/t_hash.c
index afab548d7..8588ba62e 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -14,13 +14,23 @@
  * update the expiration time of the hash object in global HFE DS. */
 #define HASH_NEW_EXPIRE_DIFF_THRESHOLD max(4000, 1<<EB_BUCKET_KEY_PRECISION)
 
-/* Returned by hashTypeGetValue() */
+/* Reserve 2 bits out of hash-field expiration time for possible future lightweight
+ * indexing/categorizing of fields. It can be achieved by hacking HFE as follows:
+ *
+ *    HPEXPIREAT key [ 2^47 + USER_INDEX ] FIELDS numfields field [field …]
+ *
+ * Redis will also need to expose kind of HEXPIRESCAN and HEXPIRECOUNT for this
+ * idea. Yet to be better defined.
+ *
+ * HFE_MAX_ABS_TIME_MSEC constraint must be enforced only at API level. Internally,
+ * the expiration time can be up to EB_EXPIRE_TIME_MAX for future readiness.
+ */
+#define HFE_MAX_ABS_TIME_MSEC (EB_EXPIRE_TIME_MAX >> 2)
+
 typedef enum GetFieldRes {
     /* common (Used by hashTypeGet* value family) */
-    GETF_OK = 0,
+    GETF_OK = 0,            /* The field was found. */
     GETF_NOT_FOUND,         /* The field was not found. */
-
-    /* used only by hashTypeGetValue() */
     GETF_EXPIRED,           /* Logically expired (Might be lazy deleted or not) */
     GETF_EXPIRED_HASH,      /* Delete hash since retrieved field was expired and
                              * it was the last field in the hash. */
@@ -168,10 +178,7 @@ static inline int isDictWithMetaHFE(dict *d) {
 
 /* Returned value of hashTypeSetEx() */
 typedef enum SetExRes {
-    /* Common res from hashTypeSetEx() */
     HSETEX_OK =                1,   /* Expiration time set/updated as expected */
-
-    /* If provided HashTypeSetEx struct to hashTypeSetEx() */
     HSETEX_NO_FIELD =         -2,   /* No such hash-field */
     HSETEX_NO_CONDITION_MET =  0,   /* Specified NX | XX | GT | LT condition not met */
     HSETEX_DELETED =           2,   /* Field deleted because the specified time is in the past */
@@ -183,12 +190,6 @@ typedef enum GetExpireTimeRes {
     HFE_GET_NO_TTL =            -1, /* No TTL attached to the field */
 } GetExpireTimeRes;
 
-typedef enum FieldGet { /* TBD */
-    FIELD_GET_NONE = 0,
-    FIELD_GET_NEW  = 1,
-    FIELD_GET_OLD  = 2
-} FieldGet;
-
 typedef enum ExpireSetCond {
     HFE_NX = 1<<0,
     HFE_XX = 1<<1,
@@ -196,11 +197,6 @@ typedef enum ExpireSetCond {
     HFE_LT = 1<<3
 } ExpireSetCond;
 
-typedef struct HashTypeSet {
-    sds value;
-    int flags;
-} HashTypeSet;
-
 /* Used by hashTypeSetEx() for setting fields or their expiry  */
 typedef struct HashTypeSetEx {
 
@@ -2933,7 +2929,7 @@ static void hexpireGenericCommand(client *c, const char *cmd, long long basetime
     }
 
     if (unit == UNIT_SECONDS) {
-        if (expire > (long long) EB_EXPIRE_TIME_MAX / 1000) {
+        if (expire > (long long) HFE_MAX_ABS_TIME_MSEC / 1000) {
             addReplyErrorExpireTime(c);
             return;
         }
@@ -2941,7 +2937,7 @@ static void hexpireGenericCommand(client *c, const char *cmd, long long basetime
     }
 
     /* Ensure that the final absolute Unix timestamp does not exceed EB_EXPIRE_TIME_MAX. */
-    if (expire > (long long) EB_EXPIRE_TIME_MAX - basetime) {
+    if (expire > (long long) HFE_MAX_ABS_TIME_MSEC - basetime) {
         addReplyErrorExpireTime(c);
         return;
     }
diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl
index 9db781689..f528097f4 100644
--- a/tests/integration/rdb.tcl
+++ b/tests/integration/rdb.tcl
@@ -429,8 +429,8 @@ start_server [list overrides [list "dir" $server_path]] {
             r HMSET key a 1 b 2 c 3 d 4 e 5
             # expected to be expired long after restart
             r HEXPIREAT key 2524600800 FIELDS 1 a
-            # expected long TTL value (6 bytes) is saved and loaded correctly
-            r HPEXPIREAT key 188900976391764 FIELDS 1 b
+            # expected long TTL value (46 bits) is saved and loaded correctly
+            r HPEXPIREAT key 65755674080852 FIELDS 1 b
             # expected to be already expired after restart
             r HPEXPIRE key 80 FIELDS 1 d
             # expected to be expired soon after restart
@@ -443,7 +443,7 @@ start_server [list overrides [list "dir" $server_path]] {
             wait_done_loading r
 
             assert_equal [lsort [r hgetall key]] "1 2 3 a b c"
-            assert_equal [r hpexpiretime key FIELDS 3 a b c] {2524600800000 188900976391764 -1}
+            assert_equal [r hpexpiretime key FIELDS 3 a b c] {2524600800000 65755674080852 -1}
             assert_equal [s rdb_last_load_keys_loaded] 1
 
             # wait until expired_hash_fields equals 2
diff --git a/tests/unit/type/hash-field-expire.tcl b/tests/unit/type/hash-field-expire.tcl
index 557bd6eaf..58ff4cfc9 100644
--- a/tests/unit/type/hash-field-expire.tcl
+++ b/tests/unit/type/hash-field-expire.tcl
@@ -208,12 +208,12 @@ start_server {tags {"external:skip needs:debug"}} {
             assert_error {*Parameter `numFields` is more than number of arguments} {r hpexpire myhash 1000 NX FIELDS 4 f1 f2 f3}
         }
 
-        test "HPEXPIRE - parameter expire-time near limit of  2^48 ($type)" {
+        test "HPEXPIRE - parameter expire-time near limit of  2^46 ($type)" {
             r del myhash
             r hset myhash f1 v1
             # below & above
-            assert_equal [r hpexpire myhash [expr (1<<48) - [clock milliseconds] - 1000 ] FIELDS 1 f1] [list  $E_OK]
-            assert_error {*invalid expire time*} {r hpexpire myhash [expr (1<<48) - [clock milliseconds] + 100 ] FIELDS 1 f1}
+            assert_equal [r hpexpire myhash [expr (1<<46) - [clock milliseconds] - 1000 ] FIELDS 1 f1] [list  $E_OK]
+            assert_error {*invalid expire time*} {r hpexpire myhash [expr (1<<46) - [clock milliseconds] + 100 ] FIELDS 1 f1}
         }
 
         test "Lazy Expire - fields are lazy deleted ($type)" {

From ed10f737b8e71014fef01af1ccaca61d01f153a1 Mon Sep 17 00:00:00 2001
From: "debing.sun" <debing.sun@redis.com>
Date: Tue, 11 Jun 2024 21:42:34 +0800
Subject: [PATCH 03/14] Add new hexpired notification for HFE (#13329)

When the hash field expired, we will send a new `hexpired` notification.
It mainly includes the following three cases:
1. When field expired by active expiration.
2. When field expired by lazy expiration.
3. When the user uses the `h(p)expire(at)` command, the user will also
get a `hexpired` notification if the field expires during the command.

## Improvement
1. Now if more than one field expires in the hmget command, we will only
send a `hexpired` notification.
2. When a field with TTL is deleted by commands like hdel without
updating the global DS, active expire will not send a notification.

---------

Co-authored-by: Ozan Tezcan <ozantezcan@gmail.com>
Co-authored-by: Moti Cohen <moti.cohen@redis.com>
---
 src/server.h          |  2 ++
 src/t_hash.c          | 75 ++++++++++++++++++++++++++-----------------
 tests/unit/pubsub.tcl | 37 +++++++++++++++++++--
 3 files changed, 82 insertions(+), 32 deletions(-)

diff --git a/src/server.h b/src/server.h
index 59bad41ab..cff87e10f 100644
--- a/src/server.h
+++ b/src/server.h
@@ -3195,6 +3195,8 @@ typedef struct dictExpireMetadata {
 #define HFE_LAZY_EXPIRE           (0) /* Delete expired field, and if last field also the hash */
 #define HFE_LAZY_AVOID_FIELD_DEL  (1<<0) /* Avoid deleting expired field */
 #define HFE_LAZY_AVOID_HASH_DEL   (1<<1) /* Avoid deleting hash if the field is the last one */
+#define HFE_LAZY_NO_NOTIFICATION  (1<<2) /* Do not send notification, used when multiple fields
+                                          * may expire and only one notification is desired. */
 
 void hashTypeConvert(robj *o, int enc, ebuckets *hexpires);
 void hashTypeTryConversion(redisDb *db, robj *subject, robj **argv, int start, int end);
diff --git a/src/t_hash.c b/src/t_hash.c
index 8588ba62e..1c5481bdb 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -750,16 +750,19 @@ GetFieldRes hashTypeGetValue(redisDb *db, robj *o, sds field, unsigned char **vs
     propagateHashFieldDeletion(db, key, field, sdslen(field));
 
     /* If the field is the last one in the hash, then the hash will be deleted */
+    res = GETF_EXPIRED;
+    robj *keyObj = createStringObject(key, sdslen(key));
+    if (!(hfeFlags & HFE_LAZY_NO_NOTIFICATION))
+        notifyKeyspaceEvent(NOTIFY_HASH, "hexpired", keyObj, db->id);
     if ((hashTypeLength(o, 0) == 0) && (!(hfeFlags & HFE_LAZY_AVOID_HASH_DEL))) {
-        robj *keyObj = createStringObject(key, sdslen(key));
-        notifyKeyspaceEvent(NOTIFY_GENERIC, "del", keyObj, db->id);
+        if (!(hfeFlags & HFE_LAZY_NO_NOTIFICATION))
+            notifyKeyspaceEvent(NOTIFY_GENERIC, "del", keyObj, db->id);
         dbDelete(db,keyObj);
-        signalModifiedKey(NULL, db, keyObj);
-        decrRefCount(keyObj);
-        return GETF_EXPIRED_HASH;
+        res = GETF_EXPIRED_HASH;
     }
-
-    return GETF_EXPIRED;
+    signalModifiedKey(NULL, db, keyObj);
+    decrRefCount(keyObj);
+    return res;
 }
 
 /* Like hashTypeGetValue() but returns a Redis object, which is useful for
@@ -1155,10 +1158,12 @@ void hashTypeSetExDone(HashTypeSetEx *ex) {
         if (ex->fieldDeleted && hashTypeLength(ex->hashObj, 0) == 0) {
             dbDelete(ex->db,ex->key);
             signalModifiedKey(ex->c, ex->db, ex->key);
+            notifyKeyspaceEvent(NOTIFY_HASH, "hexpired", ex->key, ex->db->id);
             notifyKeyspaceEvent(NOTIFY_GENERIC,"del",ex->key, ex->db->id);
         } else {
             signalModifiedKey(ex->c, ex->db, ex->key);
-            notifyKeyspaceEvent(NOTIFY_HASH, "hexpire", ex->key, ex->db->id);
+            notifyKeyspaceEvent(NOTIFY_HASH, ex->fieldDeleted ? "hexpired" : "hexpire",
+                                ex->key, ex->db->id);
 
             /* If minimum HFE of the hash is smaller than expiration time of the
              * specified fields in the command as well as it is smaller or equal
@@ -1819,16 +1824,23 @@ static ExpireAction hashTypeActiveExpire(eItem _hashObj, void *ctx) {
     /* Update quota left */
     activeExpireCtx->fieldsToExpireQuota -= info.itemsExpired;
 
+    /* In some cases, a field might have been deleted without updating the global DS.
+     * As a result, active-expire might not expire any fields, in such cases,
+     * we don't need to send notifications or perform other operations for this key. */
+    if (info.itemsExpired) {
+        robj *key = createStringObject(keystr, sdslen(keystr));
+        notifyKeyspaceEvent(NOTIFY_HASH,"hexpired",key,activeExpireCtx->db->id);
+        if (hashTypeLength(hashObj, 0) == 0) {
+            dbDelete(activeExpireCtx->db, key);
+            notifyKeyspaceEvent(NOTIFY_GENERIC,"del",key,activeExpireCtx->db->id);
+        }
+        server.dirty++;
+        signalModifiedKey(NULL, activeExpireCtx->db, key);
+        decrRefCount(key);
+    }
+
     /* If hash has no more fields to expire, remove it from HFE DB */
     if (info.nextExpireTime == EB_EXPIRE_TIME_INVALID) {
-        if (hashTypeLength(hashObj, 0) == 0) {
-            robj *key = createStringObject(keystr, sdslen(keystr));
-            dbDelete(activeExpireCtx->db, key);
-            notifyKeyspaceEvent(NOTIFY_GENERIC,"del",key, activeExpireCtx->db->id);
-            server.dirty++;
-            signalModifiedKey(NULL, &server.db[0], key);
-            decrRefCount(key);
-        }
         return ACT_REMOVE_EXP_ITEM;
     } else {
         /* Hash has more fields to expire. Update next expiration time of the hash
@@ -2164,7 +2176,7 @@ void hincrbyfloatCommand(client *c) {
     decrRefCount(newobj);
 }
 
-static GetFieldRes addHashFieldToReply(client *c, robj *o, sds field) {
+static GetFieldRes addHashFieldToReply(client *c, robj *o, sds field, int hfeFlags) {
     if (o == NULL) {
         addReplyNull(c);
         return GETF_NOT_FOUND;
@@ -2174,8 +2186,7 @@ static GetFieldRes addHashFieldToReply(client *c, robj *o, sds field) {
     unsigned int vlen = UINT_MAX;
     long long vll = LLONG_MAX;
 
-    GetFieldRes res = hashTypeGetValue(c->db, o, field, &vstr, &vlen, &vll,
-                                       HFE_LAZY_EXPIRE);
+    GetFieldRes res = hashTypeGetValue(c->db, o, field, &vstr, &vlen, &vll, hfeFlags);
     if (res == GETF_OK) {
         if (vstr) {
             addReplyBulkCBuffer(c, vstr, vlen);
@@ -2194,13 +2205,14 @@ void hgetCommand(client *c) {
     if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.null[c->resp])) == NULL ||
         checkType(c,o,OBJ_HASH)) return;
 
-    addHashFieldToReply(c, o, c->argv[2]->ptr);
+    addHashFieldToReply(c, o, c->argv[2]->ptr, HFE_LAZY_EXPIRE);
 }
 
 void hmgetCommand(client *c) {
     GetFieldRes res = GETF_OK;
     robj *o;
     int i;
+    int expired = 0, deleted = 0;
 
     /* Don't abort when the key cannot be found. Non-existing keys are empty
      * hashes, where HMGET should respond with a series of null bulks. */
@@ -2209,17 +2221,22 @@ void hmgetCommand(client *c) {
 
     addReplyArrayLen(c, c->argc-2);
     for (i = 2; i < c->argc ; i++) {
-
-        res = addHashFieldToReply(c, o, c->argv[i]->ptr);
-
-        /* If hash got lazy expired since all fields are expired (o is invalid),
-         * then fill the rest with trivial nulls and return */
-        if (res == GETF_EXPIRED_HASH) {
-            while (++i < c->argc)
-                addReplyNull(c);
-            return;
+        if (!deleted) {
+            res = addHashFieldToReply(c, o, c->argv[i]->ptr, HFE_LAZY_NO_NOTIFICATION);
+            expired += (res == GETF_EXPIRED);
+            deleted += (res == GETF_EXPIRED_HASH);
+        } else {
+            /* If hash got lazy expired since all fields are expired (o is invalid),
+             * then fill the rest with trivial nulls and return. */
+            addReplyNull(c);
         }
     }
+
+    if (expired) {
+        notifyKeyspaceEvent(NOTIFY_HASH, "hexpired", c->argv[1], c->db->id);
+        if (deleted)
+            notifyKeyspaceEvent(NOTIFY_GENERIC, "del", c->argv[1], c->db->id); 
+    }
 }
 
 void hdelCommand(client *c) {
diff --git a/tests/unit/pubsub.tcl b/tests/unit/pubsub.tcl
index 153ba059c..5ac3e8252 100644
--- a/tests/unit/pubsub.tcl
+++ b/tests/unit/pubsub.tcl
@@ -356,16 +356,17 @@ start_server {tags {"pubsub network"}} {
     foreach {type max_lp_entries} {listpackex 512 hashtable 0} {
     test "Keyspace notifications: hash events test ($type)" {
         r config set hash-max-listpack-entries $max_lp_entries
-        r config set notify-keyspace-events Kh
+        r config set notify-keyspace-events Khg
         r del myhash
         set rd1 [redis_deferring_client]
         assert_equal {1} [psubscribe $rd1 *]
-        r hmset myhash yes 1 no 0
+        r hmset myhash yes 1 no 0 f1 1 f2 2 f3_hdel 3
         r hincrby myhash yes 10
         r hexpire myhash 999999 FIELDS 1 yes
         r hexpireat myhash [expr {[clock seconds] + 999999}] NX FIELDS 1 no
         r hpexpire myhash 999999 FIELDS 1 yes
         r hpersist myhash FIELDS 1 yes
+        r hpexpire myhash 0 FIELDS 1 yes
         assert_encoding $type myhash
         assert_equal "pmessage * __keyspace@${db}__:myhash hset" [$rd1 read]
         assert_equal "pmessage * __keyspace@${db}__:myhash hincrby" [$rd1 read]
@@ -373,8 +374,38 @@ start_server {tags {"pubsub network"}} {
         assert_equal "pmessage * __keyspace@${db}__:myhash hexpire" [$rd1 read]
         assert_equal "pmessage * __keyspace@${db}__:myhash hexpire" [$rd1 read]
         assert_equal "pmessage * __keyspace@${db}__:myhash hpersist" [$rd1 read]
+        assert_equal "pmessage * __keyspace@${db}__:myhash hexpired" [$rd1 read]
+
+        # Test that we will get `hexpired` notification when
+        # a hash field is removed by active expire.
+        r hpexpire myhash 10 FIELDS 1 no
+        after 100 ;# Wait for active expire
+        assert_equal "pmessage * __keyspace@${db}__:myhash hexpire" [$rd1 read]
+        assert_equal "pmessage * __keyspace@${db}__:myhash hexpired" [$rd1 read]
+
+        # Test that when a field with TTL is deleted by commands like hdel without
+        # updating the global DS, active expire will not send a notification.
+        r hpexpire myhash 100 FIELDS 1 f3_hdel
+        r hdel myhash f3_hdel
+        after 200 ;# Wait for active expire
+        assert_equal "pmessage * __keyspace@${db}__:myhash hexpire" [$rd1 read]
+        assert_equal "pmessage * __keyspace@${db}__:myhash hdel" [$rd1 read]
+
+        # Test that we will get `hexpired` notification when
+        # a hash field is removed by lazy expire.
+        r debug set-active-expire 0
+        r hpexpire myhash 10 FIELDS 2 f1 f2
+        after 20
+        r hmget myhash f1 f2 ;# Trigger lazy expire
+        assert_equal "pmessage * __keyspace@${db}__:myhash hexpire" [$rd1 read]
+        # We should get only one `hexpired` notification even two fields was expired.
+        assert_equal "pmessage * __keyspace@${db}__:myhash hexpired" [$rd1 read]
+        # We should get a `del` notification after all fields were expired.
+        assert_equal "pmessage * __keyspace@${db}__:myhash del" [$rd1 read]
+        r debug set-active-expire 1
+
         $rd1 close
-    }
+    } {0} {needs:debug}
     } ;# foreach
 
     test "Keyspace notifications: stream events test" {

From 871c985919650b056d5d102b90e10917be92f5b9 Mon Sep 17 00:00:00 2001
From: Jo <10510431+j178@users.noreply.github.com>
Date: Fri, 14 Jun 2024 13:51:49 +0800
Subject: [PATCH 04/14] Update `FIELDS` argument to block type for HFE commands
 schema (#13339)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

I reviewed `XREAD` command syntax:
```
XREAD [COUNT count] [BLOCK milliseconds] STREAMS key [key ...] id [id ...]
```

Here’s the structure for `XREAD`:
```json
"arguments": [
            {
                "token": "COUNT",
                "name": "count",
                "type": "integer",
                "optional": true
            },
            {
                "token": "BLOCK",
                "name": "milliseconds",
                "type": "integer",
                "optional": true
            },
            {
                "name": "streams",
                "token": "STREAMS",
                "type": "block",
                "arguments": [
                    {
                        "name": "key",
                        "type": "key",
                        "key_spec_index": 0,
                        "multiple": true
                    },
                    {
                        "name": "ID",
                        "type": "string",
                        "multiple": true
                    }
                ]
            }
]
```

Now, consider the `HEXPIRE` syntax:
```
HEXPIRE key seconds [NX | XX | GT | LT] FIELDS numfields field [field ...]
```

Since the `FIELDS` token functions similarly to `STREAMS`, and given that `STREAMS` is defined as a block, I believe the `FIELDS` in `hepxire` should also be defined as a block.
---
 src/commands.def               | 108 ++++++++++++++++++++++-----------
 src/commands/hexpire.json      |  25 ++++----
 src/commands/hexpireat.json    |  25 ++++----
 src/commands/hexpiretime.json  |  25 ++++----
 src/commands/hpersist.json     |  25 ++++----
 src/commands/hpexpire.json     |  25 ++++----
 src/commands/hpexpireat.json   |  25 ++++----
 src/commands/hpexpiretime.json |  25 ++++----
 src/commands/hpttl.json        |  25 ++++----
 src/commands/httl.json         |  25 ++++----
 10 files changed, 198 insertions(+), 135 deletions(-)

diff --git a/src/commands.def b/src/commands.def
index dea507cf6..ff8b81d41 100644
--- a/src/commands.def
+++ b/src/commands.def
@@ -3330,14 +3330,18 @@ struct COMMAND_ARG HEXPIRE_condition_Subargs[] = {
 {MAKE_ARG("lt",ARG_TYPE_PURE_TOKEN,-1,"LT",NULL,NULL,CMD_ARG_NONE,0,NULL)},
 };
 
+/* HEXPIRE fields argument table */
+struct COMMAND_ARG HEXPIRE_fields_Subargs[] = {
+{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
+{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+};
+
 /* HEXPIRE argument table */
 struct COMMAND_ARG HEXPIRE_Args[] = {
 {MAKE_ARG("key",ARG_TYPE_KEY,0,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
 {MAKE_ARG("seconds",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
 {MAKE_ARG("condition",ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,CMD_ARG_OPTIONAL,4,NULL),.subargs=HEXPIRE_condition_Subargs},
-{MAKE_ARG("fields",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+{MAKE_ARG("fields",ARG_TYPE_BLOCK,-1,"FIELDS",NULL,NULL,CMD_ARG_NONE,2,NULL),.subargs=HEXPIRE_fields_Subargs},
 };
 
 /********** HEXPIREAT ********************/
@@ -3367,14 +3371,18 @@ struct COMMAND_ARG HEXPIREAT_condition_Subargs[] = {
 {MAKE_ARG("lt",ARG_TYPE_PURE_TOKEN,-1,"LT",NULL,NULL,CMD_ARG_NONE,0,NULL)},
 };
 
+/* HEXPIREAT fields argument table */
+struct COMMAND_ARG HEXPIREAT_fields_Subargs[] = {
+{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
+{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+};
+
 /* HEXPIREAT argument table */
 struct COMMAND_ARG HEXPIREAT_Args[] = {
 {MAKE_ARG("key",ARG_TYPE_KEY,0,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
 {MAKE_ARG("unix-time-seconds",ARG_TYPE_UNIX_TIME,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
 {MAKE_ARG("condition",ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,CMD_ARG_OPTIONAL,4,NULL),.subargs=HEXPIREAT_condition_Subargs},
-{MAKE_ARG("fields",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+{MAKE_ARG("fields",ARG_TYPE_BLOCK,-1,"FIELDS",NULL,NULL,CMD_ARG_NONE,2,NULL),.subargs=HEXPIREAT_fields_Subargs},
 };
 
 /********** HEXPIRETIME ********************/
@@ -3396,12 +3404,16 @@ keySpec HEXPIRETIME_Keyspecs[1] = {
 };
 #endif
 
+/* HEXPIRETIME fields argument table */
+struct COMMAND_ARG HEXPIRETIME_fields_Subargs[] = {
+{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
+{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+};
+
 /* HEXPIRETIME argument table */
 struct COMMAND_ARG HEXPIRETIME_Args[] = {
 {MAKE_ARG("key",ARG_TYPE_KEY,0,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("fields",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+{MAKE_ARG("fields",ARG_TYPE_BLOCK,-1,"FIELDS",NULL,NULL,CMD_ARG_NONE,2,NULL),.subargs=HEXPIRETIME_fields_Subargs},
 };
 
 /********** HGET ********************/
@@ -3632,12 +3644,16 @@ keySpec HPERSIST_Keyspecs[1] = {
 };
 #endif
 
+/* HPERSIST fields argument table */
+struct COMMAND_ARG HPERSIST_fields_Subargs[] = {
+{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
+{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+};
+
 /* HPERSIST argument table */
 struct COMMAND_ARG HPERSIST_Args[] = {
 {MAKE_ARG("key",ARG_TYPE_KEY,0,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("fields",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+{MAKE_ARG("fields",ARG_TYPE_BLOCK,-1,"FIELDS",NULL,NULL,CMD_ARG_NONE,2,NULL),.subargs=HPERSIST_fields_Subargs},
 };
 
 /********** HPEXPIRE ********************/
@@ -3667,14 +3683,18 @@ struct COMMAND_ARG HPEXPIRE_condition_Subargs[] = {
 {MAKE_ARG("lt",ARG_TYPE_PURE_TOKEN,-1,"LT",NULL,NULL,CMD_ARG_NONE,0,NULL)},
 };
 
+/* HPEXPIRE fields argument table */
+struct COMMAND_ARG HPEXPIRE_fields_Subargs[] = {
+{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
+{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+};
+
 /* HPEXPIRE argument table */
 struct COMMAND_ARG HPEXPIRE_Args[] = {
 {MAKE_ARG("key",ARG_TYPE_KEY,0,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
 {MAKE_ARG("milliseconds",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
 {MAKE_ARG("condition",ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,CMD_ARG_OPTIONAL,4,NULL),.subargs=HPEXPIRE_condition_Subargs},
-{MAKE_ARG("fields",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+{MAKE_ARG("fields",ARG_TYPE_BLOCK,-1,"FIELDS",NULL,NULL,CMD_ARG_NONE,2,NULL),.subargs=HPEXPIRE_fields_Subargs},
 };
 
 /********** HPEXPIREAT ********************/
@@ -3704,14 +3724,18 @@ struct COMMAND_ARG HPEXPIREAT_condition_Subargs[] = {
 {MAKE_ARG("lt",ARG_TYPE_PURE_TOKEN,-1,"LT",NULL,NULL,CMD_ARG_NONE,0,NULL)},
 };
 
+/* HPEXPIREAT fields argument table */
+struct COMMAND_ARG HPEXPIREAT_fields_Subargs[] = {
+{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
+{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+};
+
 /* HPEXPIREAT argument table */
 struct COMMAND_ARG HPEXPIREAT_Args[] = {
 {MAKE_ARG("key",ARG_TYPE_KEY,0,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
 {MAKE_ARG("unix-time-milliseconds",ARG_TYPE_UNIX_TIME,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
 {MAKE_ARG("condition",ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,CMD_ARG_OPTIONAL,4,NULL),.subargs=HPEXPIREAT_condition_Subargs},
-{MAKE_ARG("fields",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+{MAKE_ARG("fields",ARG_TYPE_BLOCK,-1,"FIELDS",NULL,NULL,CMD_ARG_NONE,2,NULL),.subargs=HPEXPIREAT_fields_Subargs},
 };
 
 /********** HPEXPIRETIME ********************/
@@ -3733,12 +3757,16 @@ keySpec HPEXPIRETIME_Keyspecs[1] = {
 };
 #endif
 
+/* HPEXPIRETIME fields argument table */
+struct COMMAND_ARG HPEXPIRETIME_fields_Subargs[] = {
+{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
+{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+};
+
 /* HPEXPIRETIME argument table */
 struct COMMAND_ARG HPEXPIRETIME_Args[] = {
 {MAKE_ARG("key",ARG_TYPE_KEY,0,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("fields",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+{MAKE_ARG("fields",ARG_TYPE_BLOCK,-1,"FIELDS",NULL,NULL,CMD_ARG_NONE,2,NULL),.subargs=HPEXPIRETIME_fields_Subargs},
 };
 
 /********** HPTTL ********************/
@@ -3760,12 +3788,16 @@ keySpec HPTTL_Keyspecs[1] = {
 };
 #endif
 
+/* HPTTL fields argument table */
+struct COMMAND_ARG HPTTL_fields_Subargs[] = {
+{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
+{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+};
+
 /* HPTTL argument table */
 struct COMMAND_ARG HPTTL_Args[] = {
 {MAKE_ARG("key",ARG_TYPE_KEY,0,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("fields",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+{MAKE_ARG("fields",ARG_TYPE_BLOCK,-1,"FIELDS",NULL,NULL,CMD_ARG_NONE,2,NULL),.subargs=HPTTL_fields_Subargs},
 };
 
 /********** HRANDFIELD ********************/
@@ -3934,12 +3966,16 @@ keySpec HTTL_Keyspecs[1] = {
 };
 #endif
 
+/* HTTL fields argument table */
+struct COMMAND_ARG HTTL_fields_Subargs[] = {
+{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
+{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+};
+
 /* HTTL argument table */
 struct COMMAND_ARG HTTL_Args[] = {
 {MAKE_ARG("key",ARG_TYPE_KEY,0,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("fields",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("numfields",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
-{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+{MAKE_ARG("fields",ARG_TYPE_BLOCK,-1,"FIELDS",NULL,NULL,CMD_ARG_NONE,2,NULL),.subargs=HTTL_fields_Subargs},
 };
 
 /********** HVALS ********************/
@@ -10993,9 +11029,9 @@ struct COMMAND_STRUCT redisCommandTable[] = {
 /* hash */
 {MAKE_CMD("hdel","Deletes one or more fields and their values from a hash. Deletes the hash if no fields remain.","O(N) where N is the number of fields to be removed.","2.0.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HDEL_History,1,HDEL_Tips,0,hdelCommand,-3,CMD_WRITE|CMD_FAST,ACL_CATEGORY_HASH,HDEL_Keyspecs,1,NULL,2),.args=HDEL_Args},
 {MAKE_CMD("hexists","Determines whether a field exists in a hash.","O(1)","2.0.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HEXISTS_History,0,HEXISTS_Tips,0,hexistsCommand,3,CMD_READONLY|CMD_FAST,ACL_CATEGORY_HASH,HEXISTS_Keyspecs,1,NULL,2),.args=HEXISTS_Args},
-{MAKE_CMD("hexpire","Set expiry for hash field using relative time to expire (seconds)","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HEXPIRE_History,0,HEXPIRE_Tips,0,hexpireCommand,-6,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_HASH,HEXPIRE_Keyspecs,1,NULL,6),.args=HEXPIRE_Args},
-{MAKE_CMD("hexpireat","Set expiry for hash field using an absolute Unix timestamp (seconds)","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HEXPIREAT_History,0,HEXPIREAT_Tips,0,hexpireatCommand,-6,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_HASH,HEXPIREAT_Keyspecs,1,NULL,6),.args=HEXPIREAT_Args},
-{MAKE_CMD("hexpiretime","Returns the expiration time of a hash field as a Unix timestamp, in seconds.","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HEXPIRETIME_History,0,HEXPIRETIME_Tips,0,hexpiretimeCommand,-5,CMD_READONLY|CMD_FAST,ACL_CATEGORY_HASH,HEXPIRETIME_Keyspecs,1,NULL,4),.args=HEXPIRETIME_Args},
+{MAKE_CMD("hexpire","Set expiry for hash field using relative time to expire (seconds)","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HEXPIRE_History,0,HEXPIRE_Tips,0,hexpireCommand,-6,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_HASH,HEXPIRE_Keyspecs,1,NULL,4),.args=HEXPIRE_Args},
+{MAKE_CMD("hexpireat","Set expiry for hash field using an absolute Unix timestamp (seconds)","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HEXPIREAT_History,0,HEXPIREAT_Tips,0,hexpireatCommand,-6,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_HASH,HEXPIREAT_Keyspecs,1,NULL,4),.args=HEXPIREAT_Args},
+{MAKE_CMD("hexpiretime","Returns the expiration time of a hash field as a Unix timestamp, in seconds.","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HEXPIRETIME_History,0,HEXPIRETIME_Tips,0,hexpiretimeCommand,-5,CMD_READONLY|CMD_FAST,ACL_CATEGORY_HASH,HEXPIRETIME_Keyspecs,1,NULL,2),.args=HEXPIRETIME_Args},
 {MAKE_CMD("hget","Returns the value of a field in a hash.","O(1)","2.0.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HGET_History,0,HGET_Tips,0,hgetCommand,3,CMD_READONLY|CMD_FAST,ACL_CATEGORY_HASH,HGET_Keyspecs,1,NULL,2),.args=HGET_Args},
 {MAKE_CMD("hgetall","Returns all fields and values in a hash.","O(N) where N is the size of the hash.","2.0.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HGETALL_History,0,HGETALL_Tips,1,hgetallCommand,2,CMD_READONLY,ACL_CATEGORY_HASH,HGETALL_Keyspecs,1,NULL,1),.args=HGETALL_Args},
 {MAKE_CMD("hincrby","Increments the integer value of a field in a hash by a number. Uses 0 as initial value if the field doesn't exist.","O(1)","2.0.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HINCRBY_History,0,HINCRBY_Tips,0,hincrbyCommand,4,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_HASH,HINCRBY_Keyspecs,1,NULL,3),.args=HINCRBY_Args},
@@ -11004,17 +11040,17 @@ struct COMMAND_STRUCT redisCommandTable[] = {
 {MAKE_CMD("hlen","Returns the number of fields in a hash.","O(1)","2.0.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HLEN_History,0,HLEN_Tips,0,hlenCommand,2,CMD_READONLY|CMD_FAST,ACL_CATEGORY_HASH,HLEN_Keyspecs,1,NULL,1),.args=HLEN_Args},
 {MAKE_CMD("hmget","Returns the values of all fields in a hash.","O(N) where N is the number of fields being requested.","2.0.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HMGET_History,0,HMGET_Tips,0,hmgetCommand,-3,CMD_READONLY|CMD_FAST,ACL_CATEGORY_HASH,HMGET_Keyspecs,1,NULL,2),.args=HMGET_Args},
 {MAKE_CMD("hmset","Sets the values of multiple fields.","O(N) where N is the number of fields being set.","2.0.0",CMD_DOC_DEPRECATED,"`HSET` with multiple field-value pairs","4.0.0","hash",COMMAND_GROUP_HASH,HMSET_History,0,HMSET_Tips,0,hsetCommand,-4,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_HASH,HMSET_Keyspecs,1,NULL,2),.args=HMSET_Args},
-{MAKE_CMD("hpersist","Removes the expiration time for each specified field","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HPERSIST_History,0,HPERSIST_Tips,0,hpersistCommand,-5,CMD_WRITE|CMD_FAST,ACL_CATEGORY_HASH,HPERSIST_Keyspecs,1,NULL,4),.args=HPERSIST_Args},
-{MAKE_CMD("hpexpire","Set expiry for hash field using relative time to expire (milliseconds)","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HPEXPIRE_History,0,HPEXPIRE_Tips,0,hpexpireCommand,-6,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_HASH,HPEXPIRE_Keyspecs,1,NULL,6),.args=HPEXPIRE_Args},
-{MAKE_CMD("hpexpireat","Set expiry for hash field using an absolute Unix timestamp (milliseconds)","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HPEXPIREAT_History,0,HPEXPIREAT_Tips,0,hpexpireatCommand,-6,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_HASH,HPEXPIREAT_Keyspecs,1,NULL,6),.args=HPEXPIREAT_Args},
-{MAKE_CMD("hpexpiretime","Returns the expiration time of a hash field as a Unix timestamp, in msec.","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HPEXPIRETIME_History,0,HPEXPIRETIME_Tips,0,hpexpiretimeCommand,-5,CMD_READONLY|CMD_FAST,ACL_CATEGORY_HASH,HPEXPIRETIME_Keyspecs,1,NULL,4),.args=HPEXPIRETIME_Args},
-{MAKE_CMD("hpttl","Returns the TTL in milliseconds of a hash field.","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HPTTL_History,0,HPTTL_Tips,0,hpttlCommand,-5,CMD_READONLY|CMD_FAST,ACL_CATEGORY_HASH,HPTTL_Keyspecs,1,NULL,4),.args=HPTTL_Args},
+{MAKE_CMD("hpersist","Removes the expiration time for each specified field","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HPERSIST_History,0,HPERSIST_Tips,0,hpersistCommand,-5,CMD_WRITE|CMD_FAST,ACL_CATEGORY_HASH,HPERSIST_Keyspecs,1,NULL,2),.args=HPERSIST_Args},
+{MAKE_CMD("hpexpire","Set expiry for hash field using relative time to expire (milliseconds)","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HPEXPIRE_History,0,HPEXPIRE_Tips,0,hpexpireCommand,-6,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_HASH,HPEXPIRE_Keyspecs,1,NULL,4),.args=HPEXPIRE_Args},
+{MAKE_CMD("hpexpireat","Set expiry for hash field using an absolute Unix timestamp (milliseconds)","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HPEXPIREAT_History,0,HPEXPIREAT_Tips,0,hpexpireatCommand,-6,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_HASH,HPEXPIREAT_Keyspecs,1,NULL,4),.args=HPEXPIREAT_Args},
+{MAKE_CMD("hpexpiretime","Returns the expiration time of a hash field as a Unix timestamp, in msec.","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HPEXPIRETIME_History,0,HPEXPIRETIME_Tips,0,hpexpiretimeCommand,-5,CMD_READONLY|CMD_FAST,ACL_CATEGORY_HASH,HPEXPIRETIME_Keyspecs,1,NULL,2),.args=HPEXPIRETIME_Args},
+{MAKE_CMD("hpttl","Returns the TTL in milliseconds of a hash field.","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HPTTL_History,0,HPTTL_Tips,0,hpttlCommand,-5,CMD_READONLY|CMD_FAST,ACL_CATEGORY_HASH,HPTTL_Keyspecs,1,NULL,2),.args=HPTTL_Args},
 {MAKE_CMD("hrandfield","Returns one or more random fields from a hash.","O(N) where N is the number of fields returned","6.2.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HRANDFIELD_History,0,HRANDFIELD_Tips,1,hrandfieldCommand,-2,CMD_READONLY,ACL_CATEGORY_HASH,HRANDFIELD_Keyspecs,1,NULL,2),.args=HRANDFIELD_Args},
 {MAKE_CMD("hscan","Iterates over fields and values of a hash.","O(1) for every call. O(N) for a complete iteration, including enough command calls for the cursor to return back to 0. N is the number of elements inside the collection.","2.8.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HSCAN_History,0,HSCAN_Tips,1,hscanCommand,-3,CMD_READONLY,ACL_CATEGORY_HASH,HSCAN_Keyspecs,1,NULL,5),.args=HSCAN_Args},
 {MAKE_CMD("hset","Creates or modifies the value of a field in a hash.","O(1) for each field/value pair added, so O(N) to add N field/value pairs when the command is called with multiple field/value pairs.","2.0.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HSET_History,1,HSET_Tips,0,hsetCommand,-4,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_HASH,HSET_Keyspecs,1,NULL,2),.args=HSET_Args},
 {MAKE_CMD("hsetnx","Sets the value of a field in a hash only when the field doesn't exist.","O(1)","2.0.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HSETNX_History,0,HSETNX_Tips,0,hsetnxCommand,4,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_HASH,HSETNX_Keyspecs,1,NULL,3),.args=HSETNX_Args},
 {MAKE_CMD("hstrlen","Returns the length of the value of a field.","O(1)","3.2.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HSTRLEN_History,0,HSTRLEN_Tips,0,hstrlenCommand,3,CMD_READONLY|CMD_FAST,ACL_CATEGORY_HASH,HSTRLEN_Keyspecs,1,NULL,2),.args=HSTRLEN_Args},
-{MAKE_CMD("httl","Returns the TTL in seconds of a hash field.","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HTTL_History,0,HTTL_Tips,0,httlCommand,-5,CMD_READONLY|CMD_FAST,ACL_CATEGORY_HASH,HTTL_Keyspecs,1,NULL,4),.args=HTTL_Args},
+{MAKE_CMD("httl","Returns the TTL in seconds of a hash field.","O(N) where N is the number of specified fields","7.4.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HTTL_History,0,HTTL_Tips,0,httlCommand,-5,CMD_READONLY|CMD_FAST,ACL_CATEGORY_HASH,HTTL_Keyspecs,1,NULL,2),.args=HTTL_Args},
 {MAKE_CMD("hvals","Returns all values in a hash.","O(N) where N is the size of the hash.","2.0.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HVALS_History,0,HVALS_Tips,1,hvalsCommand,2,CMD_READONLY,ACL_CATEGORY_HASH,HVALS_Keyspecs,1,NULL,1),.args=HVALS_Args},
 /* hyperloglog */
 {MAKE_CMD("pfadd","Adds elements to a HyperLogLog key. Creates the key if it doesn't exist.","O(1) to add every element.","2.8.9",CMD_DOC_NONE,NULL,NULL,"hyperloglog",COMMAND_GROUP_HYPERLOGLOG,PFADD_History,0,PFADD_Tips,0,pfaddCommand,-2,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_HYPERLOGLOG,PFADD_Keyspecs,1,NULL,2),.args=PFADD_Args},
diff --git a/src/commands/hexpire.json b/src/commands/hexpire.json
index 8a2154785..832c182ae 100644
--- a/src/commands/hexpire.json
+++ b/src/commands/hexpire.json
@@ -99,17 +99,20 @@
                 ]
             },
             {
-                "name": "FIELDS",
-                "type": "string"
-            },
-            {
-                "name": "numfields",
-                "type": "integer"
-            },
-            {
-                "name": "field",
-                "type": "string",
-                "multiple": true
+                "name": "fields",
+                "token": "FIELDS",
+                "type": "block",
+                "arguments": [
+                    {
+                        "name": "numfields",
+                        "type": "integer"
+                    },
+                    {
+                        "name": "field",
+                        "type": "string",
+                        "multiple": true
+                    }
+                ]
             }
         ]
     }
diff --git a/src/commands/hexpireat.json b/src/commands/hexpireat.json
index 9ad178276..4a7c0c718 100644
--- a/src/commands/hexpireat.json
+++ b/src/commands/hexpireat.json
@@ -99,17 +99,20 @@
                 ]
             },
             {
-                "name": "FIELDS",
-                "type": "string"
-            },
-            {
-                "name": "numfields",
-                "type": "integer"
-            },
-            {
-                "name": "field",
-                "type": "string",
-                "multiple": true
+                "name": "fields",
+                "token": "FIELDS",
+                "type": "block",
+                "arguments": [
+                    {
+                        "name": "numfields",
+                        "type": "integer"
+                    },
+                    {
+                        "name": "field",
+                        "type": "string",
+                        "multiple": true
+                    }
+                ]
             }
         ]
     }
diff --git a/src/commands/hexpiretime.json b/src/commands/hexpiretime.json
index c35b5d39d..28c1e5f4b 100644
--- a/src/commands/hexpiretime.json
+++ b/src/commands/hexpiretime.json
@@ -64,17 +64,20 @@
                 "key_spec_index": 0
             },
             {
-                "name": "FIELDS",
-                "type": "string"
-            },
-            {
-                "name": "numfields",
-                "type": "integer"
-            },
-            {
-                "name": "field",
-                "type": "string",
-                "multiple": true
+                "name": "fields",
+                "token": "FIELDS",
+                "type": "block",
+                "arguments": [
+                    {
+                        "name": "numfields",
+                        "type": "integer"
+                    },
+                    {
+                        "name": "field",
+                        "type": "string",
+                        "multiple": true
+                    }
+                ]
             }
         ]
     }
diff --git a/src/commands/hpersist.json b/src/commands/hpersist.json
index ba79044d1..e7c1cb11b 100644
--- a/src/commands/hpersist.json
+++ b/src/commands/hpersist.json
@@ -63,17 +63,20 @@
                 "key_spec_index": 0
             },
             {
-                "name": "FIELDS",
-                "type": "string"
-            },
-            {
-                "name": "numfields",
-                "type": "integer"
-            },
-            {
-                "name": "field",
-                "type": "string",
-                "multiple": true
+                "name": "fields",
+                "token": "FIELDS",
+                "type": "block",
+                "arguments": [
+                    {
+                        "name": "numfields",
+                        "type": "integer"
+                    },
+                    {
+                        "name": "field",
+                        "type": "string",
+                        "multiple": true
+                    }
+                ]
             }
         ]
     }
diff --git a/src/commands/hpexpire.json b/src/commands/hpexpire.json
index 6820987c6..02c68e616 100644
--- a/src/commands/hpexpire.json
+++ b/src/commands/hpexpire.json
@@ -99,17 +99,20 @@
                 ]
             },
             {
-                "name": "FIELDS",
-                "type": "string"
-            },
-            {
-                "name": "numfields",
-                "type": "integer"
-            },
-            {
-                "name": "field",
-                "type": "string",
-                "multiple": true
+                "name": "fields",
+                "token": "FIELDS",
+                "type": "block",
+                "arguments": [
+                    {
+                        "name": "numfields",
+                        "type": "integer"
+                    },
+                    {
+                        "name": "field",
+                        "type": "string",
+                        "multiple": true
+                    }
+                ]
             }
         ]
     }
diff --git a/src/commands/hpexpireat.json b/src/commands/hpexpireat.json
index 0d08bb46e..58e5555fb 100644
--- a/src/commands/hpexpireat.json
+++ b/src/commands/hpexpireat.json
@@ -99,17 +99,20 @@
                 ]
             },
             {
-                "name": "FIELDS",
-                "type": "string"
-            },
-            {
-                "name": "numfields",
-                "type": "integer"
-            },
-            {
-                "name": "field",
-                "type": "string",
-                "multiple": true
+                "name": "fields",
+                "token": "FIELDS",
+                "type": "block",
+                "arguments": [
+                    {
+                        "name": "numfields",
+                        "type": "integer"
+                    },
+                    {
+                        "name": "field",
+                        "type": "string",
+                        "multiple": true
+                    }
+                ]
             }
         ]
     }
diff --git a/src/commands/hpexpiretime.json b/src/commands/hpexpiretime.json
index 83fd4610d..67406cb7d 100644
--- a/src/commands/hpexpiretime.json
+++ b/src/commands/hpexpiretime.json
@@ -64,17 +64,20 @@
                 "key_spec_index": 0
             },
             {
-                "name": "FIELDS",
-                "type": "string"
-            },
-            {
-                "name": "numfields",
-                "type": "integer"
-            },
-            {
-                "name": "field",
-                "type": "string",
-                "multiple": true
+                "name": "fields",
+                "token": "FIELDS",
+                "type": "block",
+                "arguments": [
+                    {
+                        "name": "numfields",
+                        "type": "integer"
+                    },
+                    {
+                        "name": "field",
+                        "type": "string",
+                        "multiple": true
+                    }
+                ]
             }
         ]
     }
diff --git a/src/commands/hpttl.json b/src/commands/hpttl.json
index 7aa3eb72d..9f24bec8f 100644
--- a/src/commands/hpttl.json
+++ b/src/commands/hpttl.json
@@ -64,17 +64,20 @@
                 "key_spec_index": 0
             },
             {
-                "name": "FIELDS",
-                "type": "string"
-            },
-            {
-                "name": "numfields",
-                "type": "integer"
-            },
-            {
-                "name": "field",
-                "type": "string",
-                "multiple": true
+                "name": "fields",
+                "token": "FIELDS",
+                "type": "block",
+                "arguments": [
+                    {
+                        "name": "numfields",
+                        "type": "integer"
+                    },
+                    {
+                        "name": "field",
+                        "type": "string",
+                        "multiple": true
+                    }
+                ]
             }
         ]
     }
diff --git a/src/commands/httl.json b/src/commands/httl.json
index f7c7ce7b5..e0e865056 100644
--- a/src/commands/httl.json
+++ b/src/commands/httl.json
@@ -64,17 +64,20 @@
                 "key_spec_index": 0
             },
             {
-                "name": "FIELDS",
-                "type": "string"
-            },
-            {
-                "name": "numfields",
-                "type": "integer"
-            },
-            {
-                "name": "field",
-                "type": "string",
-                "multiple": true
+                "name": "fields",
+                "token": "FIELDS",
+                "type": "block",
+                "arguments": [
+                    {
+                        "name": "numfields",
+                        "type": "integer"
+                    },
+                    {
+                        "name": "field",
+                        "type": "string",
+                        "multiple": true
+                    }
+                ]
             }
         ]
     }

From 4aa25d042cff5ba63f244a752887bf4be369c937 Mon Sep 17 00:00:00 2001
From: Ozan Tezcan <ozantezcan@gmail.com>
Date: Fri, 14 Jun 2024 09:35:05 +0300
Subject: [PATCH 05/14] Reply with array of return codes if the key does not
 exist for HFE commands (#13343)

Currently, HFE commands reply with empty array if the key does not
exist. Though, non-existing key and empty key is the same thing.
It means fields given in the command do not exist in the empty key.
So, replying with an array of 'no field' error codes (-2) suits better
to Redis logic. e.g. Similarly, `hmget` returns array of nulls if the
key does not exist.

After this PR:
```
127.0.0.1:6379> hpersist missingkey fields 2 a b
1) (integer) -2
2) (integer) -2
```
---
 src/t_hash.c                          | 45 +++++++++++++++++++++++----
 tests/unit/type/hash-field-expire.tcl | 41 +++++++++---------------
 2 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/src/t_hash.c b/src/t_hash.c
index 1c5481bdb..8af25c830 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -2811,8 +2811,9 @@ static void httlGenericCommand(client *c, const char *cmd, long long basetime, i
     long numFields = 0, numFieldsAt = 3;
 
     /* Read the hash object */
-    if ((hashObj = lookupKeyReadOrReply(c, c->argv[1], shared.emptyarray)) == NULL ||
-        checkType(c, hashObj, OBJ_HASH)) return;
+    hashObj = lookupKeyRead(c->db, c->argv[1]);
+    if (checkType(c, hashObj, OBJ_HASH))
+        return;
 
     if (strcasecmp(c->argv[numFieldsAt-1]->ptr, "FIELDS")) {
         addReplyError(c, "Mandatory argument FIELDS is missing or not at the right position");
@@ -2830,6 +2831,16 @@ static void httlGenericCommand(client *c, const char *cmd, long long basetime, i
         return;
     }
 
+    /* Non-existing keys and empty hashes are the same thing. It also means
+     * fields in the command don't exist in the hash key. */
+    if (!hashObj) {
+        addReplyArrayLen(c, numFields);
+        for (int i = 0; i < numFields; i++) {
+            addReplyLongLong(c, HFE_GET_NO_FIELD);
+        }
+        return;
+    }
+
     if (hashObj->encoding == OBJ_ENCODING_LISTPACK) {
         void *lp = hashObj->ptr;
 
@@ -2934,8 +2945,9 @@ static void hexpireGenericCommand(client *c, const char *cmd, long long basetime
     robj *hashObj, *keyArg = c->argv[1], *expireArg = c->argv[2];
 
     /* Read the hash object */
-    if ((hashObj = lookupKeyWriteOrReply(c, keyArg, shared.emptyarray)) == NULL ||
-        checkType(c, hashObj, OBJ_HASH)) return;
+    hashObj = lookupKeyWrite(c->db, keyArg);
+    if (checkType(c, hashObj, OBJ_HASH))
+        return;
 
     /* Read the expiry time from command */
     if (getLongLongFromObjectOrReply(c, expireArg, &expire, NULL) != C_OK)
@@ -2988,6 +3000,16 @@ static void hexpireGenericCommand(client *c, const char *cmd, long long basetime
         return;
     }
 
+    /* Non-existing keys and empty hashes are the same thing. It also means
+     * fields in the command don't exist in the hash key. */
+    if (!hashObj) {
+        addReplyArrayLen(c, numFields);
+        for (int i = 0; i < numFields; i++) {
+            addReplyLongLong(c, HSETEX_NO_FIELD);
+        }
+        return;
+    }
+
     HashTypeSetEx exCtx;
     hashTypeSetExInit(keyArg, hashObj, c, c->db, cmd, expireSetCond, &exCtx);
     addReplyArrayLen(c, numFields);
@@ -3062,8 +3084,9 @@ void hpersistCommand(client *c) {
     int changed = 0; /* Used to determine whether to send a notification. */
 
     /* Read the hash object */
-    if ((hashObj = lookupKeyWriteOrReply(c, c->argv[1], shared.emptyarray)) == NULL ||
-        checkType(c, hashObj, OBJ_HASH)) return;
+    hashObj = lookupKeyWrite(c->db, c->argv[1]);
+    if (checkType(c, hashObj, OBJ_HASH))
+        return;
 
     if (strcasecmp(c->argv[numFieldsAt-1]->ptr, "FIELDS")) {
         addReplyError(c, "Mandatory argument FIELDS is missing or not at the right position");
@@ -3081,6 +3104,16 @@ void hpersistCommand(client *c) {
         return;
     }
 
+    /* Non-existing keys and empty hashes are the same thing. It also means
+     * fields in the command don't exist in the hash key. */
+    if (!hashObj) {
+        addReplyArrayLen(c, numFields);
+        for (int i = 0; i < numFields; i++) {
+            addReplyLongLong(c, HFE_PERSIST_NO_FIELD);
+        }
+        return;
+    }
+
     if (hashObj->encoding == OBJ_ENCODING_LISTPACK) {
         addReplyArrayLen(c, numFields);
         for (int i = 0 ; i < numFields ; i++) {
diff --git a/tests/unit/type/hash-field-expire.tcl b/tests/unit/type/hash-field-expire.tcl
index 58ff4cfc9..f78f2b142 100644
--- a/tests/unit/type/hash-field-expire.tcl
+++ b/tests/unit/type/hash-field-expire.tcl
@@ -117,15 +117,12 @@ start_server {tags {"external:skip needs:debug"}} {
             r config set hash-max-listpack-entries 512
         }
 
-        test "HEXPIRE/HEXPIREAT/HPEXPIRE/HPEXPIREAT - Returns empty array if key does not exist" {
+        test "HEXPIRE/HEXPIREAT/HPEXPIRE/HPEXPIREAT - Returns array if the key does not exist" {
             r del myhash
-            # Make sure we can distinguish between an empty array and a null response
-            r readraw 1
-            assert_equal {*0} [r HEXPIRE myhash 1000 FIELDS 1 a]
-            assert_equal {*0} [r HEXPIREAT myhash 1000 FIELDS 1 a]
-            assert_equal {*0} [r HPEXPIRE myhash 1000 FIELDS 1 a]
-            assert_equal {*0} [r HPEXPIREAT myhash 1000 FIELDS 1 a]
-            r readraw 0
+            assert_equal [r HEXPIRE myhash 1000 FIELDS 1 a] [list $E_NO_FIELD]
+            assert_equal [r HEXPIREAT myhash 1000 FIELDS 1 a] [list $E_NO_FIELD]
+            assert_equal [r HPEXPIRE myhash 1000 FIELDS 2 a b] [list $E_NO_FIELD $E_NO_FIELD]
+            assert_equal [r HPEXPIREAT myhash 1000 FIELDS 2 a b] [list $E_NO_FIELD $E_NO_FIELD]
         }
 
         test "HEXPIRE/HEXPIREAT/HPEXPIRE/HPEXPIREAT - Verify that the expire time does not overflow" {
@@ -305,13 +302,10 @@ start_server {tags {"external:skip needs:debug"}} {
             r flushall async
         }
 
-        test "HTTL/HPTTL - Returns empty array if key does not exist" {
+        test "HTTL/HPTTL - Returns array if the key does not exist" {
             r del myhash
-            # Make sure we can distinguish between an empty array and a null response
-            r readraw 1
-            assert_equal {*0} [r HTTL myhash FIELDS 1 a]
-            assert_equal {*0} [r HPTTL myhash FIELDS 1 a]
-            r readraw 0
+            assert_equal [r HTTL myhash FIELDS 1 a] [list $T_NO_FIELD]
+            assert_equal [r HPTTL myhash FIELDS 2 a b] [list $T_NO_FIELD $T_NO_FIELD]
         }
 
         test "HTTL/HPTTL - Input validation gets failed on nonexists field or field without expire ($type)" {
@@ -320,7 +314,6 @@ start_server {tags {"external:skip needs:debug"}} {
             r HPEXPIRE myhash 1000 NX FIELDS 1 field1
 
             foreach cmd {HTTL HPTTL} {
-                assert_equal [r $cmd non_exists_key FIELDS 1 f] {}
                 assert_equal [r $cmd myhash FIELDS 2 field2 non_exists_field] "$T_NO_EXPIRY $T_NO_FIELD"
                 # Set numFields less than actual number of fields. Fine.
                 assert_equal [r $cmd myhash FIELDS 1 non_exists_field1 non_exists_field2] "$T_NO_FIELD"
@@ -337,13 +330,10 @@ start_server {tags {"external:skip needs:debug"}} {
             assert_range $ttl 1000 2000
         }
 
-        test "HEXPIRETIME/HPEXPIRETIME - Returns empty array if key does not exist" {
+        test "HEXPIRETIME/HPEXPIRETIME - Returns array if the key does not exist" {
             r del myhash
-            # Make sure we can distinguish between an empty array and a null response
-            r readraw 1
-            assert_equal {*0} [r HEXPIRETIME myhash FIELDS 1 a]
-            assert_equal {*0} [r HPEXPIRETIME myhash FIELDS 1 a]
-            r readraw 0
+            assert_equal [r HEXPIRETIME myhash FIELDS 1 a] [list $T_NO_FIELD]
+            assert_equal [r HPEXPIRETIME myhash FIELDS 2 a b] [list $T_NO_FIELD $T_NO_FIELD]
         }
 
         test "HEXPIRETIME - returns TTL in Unix timestamp ($type)" {
@@ -711,12 +701,10 @@ start_server {tags {"external:skip needs:debug"}} {
             r debug set-active-expire 1
         }
 
-        test "HPERSIST - Returns empty array if key does not exist ($type)" {
+        test "HPERSIST - Returns array if the key does not exist ($type)" {
             r del myhash
-            # Make sure we can distinguish between an empty array and a null response
-            r readraw 1
-            assert_equal {*0} [r HPERSIST myhash FIELDS 1 a]
-            r readraw 0
+            assert_equal [r HPERSIST myhash FIELDS 1 a] [list $P_NO_FIELD]
+            assert_equal [r HPERSIST myhash FIELDS 2 a b] [list $P_NO_FIELD $P_NO_FIELD]
         }
 
         test "HPERSIST - input validation ($type)" {
@@ -726,7 +714,6 @@ start_server {tags {"external:skip needs:debug"}} {
             r hexpire myhash 1000 NX FIELDS 1 f1
             assert_error {*wrong number of arguments*} {r hpersist myhash}
             assert_error {*wrong number of arguments*} {r hpersist myhash FIELDS 1}
-            assert_equal [r hpersist not-exists-key FIELDS 1 f1] {}
             assert_equal [r hpersist myhash FIELDS 2 f1 not-exists-field] "$P_OK $P_NO_FIELD"
             assert_equal [r hpersist myhash FIELDS 1 f2] "$P_NO_EXPIRY"
         }

From 24c85cc36807a354361bdf273dad4f48eaa5ffca Mon Sep 17 00:00:00 2001
From: "Filipe Oliveira (Redis)"
 <52153106+fcostaoliveira@users.noreply.github.com>
Date: Tue, 18 Jun 2024 11:00:47 +0100
Subject: [PATCH 06/14] reduce getNodeByQuery CPU time by using less cache
 lines (from 2064 Bytes struct to 64 Bytes): reduces LLC misses and Memory
 Loads (#13296)

The following PR goes from 33 cacheline on getKeysResult struct (by
default has 256 static buffer)

```
root@hpe10:~/redis# pahole -p   ./src/server.o -C getKeysResult
typedef struct {
	keyReference               keysbuf[256];         /*     0  2048 */
	/* --- cacheline 32 boundary (2048 bytes) --- */
	/* typedef keyReference */ struct {
		int                pos;
		int                flags;
	} *keys; /*  2048     8 */
	int                        numkeys;              /*  2056     4 */
	int                        size;                 /*  2060     4 */

	/* size: 2064, cachelines: 33, members: 4 */
	/* last cacheline: 16 bytes */
} getKeysResult;
```


to 1 cacheline with a static buffer of 6 keys per command):
```
root@hpe10:~/redis# pahole -p   ./src/server.o -C getKeysResult
typedef struct {
	int                        numkeys;              /*     0     4 */
	int                        size;                 /*     4     4 */
	keyReference               keysbuf[6];           /*     8    48 */
	/* typedef keyReference */ struct {
		int                pos;
		int                flags;
	} *keys; /*    56     8 */

	/* size: 64, cachelines: 1, members: 4 */
} getKeysResult;
```

we get around 1.5% higher ops/sec, and a confirmation of around 15% less
LLC loads on getNodeByQuery and 37% less Stores.

Function / Call Stack | CPU Time: Difference | CPU Time:
9462436fa444e746716845b1d807c74d8945831b | CPU Time: this PR | Loads:
Difference | Loads: 9462436fa444e746716845b1d807c74d8945831b | Loads:
this PR | Stores: Difference | Stores:
9462436fa444e746716845b1d807c74d8945831b | Stores: This PR
-- | -- | -- | -- | -- | -- | -- | -- | -- | --
getNodeByQuery | 0.753767 | 1.57118 | 0.817416 | 144297829 (15% less
loads) | 920575969 | 776278140 | 367607824 (37% less stores) | 991642384
| 624034560

## results on client side

### baseline
```
taskset -c 2,3 memtier_benchmark -s 192.168.1.200 --port 6379 --authenticate perf --cluster-mode --pipeline 10 --data-size 100 --ratio 1:0 --key-pattern P:P --key-minimum=1 --key-maximum 1000000 --test-time 180 -c 25 -t 2 --hide-histogram
Writing results to stdout
[RUN #1] Preparing benchmark client...
[RUN #1] Launching threads now...
[RUN #1 100%, 180 secs]  0 threads:   110333450 ops,  604992 (avg:  612942) ops/sec, 84.75MB/sec (avg: 85.86MB/sec),  0.82 (avg:  0.81) msec latency

2         Threads
25        Connections per thread
180       Seconds


ALL STATS
======================================================================================================================================================
Type         Ops/sec     Hits/sec   Misses/sec    MOVED/sec      ASK/sec    Avg. Latency     p50 Latency     p99 Latency   p99.9 Latency       KB/sec
------------------------------------------------------------------------------------------------------------------------------------------------------
Sets       612942.14          ---          ---         0.00         0.00         0.81332         0.80700         1.26300         2.92700     87924.12
Gets            0.00         0.00         0.00         0.00         0.00             ---             ---             ---             ---         0.00
Waits           0.00          ---          ---          ---          ---             ---             ---             ---             ---          ---
Totals     612942.14         0.00         0.00         0.00         0.00         0.81332         0.80700         1.26300         2.92700     87924.12
```

### comparison
```
taskset -c 2,3 memtier_benchmark -s 192.168.1.200 --port 6379 --authenticate perf --cluster-mode --pipeline 10 --data-size 100 --ratio 1:0 --key-pattern P:P --key-minimum=1 --key-maximum 1000000 --test-time 180 -c 25 -t 2 --hide-histogram
Writing results to stdout
[RUN #1] Preparing benchmark client...
[RUN #1] Launching threads now...
[RUN #1 100%, 180 secs]  0 threads:   111731310 ops,  610195 (avg:  620707) ops/sec, 85.48MB/sec (avg: 86.95MB/sec),  0.82 (avg:  0.80) msec latency

2         Threads
25        Connections per thread
180       Seconds


ALL STATS
======================================================================================================================================================
Type         Ops/sec     Hits/sec   Misses/sec    MOVED/sec      ASK/sec    Avg. Latency     p50 Latency     p99 Latency   p99.9 Latency       KB/sec
------------------------------------------------------------------------------------------------------------------------------------------------------
Sets       620707.72          ---          ---         0.00         0.00         0.80312         0.79900         1.23900         2.87900     89037.78
Gets            0.00         0.00         0.00         0.00         0.00             ---             ---             ---             ---         0.00
Waits           0.00          ---          ---          ---          ---             ---             ---             ---             ---          ---
Totals     620707.72         0.00         0.00         0.00         0.00         0.80312         0.79900         1.23900         2.87900     89037.78
```

Co-authored-by: filipecosta90 <filipecosta.90@gmail.com>
---
 src/server.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/server.h b/src/server.h
index cff87e10f..dbb1b9621 100644
--- a/src/server.h
+++ b/src/server.h
@@ -2071,7 +2071,8 @@ struct redisServer {
     char *locale_collate;
 };
 
-#define MAX_KEYS_BUFFER 256
+/* we use 6 so that all getKeyResult fits a cacheline */
+#define MAX_KEYS_BUFFER 6
 
 typedef struct {
     int pos; /* The position of the key within the client array */
@@ -2084,12 +2085,12 @@ typedef struct {
  * for returning channel information.
  */
 typedef struct {
+    int numkeys;                                 /* Number of key indices return */
+    int size;                                    /* Available array size */
     keyReference keysbuf[MAX_KEYS_BUFFER];       /* Pre-allocated buffer, to save heap allocations */
     keyReference *keys;                          /* Key indices array, points to keysbuf or heap */
-    int numkeys;                        /* Number of key indices return */
-    int size;                           /* Available array size */
 } getKeysResult;
-#define GETKEYS_RESULT_INIT { {{0}}, NULL, 0, MAX_KEYS_BUFFER }
+#define GETKEYS_RESULT_INIT { 0, MAX_KEYS_BUFFER, {{0}}, NULL }
 
 /* Key specs definitions.
  *

From e18a173a810725723468eb8e877a970001a250d4 Mon Sep 17 00:00:00 2001
From: Moti Cohen <moticless@gmail.com>
Date: Thu, 20 Jun 2024 13:53:47 +0300
Subject: [PATCH 07/14] Fix rdbLoadObject() empty hash (#13347)

As part of HFE feature, the logic of rdbLoadObject() was wrongly
modified to indicate of loaded empty hash from RDB as hash that all its
fields got expired. Rollback to `emptykey` logic. This function should
load blindly all fields, expired or not. Manually verified.

Few more minor fixes:
- remove hash double check of emptyKey
- Fix from `sds` to `hfield` in rdbLoadObject() (not really a bug. Both
are of type char*)
- Revert code rdbLoadObject() to get dbid instead of db
---
 src/cluster.c         | 10 +++++----
 src/rdb.c             | 49 +++++++++++--------------------------------
 src/rdb.h             |  5 ++---
 src/redis-check-rdb.c |  4 +++-
 src/t_hash.c          |  4 +++-
 5 files changed, 26 insertions(+), 46 deletions(-)

diff --git a/src/cluster.c b/src/cluster.c
index 9472a018d..fa3e6da3a 100644
--- a/src/cluster.c
+++ b/src/cluster.c
@@ -176,7 +176,6 @@ void dumpCommand(client *c) {
 
 /* RESTORE key ttl serialized-value [REPLACE] [ABSTTL] [IDLETIME seconds] [FREQ frequency] */
 void restoreCommand(client *c) {
-    uint64_t minExpiredField = EB_EXPIRE_TIME_INVALID;
     long long ttl, lfu_freq = -1, lru_idle = -1, lru_clock = -1;
     rio payload;
     int j, type, replace = 0, absttl = 0;
@@ -240,7 +239,7 @@ void restoreCommand(client *c) {
 
     rioInitWithBuffer(&payload,c->argv[3]->ptr);
     if (((type = rdbLoadObjectType(&payload)) == -1) ||
-        ((obj = rdbLoadObject(type,&payload,key->ptr,c->db,NULL, &minExpiredField)) == NULL))
+        ((obj = rdbLoadObject(type,&payload,key->ptr,c->db->id,NULL)) == NULL))
     {
         addReplyError(c,"Bad data format");
         return;
@@ -270,8 +269,11 @@ void restoreCommand(client *c) {
 
     /* If minExpiredField was set, then the object is hash with expiration
      * on fields and need to register it in global HFE DS */
-    if (minExpiredField != EB_EXPIRE_TIME_INVALID)
-        hashTypeAddToExpires(c->db, dictGetKey(de), obj, minExpiredField);
+    if (obj->type == OBJ_HASH) {
+        uint64_t minExpiredField = hashTypeGetNextTimeToExpire(obj);
+        if (minExpiredField != EB_EXPIRE_TIME_INVALID)
+            hashTypeAddToExpires(c->db, dictGetKey(de), obj, minExpiredField);
+    }
 
     if (ttl) {
         setExpire(c,c->db,key,ttl);
diff --git a/src/rdb.c b/src/rdb.c
index 4330c5694..a4749eb22 100644
--- a/src/rdb.c
+++ b/src/rdb.c
@@ -1897,10 +1897,8 @@ int lpValidateIntegrityAndDups(unsigned char *lp, size_t size, int deep, int tup
  *   no fields with expiration or it is not a hash, then it will set be to
  *   EB_EXPIRE_TIME_INVALID.
  */
-robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, redisDb* db, int *error,
-                    uint64_t *minExpiredField)
+robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error)
 {
-    uint64_t minExpField = EB_EXPIRE_TIME_INVALID;
     robj *o = NULL, *ele, *dec;
     uint64_t len;
     unsigned int i;
@@ -2241,8 +2239,8 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, redisDb* db, int *error,
         /* All pairs should be read by now */
         serverAssert(len == 0);
     } else if (rdbtype == RDB_TYPE_HASH_METADATA) {
-        size_t fieldLen;
-        sds value, field;
+        sds value;
+        hfield field;
         uint64_t expireAt;
         dict *dupSearchDict = NULL;
 
@@ -2285,9 +2283,9 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, redisDb* db, int *error,
 
             /* if needed create field with TTL metadata  */
             if (expireAt !=0)
-                field = rdbGenericLoadStringObject(rdb, RDB_LOAD_HFLD_TTL, &fieldLen);
+                field = rdbGenericLoadStringObject(rdb, RDB_LOAD_HFLD_TTL, NULL);
             else
-                field = rdbGenericLoadStringObject(rdb, RDB_LOAD_HFLD, &fieldLen);
+                field = rdbGenericLoadStringObject(rdb, RDB_LOAD_HFLD, NULL);
 
             if (field == NULL) {
                 serverLog(LL_WARNING, "failed reading hash field");
@@ -2305,9 +2303,6 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, redisDb* db, int *error,
                 return NULL;
             }
 
-            /* keep the nearest expiration to connect listpack object to db expiry */
-            if ((expireAt != 0) && (expireAt < minExpField)) minExpField = expireAt;
-
             /* store the values read - either to listpack or dict */
             if (o->encoding == OBJ_ENCODING_LISTPACK_EX) {
                 /* integrity - check for key duplication (if required) */
@@ -2378,11 +2373,6 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, redisDb* db, int *error,
 
         if (dupSearchDict != NULL) dictRelease(dupSearchDict);
 
-        /* check for empty key (if all fields were expired) */
-        if (hashTypeLength(o, 0) == 0) {
-            decrRefCount(o);
-            goto expiredHash;
-        }
     } else if (rdbtype == RDB_TYPE_LIST_QUICKLIST || rdbtype == RDB_TYPE_LIST_QUICKLIST_2) {
         if ((len = rdbLoadLen(rdb,NULL)) == RDB_LENERR) return NULL;
         if (len == 0) goto emptykey;
@@ -2706,9 +2696,6 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, redisDb* db, int *error,
                     goto emptykey;
                 }
 
-                /* for TTL listpack, find the minimum expiry */
-                minExpField = hashTypeGetNextTimeToExpire(o);
-
                 /* Convert listpack to hash table without registering in global HFE DS,
                  * if has HFEs, since the listpack is not connected yet to the DB */
                 if (hashTypeLength(o, 0) > server.hash_max_listpack_entries)
@@ -3053,13 +3040,7 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, redisDb* db, int *error,
         RedisModuleIO io;
         robj keyobj;
         initStaticStringObject(keyobj,key);
-        /* shouldn't happen since db is NULL only in RDB check mode, and
-         * in this mode the module load code returns few lines above after
-         * checking module name, few lines above. So this check is only
-         * for safety.
-         */
-        if (db == NULL) return NULL;
-        moduleInitIOContext(io,mt,rdb,&keyobj,db->id);
+        moduleInitIOContext(io,mt,rdb,&keyobj,dbid);
         /* Call the rdb_load method of the module providing the 10 bit
          * encoding version in the lower 10 bits of the module ID. */
         void *ptr = mt->rdb_load(&io,moduleid&1023);
@@ -3099,17 +3080,12 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, redisDb* db, int *error,
         return NULL;
     }
 
-    if (minExpiredField) *minExpiredField = minExpField;
-
     if (error) *error = 0;
     return o;
 
 emptykey:
     if (error) *error = RDB_LOAD_ERR_EMPTY_KEY;
     return NULL;
-expiredHash:
-    if (error) *error = RDB_LOAD_ERR_EXPIRED_HASH;
-    return NULL;
 }
 
 /* Mark that we are loading in the global state and setup the fields
@@ -3279,7 +3255,6 @@ int rdbLoadRio(rio *rdb, int rdbflags, rdbSaveInfo *rsi) {
  * currently it only allow to set db object and functionLibCtx to which the data
  * will be loaded (in the future it might contains more such objects). */
 int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadingCtx *rdb_loading_ctx) {
-    uint64_t minExpiredField = EB_EXPIRE_TIME_INVALID;
     uint64_t dbid = 0;
     int type, rdbver;
     uint64_t db_size = 0, expires_size = 0;
@@ -3521,7 +3496,7 @@ int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadin
         if ((key = rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) == NULL)
             goto eoferr;
         /* Read value */
-        val = rdbLoadObject(type,rdb,key,db,&error, &minExpiredField);
+        val = rdbLoadObject(type,rdb,key,db->id,&error);
 
         /* Check if the key already expired. This function is used when loading
          * an RDB file from disk, either at startup, or when an RDB was
@@ -3540,9 +3515,6 @@ int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadin
                 if(empty_keys_skipped++ < 10)
                     serverLog(LL_NOTICE, "rdbLoadObject skipping empty key: %s", key);
                 sdsfree(key);
-            } else if (error == RDB_LOAD_ERR_EXPIRED_HASH) {
-                /* Valid flow. Continue. */
-                sdsfree(key);
             } else {
                 sdsfree(key);
                 goto eoferr;
@@ -3589,8 +3561,11 @@ int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadin
 
             /* If minExpiredField was set, then the object is hash with expiration
              * on fields and need to register it in global HFE DS */
-             if (minExpiredField != EB_EXPIRE_TIME_INVALID)
-                hashTypeAddToExpires(db, key, val, minExpiredField);
+            if (val->type == OBJ_HASH) {
+                uint64_t minExpiredField = hashTypeGetNextTimeToExpire(val);
+                if (minExpiredField != EB_EXPIRE_TIME_INVALID)
+                    hashTypeAddToExpires(db, key, val, minExpiredField);
+            }
 
             /* Set the expire time if needed */
             if (expiretime != -1) {
diff --git a/src/rdb.h b/src/rdb.h
index f34e139c1..65da19322 100644
--- a/src/rdb.h
+++ b/src/rdb.h
@@ -121,8 +121,7 @@
 /* When rdbLoadObject() returns NULL, the err flag is
  * set to hold the type of error that occurred */
 #define RDB_LOAD_ERR_EMPTY_KEY       1   /* Error of empty key */
-#define RDB_LOAD_ERR_EXPIRED_HASH    2   /* Expired hash since all its fields are expired */
-#define RDB_LOAD_ERR_OTHER           3   /* Any other errors */
+#define RDB_LOAD_ERR_OTHER           2   /* Any other errors */
 
 ssize_t rdbWriteRaw(rio *rdb, void *p, size_t len);
 int rdbSaveType(rio *rdb, unsigned char type);
@@ -143,7 +142,7 @@ int rdbSaveToFile(const char *filename);
 int rdbSave(int req, char *filename, rdbSaveInfo *rsi, int rdbflags);
 ssize_t rdbSaveObject(rio *rdb, robj *o, robj *key, int dbid);
 size_t rdbSavedObjectLen(robj *o, robj *key, int dbid);
-robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, redisDb *db, int *error, uint64_t *minExpiredField);
+robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error);
 void backgroundSaveDoneHandler(int exitcode, int bysignal);
 int rdbSaveKeyValuePair(rio *rdb, robj *key, robj *val, long long expiretime,int dbid);
 ssize_t rdbSaveSingleModuleAux(rio *rdb, int when, moduleType *mt);
diff --git a/src/redis-check-rdb.c b/src/redis-check-rdb.c
index f364bf7b7..090c1bd44 100644
--- a/src/redis-check-rdb.c
+++ b/src/redis-check-rdb.c
@@ -175,6 +175,7 @@ void rdbCheckSetupSignals(void) {
  * otherwise the already open file 'fp' is checked. */
 int redis_check_rdb(char *rdbfilename, FILE *fp) {
     uint64_t dbid;
+    int selected_dbid = -1;
     int type, rdbver;
     char buf[1024];
     long long expiretime, now = mstime();
@@ -246,6 +247,7 @@ int redis_check_rdb(char *rdbfilename, FILE *fp) {
             if ((dbid = rdbLoadLen(&rdb,NULL)) == RDB_LENERR)
                 goto eoferr;
             rdbCheckInfo("Selecting DB ID %llu", (unsigned long long)dbid);
+            selected_dbid = dbid;
             continue; /* Read type again. */
         } else if (type == RDB_OPCODE_RESIZEDB) {
             /* RESIZEDB: Hint about the size of the keys in the currently
@@ -331,7 +333,7 @@ int redis_check_rdb(char *rdbfilename, FILE *fp) {
         rdbstate.keys++;
         /* Read value */
         rdbstate.doing = RDB_CHECK_DOING_READ_OBJECT_VALUE;
-        if ((val = rdbLoadObject(type,&rdb,key->ptr,NULL,NULL,NULL)) == NULL)
+        if ((val = rdbLoadObject(type,&rdb,key->ptr,selected_dbid,NULL)) == NULL)
             goto eoferr;
         /* Check if the key already expired. */
         if (expiretime != -1 && expiretime < now)
diff --git a/src/t_hash.c b/src/t_hash.c
index 8af25c830..902094491 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -1853,7 +1853,9 @@ static ExpireAction hashTypeActiveExpire(eItem _hashObj, void *ctx) {
 /* Return the next/minimum expiry time of the hash-field. This is useful if a
  * field with the minimum expiry is deleted, and you want to get the next
  * minimum expiry. Otherwise, consider using hashTypeGetMinExpire() which will
- * be faster. If there is no field with expiry, returns EB_EXPIRE_TIME_INVALID */
+ * be faster but less accurate.
+ *
+ * Return next min expiry. If none return EB_EXPIRE_TIME_INVALID  */
 uint64_t hashTypeGetNextTimeToExpire(robj *o) {
     if (o->encoding == OBJ_ENCODING_LISTPACK) {
         return EB_EXPIRE_TIME_INVALID;

From a03b6e29a96a724341a387abfa60cb39948a9525 Mon Sep 17 00:00:00 2001
From: "debing.sun" <debing.sun@redis.com>
Date: Fri, 21 Jun 2024 17:59:49 +0800
Subject: [PATCH 08/14] Sort out mess document for RM_Replicate and
 RM_ReplicateVerbatim (#13323)

Related to #12647

1. Make clear that `RM_Replicate` and `RM_ReplicateVerbatim` are
non-thread safe.
2. Make clear that `RM_Replicate` and `RM_ReplicateVerbatim` are alwarys
wrapped into MULTI in any case.
---
 src/module.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/module.c b/src/module.c
index a279d4029..3920f1cff 100644
--- a/src/module.c
+++ b/src/module.c
@@ -3524,9 +3524,7 @@ int RM_ReplyWithLongDouble(RedisModuleCtx *ctx, long double ld) {
  *
  * The replicated commands are always wrapped into the MULTI/EXEC that
  * contains all the commands replicated in a given module command
- * execution. However the commands replicated with RedisModule_Call()
- * are the first items, the ones replicated with RedisModule_Replicate()
- * will all follow before the EXEC.
+ * execution, in the order they were executed.
  *
  * Modules should try to use one interface or the other.
  *
@@ -3548,9 +3546,8 @@ int RM_ReplyWithLongDouble(RedisModuleCtx *ctx, long double ld) {
  * the callback, and will propagate all the commands wrapped in a MULTI/EXEC
  * transaction. However when calling this function from a threaded safe context
  * that can live an undefined amount of time, and can be locked/unlocked in
- * at will, the behavior is different: MULTI/EXEC wrapper is not emitted
- * and the command specified is inserted in the AOF and replication stream
- * immediately.
+ * at will, it is important to note that this API is not thread-safe and
+ * must be executed while holding the GIL.
  *
  * #### Return value
  *
@@ -3588,15 +3585,18 @@ int RM_Replicate(RedisModuleCtx *ctx, const char *cmdname, const char *fmt, ...)
 }
 
 /* This function will replicate the command exactly as it was invoked
- * by the client. Note that this function will not wrap the command into
- * a MULTI/EXEC stanza, so it should not be mixed with other replication
- * commands.
+ * by the client. Note that the replicated commands are always wrapped
+ * into the MULTI/EXEC that contains all the commands replicated in a
+ * given module command execution, in the order they were executed.
  *
  * Basically this form of replication is useful when you want to propagate
  * the command to the slaves and AOF file exactly as it was called, since
  * the command can just be re-executed to deterministically re-create the
  * new state starting from the old one.
  *
+ * It is important to note that this API is not thread-safe and
+ * must be executed while holding the GIL.
+ *
  * The function always returns REDISMODULE_OK. */
 int RM_ReplicateVerbatim(RedisModuleCtx *ctx) {
     alsoPropagate(ctx->client->db->id,

From 811c5d7aeb0b76494d78efe61e418f574c310ec0 Mon Sep 17 00:00:00 2001
From: AcherTT <937889516@qq.com>
Date: Fri, 21 Jun 2024 18:21:25 +0800
Subject: [PATCH 09/14] Add debug script command (#13289)

Add two new debug commands for outputing script.
1. `DEBUG SCRIPT LIST`
   Output all scripts.
2. `DEBUG SCRIPT <sha1>`
    Output a specific script.

Close #3846
---
 src/debug.c  | 24 ++++++++++++++++++++++++
 src/eval.c   |  4 ++++
 src/script.h |  1 +
 3 files changed, 29 insertions(+)

diff --git a/src/debug.c b/src/debug.c
index 4b5f73061..b774ccc65 100644
--- a/src/debug.c
+++ b/src/debug.c
@@ -15,6 +15,7 @@
 #include "fpconv_dtoa.h"
 #include "cluster.h"
 #include "threads_mngr.h"
+#include "script.h"
 
 #include <arpa/inet.h>
 #include <signal.h>
@@ -1013,6 +1014,29 @@ NULL
     } else if (!strcasecmp(c->argv[1]->ptr, "dict-resizing") && c->argc == 3) {
         server.dict_resizing = atoi(c->argv[2]->ptr);
         addReply(c, shared.ok);
+    } else if (!strcasecmp(c->argv[1]->ptr,"script") && c->argc == 3) {
+        if (!strcasecmp(c->argv[2]->ptr,"list")) {
+            dictIterator *di = dictGetIterator(getLuaScripts());
+            dictEntry *de;
+            while ((de = dictNext(di)) != NULL) {
+                luaScript *script = dictGetVal(de);
+                sds *sha = dictGetKey(de);
+                serverLog(LL_WARNING, "SCRIPT SHA: %s\n%s", (char*)sha, (char*)script->body->ptr);
+            }
+            dictReleaseIterator(di);
+        } else if (sdslen(c->argv[2]->ptr) == 40) {
+            dictEntry *de;
+            if ((de = dictFind(getLuaScripts(), c->argv[2]->ptr)) == NULL) {
+                addReplyErrorObject(c, shared.noscripterr);
+                return;
+            }
+            luaScript *script = dictGetVal(de);
+            serverLog(LL_WARNING, "SCRIPT SHA: %s\n%s", (char*)c->argv[2]->ptr, (char*)script->body->ptr);
+        } else {
+            addReplySubcommandSyntaxError(c);
+            return;
+        }
+        addReply(c,shared.ok);
     } else if(!handleDebugClusterCommand(c)) {
         addReplySubcommandSyntaxError(c);
         return;
diff --git a/src/eval.c b/src/eval.c
index 02b472098..1cea9e6db 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -1737,3 +1737,7 @@ void luaLdbLineHook(lua_State *lua, lua_Debug *ar) {
         rctx->start_time = getMonotonicUs();
     }
 }
+
+dict *getLuaScripts(void) {
+    return lctx.lua_scripts;
+}
diff --git a/src/script.h b/src/script.h
index 12d01f754..8d604e493 100644
--- a/src/script.h
+++ b/src/script.h
@@ -74,6 +74,7 @@ extern scriptFlag scripts_flags_def[];
 
 void luaEnvInit(void);
 lua_State *createLuaState(void);
+dict *getLuaScripts(void);
 uint64_t scriptFlagsToCmdFlags(uint64_t cmd_flags, uint64_t script_flags);
 int scriptPrepareForRun(scriptRunCtx *r_ctx, client *engine_client, client *caller, const char *funcname, uint64_t script_flags, int ro);
 void scriptResetRun(scriptRunCtx *r_ctx);

From e26ea35cd4b683dd4bd43ef0569086f36ee65914 Mon Sep 17 00:00:00 2001
From: Moti Cohen <moticless@gmail.com>
Date: Mon, 24 Jun 2024 18:11:53 +0300
Subject: [PATCH 10/14] Adapt HRANDFIELD to HFE feature (#13348)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Considerations for the selected imp of HRANDFIELD & HFE feature:

HRANDFIELD might access any of the fields in the hash as some of them
might be expired. And so the Implementation of HRANDFIELD along with HFEs
might be one of the two options:
1. Expire hash-fields before diving into handling HRANDFIELD.
2. Refine HRANDFIELD cases to deal with expired fields.

Regarding the first option, as reference, the command RANDOMKEY also
declareson O(1) complexity, yet might be stuck on a very long (but not infinite)
loop trying to find non-expired keys. Furthermore RANDOMKEY also evicts expired
keys along the way even though it is categorized as a read-only command. Note
that the case of HRANDFIELD is more lightweight versus RANDOMKEY since
HFEs have much more effective and aggressive active-expiration for fields behind.

The second option introduces additional implementation complexity to HRANDFIELD.
We could further refine HRANDFIELD cases to differentiate between scenarios
with many expired fields versus few expired fields, and adjust based on the
percentage of expired fields. However, this approach could still lead to long
loops or necessitate expiring fields before selecting them. For the “lightweight”
cases it is also expected to have a lightweight expiration.

Considering the pros and cons, and the fact that HRANDFIELD is an infrequent
command (particularly with HFEs) and the fact we have effective active-expiration
behind for hash-fields, it is better to keep it simple and choose option number 1.

Other changes:
* Don't mark command dirty by internal hashTypeExpire(). It causes to read
  only command of HRANDFIELD to be accidently propagated (This flag
  should be indicated at higher level, by the command functions).
* Align `hashTypeExpireIfNeeded()` and `hashTypeGetValue()` to be more
  aligned with `expireIfNeeded()` logic of keyspace.
---
 src/aof.c                             |   2 +-
 src/cluster.c                         |   2 +-
 src/defrag.c                          |   2 +-
 src/rdb.c                             |   6 +-
 src/server.c                          |   4 +
 src/server.h                          |  14 +-
 src/t_hash.c                          | 395 ++++++++++++++++----------
 tests/unit/type/hash-field-expire.tcl | 125 ++++----
 8 files changed, 324 insertions(+), 226 deletions(-)

diff --git a/src/aof.c b/src/aof.c
index 17e72febb..ec631c0e2 100644
--- a/src/aof.c
+++ b/src/aof.c
@@ -1968,7 +1968,7 @@ int rewriteHashObject(rio *r, robj *key, robj *o) {
     hashTypeIterator *hi;
     long long count = 0, items = hashTypeLength(o, 0);
 
-    int isHFE = hashTypeGetMinExpire(o) != EB_EXPIRE_TIME_INVALID;
+    int isHFE = hashTypeGetMinExpire(o, 0) != EB_EXPIRE_TIME_INVALID;
     hi = hashTypeInitIterator(o);
 
     if (!isHFE) {
diff --git a/src/cluster.c b/src/cluster.c
index fa3e6da3a..d09a455b3 100644
--- a/src/cluster.c
+++ b/src/cluster.c
@@ -270,7 +270,7 @@ void restoreCommand(client *c) {
     /* If minExpiredField was set, then the object is hash with expiration
      * on fields and need to register it in global HFE DS */
     if (obj->type == OBJ_HASH) {
-        uint64_t minExpiredField = hashTypeGetNextTimeToExpire(obj);
+        uint64_t minExpiredField = hashTypeGetMinExpire(obj, 1);
         if (minExpiredField != EB_EXPIRE_TIME_INVALID)
             hashTypeAddToExpires(c->db, dictGetKey(de), obj, minExpiredField);
     }
diff --git a/src/defrag.c b/src/defrag.c
index 122598c4a..78de72248 100644
--- a/src/defrag.c
+++ b/src/defrag.c
@@ -751,7 +751,7 @@ void defragKey(defragCtx *ctx, dictEntry *de) {
     }
 
     /* Try to defrag robj and / or string value. */
-    if (unlikely(ob->type == OBJ_HASH && hashTypeGetMinExpire(ob) != EB_EXPIRE_TIME_INVALID)) {
+    if (unlikely(ob->type == OBJ_HASH && hashTypeGetMinExpire(ob, 0) != EB_EXPIRE_TIME_INVALID)) {
         /* Update its reference in the ebucket while defragging it. */
         newob = ebDefragItem(&db->hexpires, &hashExpireBucketsType, ob,
                              (ebDefragFunction *)activeDefragStringOb);
diff --git a/src/rdb.c b/src/rdb.c
index a4749eb22..c5c0b04f6 100644
--- a/src/rdb.c
+++ b/src/rdb.c
@@ -699,7 +699,7 @@ int rdbSaveObjectType(rio *rdb, robj *o) {
         else if (o->encoding == OBJ_ENCODING_LISTPACK_EX)
             return rdbSaveType(rdb,RDB_TYPE_HASH_LISTPACK_EX);
         else if (o->encoding == OBJ_ENCODING_HT) {
-            if (hashTypeGetMinExpire(o) == EB_EXPIRE_TIME_INVALID)
+            if (hashTypeGetMinExpire(o, 0) == EB_EXPIRE_TIME_INVALID)
                 return rdbSaveType(rdb,RDB_TYPE_HASH);
             else
                 return rdbSaveType(rdb,RDB_TYPE_HASH_METADATA);
@@ -960,7 +960,7 @@ ssize_t rdbSaveObject(rio *rdb, robj *o, robj *key, int dbid) {
              * RDB_TYPE_HASH_METADATA layout, including tuples of [ttl][field][value].
              * Otherwise, use the standard RDB_TYPE_HASH layout containing only
              * the tuples [field][value]. */
-            int with_ttl = (hashTypeGetMinExpire(o) != EB_EXPIRE_TIME_INVALID);
+            int with_ttl = (hashTypeGetMinExpire(o, 0) != EB_EXPIRE_TIME_INVALID);
 
             /* save number of fields in hash */
             if ((n = rdbSaveLen(rdb,dictSize((dict*)o->ptr))) == -1) {
@@ -3562,7 +3562,7 @@ int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadin
             /* If minExpiredField was set, then the object is hash with expiration
              * on fields and need to register it in global HFE DS */
             if (val->type == OBJ_HASH) {
-                uint64_t minExpiredField = hashTypeGetNextTimeToExpire(val);
+                uint64_t minExpiredField = hashTypeGetMinExpire(val, 1);
                 if (minExpiredField != EB_EXPIRE_TIME_INVALID)
                     hashTypeAddToExpires(db, key, val, minExpiredField);
             }
diff --git a/src/server.c b/src/server.c
index 2815f1010..11646e256 100644
--- a/src/server.c
+++ b/src/server.c
@@ -334,6 +334,10 @@ uint64_t dictObjHash(const void *key) {
     return dictGenHashFunction(o->ptr, sdslen((sds)o->ptr));
 }
 
+uint64_t dictPtrHash(const void *key) {
+    return dictGenHashFunction((unsigned char*)&key,sizeof(key));
+}
+
 uint64_t dictSdsHash(const void *key) {
     return dictGenHashFunction((unsigned char*)key, sdslen((char*)key));
 }
diff --git a/src/server.h b/src/server.h
index dbb1b9621..25d14ebe5 100644
--- a/src/server.h
+++ b/src/server.h
@@ -3164,7 +3164,9 @@ robj *setTypeDup(robj *o);
 typedef struct listpackEx {
     ExpireMeta meta;  /* To be used in order to register the hash in the
                          global ebuckets (i.e. db->hexpires) with next,
-                         minimum, hash-field to expire. */
+                         minimum, hash-field to expire. TTL value might be
+                         inaccurate up-to few seconds due to optimization
+                         consideration.  */
     sds key;          /* reference to the key, same one that stored in
                          db->dict. Will be used from active-expiration flow
                          for notification and deletion of the object, if
@@ -3179,7 +3181,9 @@ typedef struct dictExpireMetadata {
     ExpireMeta expireMeta;   /* embedded ExpireMeta in dict.
                                 To be used in order to register the hash in the
                                 global ebuckets (i.e db->hexpires) with next,
-                                minimum, hash-field to expire */
+                                minimum, hash-field to expire. TTL value might be
+                                inaccurate up-to few seconds due to optimization
+                                consideration. */
     ebuckets hfe;            /* DS of Hash Fields Expiration, associated to each hash */
     sds key;                 /* reference to the key, same one that stored in
                                db->dict. Will be used from active-expiration flow
@@ -3225,13 +3229,10 @@ uint64_t hashTypeRemoveFromExpires(ebuckets *hexpires, robj *o);
 void hashTypeAddToExpires(redisDb *db, sds key, robj *hashObj, uint64_t expireTime);
 void hashTypeFree(robj *o);
 int hashTypeIsExpired(const robj *o, uint64_t expireAt);
-uint64_t hashTypeGetMinExpire(robj *o);
 unsigned char *hashTypeListpackGetLp(robj *o);
-uint64_t hashTypeGetMinExpire(robj *o);
+uint64_t hashTypeGetMinExpire(robj *o, int accurate);
 void hashTypeUpdateKeyRef(robj *o, sds newkey);
 ebuckets *hashTypeGetDictMetaHFE(dict *d);
-uint64_t hashTypeGetMinExpire(robj *keyObj);
-uint64_t hashTypeGetNextTimeToExpire(robj *o);
 void initDictExpireMetadata(sds key, robj *o);
 struct listpackEx *listpackExCreate(void);
 void listpackExAddNew(robj *o, char *field, size_t flen,
@@ -3539,6 +3540,7 @@ void startEvictionTimeProc(void);
 
 /* Keys hashing / comparison functions for dict.c hash tables. */
 uint64_t dictSdsHash(const void *key);
+uint64_t dictPtrHash(const void *key);
 uint64_t dictSdsCaseHash(const void *key);
 int dictSdsKeyCompare(dict *d, const void *key1, const void *key2);
 int dictSdsMstrKeyCompare(dict *d, const void *sdsLookup, const void *mstrStored);
diff --git a/src/t_hash.c b/src/t_hash.c
index 902094491..208e6775b 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -36,12 +36,21 @@ typedef enum GetFieldRes {
                              * it was the last field in the hash. */
 } GetFieldRes;
 
+/* ActiveExpireCtx passed to hashTypeActiveExpire() */
+typedef struct ExpireCtx {
+    uint32_t fieldsToExpireQuota;
+    redisDb *db;
+} ExpireCtx;
+
+typedef listpackEntry CommonEntry; /* extend usage beyond lp */
+
 /* hash field expiration (HFE) funcs */
 static ExpireAction onFieldExpire(eItem item, void *ctx);
 static ExpireMeta* hfieldGetExpireMeta(const eItem field);
 static ExpireMeta *hashGetExpireMeta(const eItem hash);
 static void hexpireGenericCommand(client *c, const char *cmd, long long basetime, int unit);
 static ExpireAction hashTypeActiveExpire(eItem hashObj, void *ctx);
+static uint64_t hashTypeExpire(robj *o, ExpireCtx *expireCtx, int updateGlobalHFE);
 static void hfieldPersist(robj *hashObj, hfield field);
 static void propagateHashFieldDeletion(redisDb *db, sds key, char *field, size_t fieldLen);
 
@@ -118,12 +127,6 @@ EbucketsType hashFieldExpireBucketsType = {
     .itemsAddrAreOdd = 1,                 /* Addresses of hfield (mstr) are odd!! */
 };
 
-/* ActiveExpireCtx passed to hashTypeActiveExpire() */
-typedef struct ActiveExpireCtx {
-    uint32_t fieldsToExpireQuota;
-    redisDb *db;
-} ActiveExpireCtx;
-
 /* OnFieldExpireCtx passed to OnFieldExpire() */
 typedef struct OnFieldExpireCtx {
     robj *hashObj;
@@ -421,7 +424,7 @@ void listpackExExpire(redisDb *db, robj *o, ExpireInfo *info) {
     if (expired)
         lpt->lp = lpDeleteRange(lpt->lp, 0, expired * 3);
 
-    min = hashTypeGetNextTimeToExpire(o);
+    min = hashTypeGetMinExpire(o, 1 /*accurate*/);
     info->nextExpireTime = min;
 }
 
@@ -727,17 +730,22 @@ GetFieldRes hashTypeGetValue(redisDb *db, robj *o, sds field, unsigned char **vs
         serverPanic("Unknown hash encoding");
     }
 
-    /* Don't expire anything while loading. It will be done later. */
-    if ( (server.loading) ||
-         (server.lazy_expire_disabled) ||
-         ((server.masterhost) && (server.current_client && (server.current_client->flags & CLIENT_MASTER))) ||
-         (expiredAt >= (uint64_t) commandTimeSnapshot()) )
+    if (expiredAt >= (uint64_t) commandTimeSnapshot())
         return GETF_OK;
 
-    /* Field is expired */
+    if (server.masterhost) {
+        /* If CLIENT_MASTER, assume valid as long as it didn't get delete */
+        if (server.current_client && (server.current_client->flags & CLIENT_MASTER))
+            return GETF_OK;
 
-    /* If indicated to avoid deleting expired field */
-    if (hfeFlags & HFE_LAZY_AVOID_FIELD_DEL)
+        /* If user client, then act as if expired, but don't delete! */
+        return GETF_EXPIRED;
+    }
+
+    if ((server.loading) ||
+        (server.lazy_expire_disabled) ||
+        (hfeFlags & HFE_LAZY_AVOID_FIELD_DEL) ||
+        (isPausedActionsWithUpdate(PAUSE_ACTION_EXPIRE)))
         return GETF_EXPIRED;
 
     if (o->encoding == OBJ_ENCODING_LISTPACK_EX)
@@ -1142,7 +1150,8 @@ int hashTypeSetExInit(robj *key, robj *o, client *c, redisDb *db, const char *cm
         }
     }
 
-    ex->minExpire = hashTypeGetMinExpire(ex->hashObj);
+    /* Read minExpire from attached ExpireMeta to the hash */
+    ex->minExpire = hashTypeGetMinExpire(ex->hashObj, 0);
     return C_OK;
 }
 
@@ -1172,8 +1181,8 @@ void hashTypeSetExDone(HashTypeSetEx *ex) {
             if ((ex->minExpire < ex->minExpireFields))
                 return;
 
-            /* retrieve new expired time. It might have changed. */
-            uint64_t newMinExpire = hashTypeGetNextTimeToExpire(ex->hashObj);
+            /* Retrieve new expired time. It might have changed. */
+            uint64_t newMinExpire = hashTypeGetMinExpire(ex->hashObj, 1 /*accurate*/);
 
             /* Calculate the diff between old minExpire and newMinExpire. If it is
              * only few seconds, then don't have to update global HFE DS. At the worst
@@ -1580,7 +1589,7 @@ void hashTypeConvertListpackEx(robj *o, int enc, ebuckets *hexpires) {
         dict *dict;
         dictExpireMetadata *dictExpireMeta;
         listpackEx *lpt = o->ptr;
-        uint64_t minExpire = hashTypeGetMinExpire(o);
+        uint64_t minExpire = hashTypeGetMinExpire(o, 0);
 
         if (hexpires && lpt->meta.trash != 1)
             ebRemove(hexpires, &hashExpireBucketsType, o);
@@ -1745,7 +1754,7 @@ void hashReplyFromListpackEntry(client *c, listpackEntry *e) {
  * 'key' and 'val' will be set to hold the element.
  * The memory in them is not to be freed or modified by the caller.
  * 'val' can be NULL in which case it's not extracted. */
-void hashTypeRandomElement(robj *hashobj, unsigned long hashsize, listpackEntry *key, listpackEntry *val) {
+void hashTypeRandomElement(robj *hashobj, unsigned long hashsize, CommonEntry *key, CommonEntry *val) {
     if (hashobj->encoding == OBJ_ENCODING_HT) {
         dictEntry *de = dictGetFairRandomKey(hashobj->ptr);
         hfield field = dictGetKey(de);
@@ -1757,9 +1766,10 @@ void hashTypeRandomElement(robj *hashobj, unsigned long hashsize, listpackEntry
             val->slen = sdslen(s);
         }
     } else if (hashobj->encoding == OBJ_ENCODING_LISTPACK) {
-        lpRandomPair(hashobj->ptr, hashsize, key, val, 2);
+        lpRandomPair(hashobj->ptr, hashsize, (listpackEntry *) key, (listpackEntry *) val, 2);
     } else if (hashobj->encoding == OBJ_ENCODING_LISTPACK_EX) {
-        lpRandomPair(hashTypeListpackGetLp(hashobj), hashsize, key, val, 3);
+        lpRandomPair(hashTypeListpackGetLp(hashobj), hashsize, (listpackEntry *) key,
+                     (listpackEntry *) val, 3);
     } else {
         serverPanic("Unknown hash encoding");
     }
@@ -1780,38 +1790,61 @@ void hashTypeRandomElement(robj *hashobj, unsigned long hashsize, listpackEntry
  *   by returning ACT_REMOVE_EXP_ITEM.
  * - If hash has no more fields afterward, it will remove the hash from keyspace.
  */
-static ExpireAction hashTypeActiveExpire(eItem _hashObj, void *ctx) {
-    robj *hashObj = (robj *) _hashObj;
-    ActiveExpireCtx *activeExpireCtx = (ActiveExpireCtx *) ctx;
+static ExpireAction hashTypeActiveExpire(eItem item, void *ctx) {
+    ExpireCtx *expireCtx = ctx;
+
+    /* If no more quota left for this callback, stop */
+    if (expireCtx->fieldsToExpireQuota == 0)
+        return ACT_STOP_ACTIVE_EXP;
+
+    uint64_t nextExpTime = hashTypeExpire((robj *) item, expireCtx, 0);
+
+    /* If hash has no more fields to expire or got deleted, indicate
+     * to remove it from HFE DB to the caller ebExpire() */
+    if (nextExpTime == EB_EXPIRE_TIME_INVALID || nextExpTime == 0) {
+        return ACT_REMOVE_EXP_ITEM;
+    } else {
+        /* Hash has more fields to expire. Update next expiration time of the hash
+         * and indicate to add it back to global HFE DS */
+        ebSetMetaExpTime(hashGetExpireMeta(item), nextExpTime);
+        return ACT_UPDATE_EXP_ITEM;
+    }
+}
+
+/* Delete all expired fields from the hash and delete the hash if left empty.
+ *
+ * updateGlobalHFE - If the hash should be updated in the global HFE DS with new
+ *                   expiration time in case expired fields were deleted.
+ *
+ * Return next Expire time of the hash
+ * - 0 if hash got deleted
+ * - EB_EXPIRE_TIME_INVALID if no more fields to expire
+ */
+static uint64_t hashTypeExpire(robj *o, ExpireCtx *expireCtx, int updateGlobalHFE) {
+    uint64_t noExpireLeftRes = EB_EXPIRE_TIME_INVALID;
+    redisDb *db = expireCtx->db;
     sds keystr = NULL;
     ExpireInfo info = {0};
 
-    /* If no more quota left for this callback, stop */
-    if (activeExpireCtx->fieldsToExpireQuota == 0)
-        return ACT_STOP_ACTIVE_EXP;
-
-    if (hashObj->encoding == OBJ_ENCODING_LISTPACK_EX) {
-        info = (ExpireInfo){
-                .maxToExpire = activeExpireCtx->fieldsToExpireQuota,
+    if (o->encoding == OBJ_ENCODING_LISTPACK_EX) {
+        info = (ExpireInfo) {
+                .maxToExpire = expireCtx->fieldsToExpireQuota,
                 .now = commandTimeSnapshot(),
                 .itemsExpired = 0};
 
-        listpackExExpire(activeExpireCtx->db, hashObj, &info);
+        listpackExExpire(db, o, &info);
         server.stat_expired_hash_fields += info.itemsExpired;
-        keystr = ((listpackEx*)hashObj->ptr)->key;
+        keystr = ((listpackEx*)o->ptr)->key;
     } else {
-        serverAssert(hashObj->encoding == OBJ_ENCODING_HT);
+        serverAssert(o->encoding == OBJ_ENCODING_HT);
 
-        dict *d = hashObj->ptr;
+        dict *d = o->ptr;
         dictExpireMetadata *dictExpireMeta = (dictExpireMetadata *) dictMetadata(d);
 
-        OnFieldExpireCtx onFieldExpireCtx = {
-            .hashObj = hashObj,
-            .db = activeExpireCtx->db
-        };
+        OnFieldExpireCtx onFieldExpireCtx = { .hashObj = o, .db = db };
 
         info = (ExpireInfo){
-            .maxToExpire = activeExpireCtx->fieldsToExpireQuota,
+            .maxToExpire = expireCtx->fieldsToExpireQuota,
             .onExpireItem = onFieldExpire,
             .ctx = &onFieldExpireCtx,
             .now = commandTimeSnapshot()
@@ -1822,41 +1855,98 @@ static ExpireAction hashTypeActiveExpire(eItem _hashObj, void *ctx) {
     }
 
     /* Update quota left */
-    activeExpireCtx->fieldsToExpireQuota -= info.itemsExpired;
+    expireCtx->fieldsToExpireQuota -= info.itemsExpired;
 
     /* In some cases, a field might have been deleted without updating the global DS.
      * As a result, active-expire might not expire any fields, in such cases,
      * we don't need to send notifications or perform other operations for this key. */
     if (info.itemsExpired) {
         robj *key = createStringObject(keystr, sdslen(keystr));
-        notifyKeyspaceEvent(NOTIFY_HASH,"hexpired",key,activeExpireCtx->db->id);
-        if (hashTypeLength(hashObj, 0) == 0) {
-            dbDelete(activeExpireCtx->db, key);
-            notifyKeyspaceEvent(NOTIFY_GENERIC,"del",key,activeExpireCtx->db->id);
+        notifyKeyspaceEvent(NOTIFY_HASH, "hexpired", key, db->id);
+
+        if (updateGlobalHFE)
+            ebRemove(&db->hexpires, &hashExpireBucketsType, o);
+
+        if (hashTypeLength(o, 0) == 0) {
+            dbDelete(db, key);
+            notifyKeyspaceEvent(NOTIFY_GENERIC, "del", key, db->id);
+            noExpireLeftRes = 0;
+        } else {
+            if ((updateGlobalHFE) && (info.nextExpireTime != EB_EXPIRE_TIME_INVALID))
+                ebAdd(&db->hexpires, &hashExpireBucketsType, o, info.nextExpireTime);
         }
-        server.dirty++;
-        signalModifiedKey(NULL, activeExpireCtx->db, key);
+
+        signalModifiedKey(NULL, db, key);
         decrRefCount(key);
     }
 
-    /* If hash has no more fields to expire, remove it from HFE DB */
-    if (info.nextExpireTime == EB_EXPIRE_TIME_INVALID) {
-        return ACT_REMOVE_EXP_ITEM;
-    } else {
-        /* Hash has more fields to expire. Update next expiration time of the hash
-         * and indicate to add it back to global HFE DS */
-        ebSetMetaExpTime(hashGetExpireMeta(hashObj), info.nextExpireTime);
-        return ACT_UPDATE_EXP_ITEM;
-    }
+    /* return 0 if hash got deleted, EB_EXPIRE_TIME_INVALID if no more fields
+     * with expiration. Else return next expiration time */
+    return (info.nextExpireTime == EB_EXPIRE_TIME_INVALID) ? noExpireLeftRes : info.nextExpireTime;
 }
 
-/* Return the next/minimum expiry time of the hash-field. This is useful if a
- * field with the minimum expiry is deleted, and you want to get the next
- * minimum expiry. Otherwise, consider using hashTypeGetMinExpire() which will
- * be faster but less accurate.
+/* Delete all expired fields in hash if needed (Currently used only by HRANDFIELD)
  *
- * Return next min expiry. If none return EB_EXPIRE_TIME_INVALID  */
-uint64_t hashTypeGetNextTimeToExpire(robj *o) {
+ * Return 1 if the entire hash was deleted, 0 otherwise.
+ * This function might be pricy in case there are many expired fields.
+ */
+static int hashTypeExpireIfNeeded(redisDb *db, robj *o) {
+    uint64_t nextExpireTime;
+    uint64_t minExpire = hashTypeGetMinExpire(o, 1 /*accurate*/);
+
+    /* Nothing to expire */
+    if ((mstime_t) minExpire >= commandTimeSnapshot())
+        return 0;
+
+    /* Follow expireIfNeeded() conditions of when not lazy-expire */
+    if ( (server.loading) ||
+         (server.lazy_expire_disabled) ||
+         (server.masterhost) ||  /* master-client or user-client, don't delete */
+         (isPausedActionsWithUpdate(PAUSE_ACTION_EXPIRE)))
+        return 0;
+
+    /* Take care to expire all the fields */
+    ExpireCtx expireCtx = { .db = db, .fieldsToExpireQuota = UINT32_MAX };
+    nextExpireTime = hashTypeExpire(o, &expireCtx, 1);
+    /* return 1 if the entire hash was deleted */
+    return nextExpireTime == 0;
+}
+
+/* Return the next/minimum expiry time of the hash-field.
+ * accurate=1 - Return the exact time by looking into the object DS.
+ * accurate=0 - Return the minimum expiration time maintained in expireMeta which
+ *              might not be accurate due to optimization reasons.
+ *
+ * If not found, return EB_EXPIRE_TIME_INVALID
+ */
+uint64_t hashTypeGetMinExpire(robj *o, int accurate) {
+    ExpireMeta *expireMeta = NULL;
+
+    if (!accurate) {
+        if (o->encoding == OBJ_ENCODING_LISTPACK) {
+            return EB_EXPIRE_TIME_INVALID;
+        } else if (o->encoding == OBJ_ENCODING_LISTPACK_EX) {
+            listpackEx *lpt = o->ptr;
+            expireMeta = &lpt->meta;
+        } else {
+            serverAssert(o->encoding == OBJ_ENCODING_HT);
+
+            dict *d = o->ptr;
+            if (!isDictWithMetaHFE(d))
+                return EB_EXPIRE_TIME_INVALID;
+
+            expireMeta = &((dictExpireMetadata *) dictMetadata(d))->expireMeta;
+        }
+
+        /* Keep aside next hash-field expiry before updating HFE DS. Verify it is not trash */
+        if (expireMeta->trash == 1)
+            return EB_EXPIRE_TIME_INVALID;
+
+        return ebGetMetaExpTime(expireMeta);
+    }
+
+    /* accurate == 1 */
+
     if (o->encoding == OBJ_ENCODING_LISTPACK) {
         return EB_EXPIRE_TIME_INVALID;
     } else if (o->encoding == OBJ_ENCODING_LISTPACK_EX) {
@@ -1873,33 +1963,6 @@ uint64_t hashTypeGetNextTimeToExpire(robj *o) {
     }
 }
 
-/* Return the next/minimum expiry time of the hash-field.
- * If not found, return EB_EXPIRE_TIME_INVALID */
-uint64_t hashTypeGetMinExpire(robj *o) {
-    ExpireMeta *expireMeta = NULL;
-
-    if (o->encoding == OBJ_ENCODING_LISTPACK) {
-        return EB_EXPIRE_TIME_INVALID;
-    } else if (o->encoding == OBJ_ENCODING_LISTPACK_EX) {
-        listpackEx *lpt = o->ptr;
-        expireMeta = &lpt->meta;
-    } else {
-        serverAssert(o->encoding == OBJ_ENCODING_HT);
-
-        dict *d = o->ptr;
-        if (!isDictWithMetaHFE(d))
-            return EB_EXPIRE_TIME_INVALID;
-
-        expireMeta = &((dictExpireMetadata *) dictMetadata(d))->expireMeta;
-    }
-
-    /* Keep aside next hash-field expiry before updating HFE DS. Verify it is not trash */
-    if (expireMeta->trash == 1)
-        return EB_EXPIRE_TIME_INVALID;
-
-    return ebGetMetaExpTime(expireMeta);
-}
-
 uint64_t hashTypeRemoveFromExpires(ebuckets *hexpires, robj *o) {
     if (o->encoding == OBJ_ENCODING_LISTPACK) {
         return EB_EXPIRE_TIME_INVALID;
@@ -1963,7 +2026,7 @@ void hashTypeAddToExpires(redisDb *db, sds key, robj *hashObj, uint64_t expireTi
  * Returns number of fields active-expired.
  */
 uint64_t hashTypeDbActiveExpire(redisDb *db, uint32_t maxFieldsToExpire) {
-    ActiveExpireCtx ctx = { .db = db, .fieldsToExpireQuota = maxFieldsToExpire };
+    ExpireCtx ctx = { .db = db, .fieldsToExpireQuota = maxFieldsToExpire };
     ExpireInfo info = {
             .maxToExpire = UINT64_MAX, /* Only maxFieldsToExpire play a role */
             .onExpireItem = hashTypeActiveExpire,
@@ -2345,7 +2408,7 @@ void genericHgetallCommand(client *c, int flags) {
 
     /* Skip expired fields if the hash has an expire time set at global HFE DS. We could
      * set it to constant 1, but then it will make another lookup for each field expiration */
-    int skipExpiredFields = (EB_EXPIRE_TIME_INVALID == hashTypeGetMinExpire(o)) ? 0 : 1;
+    int skipExpiredFields = (EB_EXPIRE_TIME_INVALID == hashTypeGetMinExpire(o, 0)) ? 0 : 1;
 
     while (hashTypeNext(hi, skipExpiredFields) != C_ERR) {
         if (flags & OBJ_HASH_KEY) {
@@ -2431,8 +2494,6 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) {
 
     if ((hash = lookupKeyReadOrReply(c,c->argv[1],shared.emptyarray))
         == NULL || checkType(c,hash,OBJ_HASH)) return;
-    /* TODO: Active-expire */
-    size = hashTypeLength(hash, 0);
 
     if(l >= 0) {
         count = (unsigned long) l;
@@ -2441,6 +2502,15 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) {
         uniq = 0;
     }
 
+    /* Delete all expired fields. If the entire hash got deleted then return empty array. */
+    if (hashTypeExpireIfNeeded(c->db, hash)) {
+        addReply(c, shared.emptyarray);
+        return;
+    }
+
+    /* Delete expired fields */
+    size = hashTypeLength(hash, 0);
+
     /* If count is zero, serve it ASAP to avoid special cases later. */
     if (count == 0) {
         addReply(c,shared.emptyarray);
@@ -2544,64 +2614,50 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) {
     }
 
     /* CASE 3:
-     * The number of elements inside the hash is not greater than
+     * The number of elements inside the hash of type dict is not greater than
      * HRANDFIELD_SUB_STRATEGY_MUL times the number of requested elements.
-     * In this case we create a hash from scratch with all the elements, and
-     * subtract random elements to reach the requested number of elements.
+     * In this case we create an array of dictEntry pointers from the original hash,
+     * and subtract random elements to reach the requested number of elements.
      *
      * This is done because if the number of requested elements is just
      * a bit less than the number of elements in the hash, the natural approach
      * used into CASE 4 is highly inefficient. */
     if (count*HRANDFIELD_SUB_STRATEGY_MUL > size) {
         /* Hashtable encoding (generic implementation) */
-        dict *d = dictCreate(&sdsReplyDictType);  /* without metadata! */
-        dictExpand(d, size);
-        hashTypeIterator *hi = hashTypeInitIterator(hash);
+        dict *ht = hash->ptr;
+        dictIterator *di;
+        dictEntry *de;
+        unsigned long idx = 0;
 
-        /* Add all the elements into the temporary dictionary. */
-        while ((hashTypeNext(hi, 0)) != C_ERR) {
-            int ret = DICT_ERR;
-            sds key, value = NULL;
+        /* Allocate a temporary array of pointers to stored key-values in dict and
+         * assist it to remove random elements to reach the right count. */
+        struct FieldValPair {
+            hfield field;
+            sds value;
+        } *pairs = zmalloc(sizeof(struct FieldValPair) * size);
 
-            key = hashTypeCurrentObjectNewSds(hi,OBJ_HASH_KEY);
-            if (withvalues)
-                value = hashTypeCurrentObjectNewSds(hi,OBJ_HASH_VALUE);
-            ret = dictAdd(d, key, value);
-
-            serverAssert(ret == DICT_OK);
-        }
-        serverAssert(dictSize(d) == size);
-        hashTypeReleaseIterator(hi);
+        /* Add all the elements into the temporary array. */
+        di = dictGetIterator(ht);
+        while((de = dictNext(di)) != NULL)
+              pairs[idx++] = (struct FieldValPair) {dictGetKey(de), dictGetVal(de)};
+        dictReleaseIterator(di);
 
         /* Remove random elements to reach the right count. */
         while (size > count) {
-            dictEntry *de;
-            de = dictGetFairRandomKey(d);
-            dictUseStoredKeyApi(d, 1);
-            dictUnlink(d,dictGetKey(de));
-            dictUseStoredKeyApi(d, 0);
-            sdsfree(dictGetKey(de));
-            sdsfree(dictGetVal(de));
-            dictFreeUnlinkedEntry(d,de);
-            size--;
+            unsigned long toDiscardIdx = rand() % size;
+            pairs[toDiscardIdx] = pairs[--size];
         }
 
-        /* Reply with what's in the dict and release memory */
-        dictIterator *di;
-        dictEntry *de;
-        di = dictGetIterator(d);
-        while ((de = dictNext(di)) != NULL) {
-            sds key = dictGetKey(de);
-            sds value = dictGetVal(de);
+        /* Reply with what's in the array */
+        for (idx = 0; idx < size; idx++) {
             if (withvalues && c->resp > 2)
                 addReplyArrayLen(c,2);
-            addReplyBulkSds(c, key);
+            addReplyBulkCBuffer(c, pairs[idx].field, hfieldlen(pairs[idx].field));
             if (withvalues)
-                addReplyBulkSds(c, value);
+                addReplyBulkCBuffer(c, pairs[idx].value, sdslen(pairs[idx].value));
         }
 
-        dictReleaseIterator(di);
-        dictRelease(d);
+        zfree(pairs);
     }
 
     /* CASE 4: We have a big hash compared to the requested number of elements.
@@ -2609,43 +2665,78 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) {
      * to the temporary hash, trying to eventually get enough unique elements
      * to reach the specified count. */
     else {
+        /* Allocate temporary dictUnique to find unique elements. Just keep ref
+         * to key-value from the original hash. This dict relaxes hash function
+         * to be based on field's pointer */
+        dictType uniqueDictType = { .hashFunction =  dictPtrHash };
+        dict *dictUnique = dictCreate(&uniqueDictType);
+        dictExpand(dictUnique, count);
+
         /* Hashtable encoding (generic implementation) */
         unsigned long added = 0;
-        listpackEntry key, value;
-        dict *d = dictCreate(&hashDictType);
-        dictExpand(d, count);
+
         while(added < count) {
-            hashTypeRandomElement(hash, size, &key, withvalues? &value : NULL);
+            dictEntry *de = dictGetFairRandomKey(hash->ptr);
+            serverAssert(de != NULL);
+            hfield field = dictGetKey(de);
+            sds value = dictGetVal(de);
 
             /* Try to add the object to the dictionary. If it already exists
             * free it, otherwise increment the number of objects we have
             * in the result dictionary. */
-            sds skey = hashSdsFromListpackEntry(&key);
-            if (dictAdd(d,skey,NULL) != DICT_OK) {
-                sdsfree(skey);
+            if (dictAdd(dictUnique, field, value) != DICT_OK)
                 continue;
-            }
+
             added++;
 
             /* We can reply right away, so that we don't need to store the value in the dict. */
             if (withvalues && c->resp > 2)
                 addReplyArrayLen(c,2);
-            hashReplyFromListpackEntry(c, &key);
+
+            addReplyBulkCBuffer(c, field, hfieldlen(field));
             if (withvalues)
-                hashReplyFromListpackEntry(c, &value);
+                addReplyBulkCBuffer(c, value, sdslen(value));
         }
 
         /* Release memory */
-        dictRelease(d);
+        dictRelease(dictUnique);
     }
 }
 
-/* HRANDFIELD key [<count> [WITHVALUES]] */
+/*
+ * HRANDFIELD - Return a random field from the hash value stored at key.
+ * CLI usage: HRANDFIELD key [<count> [WITHVALUES]]
+ *
+ * Considerations for the current imp of HRANDFIELD & HFE feature:
+ *  HRANDFIELD might access any of the fields in the hash as some of them might
+ *  be expired. And so the Implementation of HRANDFIELD along with HFEs
+ *  might be one of the two options:
+ *  1. Expire hash-fields before diving into handling HRANDFIELD.
+ *  2. Refine HRANDFIELD cases to deal with expired fields.
+ *
+ *  Regarding the first option, as reference, the command RANDOMKEY also declares
+ *  on O(1) complexity, yet might be stuck on a very long (but not infinite) loop
+ *  trying to find non-expired keys. Furthermore RANDOMKEY also evicts expired keys
+ *  along the way even though it is categorized as a read-only command. Note that
+ *  the case of HRANDFIELD is more lightweight versus RANDOMKEY since HFEs have
+ *  much more effective and aggressive active-expiration for fields behind.
+ *
+ *  The second option introduces additional implementation complexity to HRANDFIELD.
+ *  We could further refine HRANDFIELD cases to differentiate between scenarios
+ *  with many expired fields versus few expired fields, and adjust based on the
+ *  percentage of expired fields. However, this approach could still lead to long
+ *  loops or necessitate expiring fields before selecting them. For the “lightweight”
+ *  cases it is also expected to have a lightweight expiration.
+ *
+ *  Considering the pros and cons, and the fact that HRANDFIELD is an infrequent
+ *  command (particularly with HFEs) and the fact we have effective active-expiration
+ *  behind for hash-fields, it is better to keep it simple and choose the option #1.
+ */
 void hrandfieldCommand(client *c) {
     long l;
     int withvalues = 0;
     robj *hash;
-    listpackEntry ele;
+    CommonEntry ele;
 
     if (c->argc >= 3) {
         if (getRangeLongFromObjectOrReply(c,c->argv[2],-LONG_MAX,LONG_MAX,&l,NULL) != C_OK) return;
@@ -2669,8 +2760,18 @@ void hrandfieldCommand(client *c) {
         return;
     }
 
+    /* Delete all expired fields. If the entire hash got deleted then return null. */
+    if (hashTypeExpireIfNeeded(c->db, hash)) {
+        addReply(c,shared.null[c->resp]);
+        return;
+    }
+
     hashTypeRandomElement(hash,hashTypeLength(hash, 0),&ele,NULL);
-    hashReplyFromListpackEntry(c, &ele);
+
+    if (ele.sval)
+        addReplyBulkCBuffer(c, ele.sval, ele.slen);
+    else
+        addReplyBulkLongLong(c, ele.lval);
 }
 
 /*-----------------------------------------------------------------------------
diff --git a/tests/unit/type/hash-field-expire.tcl b/tests/unit/type/hash-field-expire.tcl
index f78f2b142..cca616b61 100644
--- a/tests/unit/type/hash-field-expire.tcl
+++ b/tests/unit/type/hash-field-expire.tcl
@@ -32,13 +32,6 @@ proc get_hashes_with_expiry_fields {r} {
     return 0
 }
 
-proc create_hash {key entries} {
-    r del $key
-    foreach entry $entries {
-        r hset $key [lindex $entry 0] [lindex $entry 1]
-    }
-}
-
 proc get_keys {l} {
     set res {}
     foreach entry $l {
@@ -48,22 +41,6 @@ proc get_keys {l} {
     return $res
 }
 
-proc cmp_hrandfield_result {hash_name expected_result} {
-    # Accumulate hrandfield results
-    unset -nocomplain myhash
-    array set myhash {}
-    for {set i 0} {$i < 100} {incr i} {
-        set key [r hrandfield $hash_name]
-        set myhash($key) 1
-    }
-     set res [lsort [array names myhash]]
-     if {$res eq $expected_result} {
-        return 1
-     } else {
-        return $res
-     }
-}
-
 proc dumpAllHashes {client} {
     set keyAndFields(0,0) 0
     unset keyAndFields
@@ -77,36 +54,6 @@ proc dumpAllHashes {client} {
     return [array get keyAndFields]
 }
 
-proc hrandfieldTest {activeExpireConfig} {
-    r debug set-active-expire $activeExpireConfig
-    r del myhash
-    set contents {{field1 1} {field2 2} }
-    create_hash myhash $contents
-
-    set factorValgrind [expr {$::valgrind ? 2 : 1}]
-
-    # Set expiration time for field1 and field2 such that field1 expires first
-    r hpexpire myhash 1 NX FIELDS 1 field1
-    r hpexpire myhash 100 NX FIELDS 1 field2
-
-    # On call hrandfield command lazy expire deletes field1 first
-    wait_for_condition 8 10 {
-        [cmp_hrandfield_result myhash "field2"] == 1
-    } else {
-        fail "Expected field2 to be returned by HRANDFIELD."
-    }
-
-    # On call hrandfield command lazy expire deletes field2 as well
-    wait_for_condition 8 20 {
-        [cmp_hrandfield_result myhash "{}"] == 1
-    } else {
-        fail "Expected {} to be returned by HRANDFIELD."
-    }
-
-    # restore the default value
-    r debug set-active-expire 1
-}
-
 ############################### TESTS #########################################
 
 start_server {tags {"external:skip needs:debug"}} {
@@ -396,22 +343,33 @@ start_server {tags {"external:skip needs:debug"}} {
             r debug set-active-expire 1
         }
 
-        # OPEN: To decide if to delete expired fields at start of HRANDFIELD.
-        #    test "Test HRANDFIELD does not return expired fields ($type)" {
-        #        hrandfieldTest 0
-        #        hrandfieldTest 1
-        #    }
-
-        test "Test HRANDFIELD can return expired fields ($type)" {
+        test "Test HRANDFIELD deletes all expired fields ($type)" {
             r debug set-active-expire 0
-            r del myhash
+            r flushall
             r hset myhash f1 v1 f2 v2 f3 v3 f4 v4 f5 v5
-            r hpexpire myhash 1 NX FIELDS 4 f1 f2 f3 f4
+            r hpexpire myhash 1 FIELDS 2 f1 f2
             after 5
-            set res [cmp_hrandfield_result myhash "f1 f2 f3 f4 f5"]
-            assert {$res == 1}
-            r debug set-active-expire 1
+            assert_equal [lsort [r hrandfield myhash 5]] "f3 f4 f5"
+            r hpexpire myhash 1 FIELDS 3 f3 f4 f5
+            after 5
+            assert_equal [lsort [r hrandfield myhash 5]] ""
+            assert_equal [r keys *] ""
 
+            r del myhash
+            r hset myhash f1 v1 f2 v2 f3 v3
+            r hpexpire myhash 1 FIELDS 1 f1
+            after 5
+            set res [r hrandfield myhash]
+            assert {$res == "f2" || $res == "f3"}
+            r hpexpire myhash 1 FIELDS 1 f2
+            after 5
+            assert_equal [lsort [r hrandfield myhash 5]] "f3"
+            r hpexpire myhash 1 FIELDS 1 f3
+            after 5
+            assert_equal [r hrandfield myhash] ""
+            assert_equal [r keys *] ""
+
+            r debug set-active-expire 1
         }
 
         test "Lazy Expire - HLEN does count expired fields ($type)" {
@@ -1054,7 +1012,13 @@ start_server {tags {"external:skip needs:debug"}} {
                 r hpexpireat h1 [expr [clock seconds]*1000+100000] NX FIELDS 1 f2
                 r hpexpire h1 100000 NX FIELDS 3 f3 f4 f5
                 r hexpire h1 100000 FIELDS 1 f6
-                r hset h5 f1 v1
+
+                # Verify HRANDFIELD deletes expired fields and propagates it
+                r hset h2 f1 v1 f2 v2
+                r hpexpire h2 1 FIELDS 1 f1
+                r hpexpire h2 50 FIELDS 1 f2
+                assert_equal [r hrandfield h4 2] ""
+                after 200
 
                 assert_aof_content $aof {
                     {select *}
@@ -1063,7 +1027,11 @@ start_server {tags {"external:skip needs:debug"}} {
                     {hpexpireat h1 * FIELDS 1 f2}
                     {hpexpireat h1 * NX FIELDS 3 f3 f4 f5}
                     {hpexpireat h1 * FIELDS 1 f6}
-                    {hset h5 f1 v1}
+                    {hset h2 f1 v1 f2 v2}
+                    {hpexpireat h2 * FIELDS 1 f1}
+                    {hpexpireat h2 * FIELDS 1 f2}
+                    {hdel h2 f1}
+                    {hdel h2 f2}
                 }
 
                 array set keyAndFields1 [dumpAllHashes r]
@@ -1086,6 +1054,7 @@ start_server {tags {"external:skip needs:debug"}} {
             r flushall ; # Clean up keyspace to avoid interference by keys from other tests
             set repl [attach_to_replication_stream]
 
+            # HEXPIRE/HPEXPIRE should be translated into HPEXPIREAT
             r hset h1 f1 v1
             r hexpireat h1 [expr [clock seconds]+100] NX FIELDS 1 f1
             r hset h2 f2 v2
@@ -1109,6 +1078,28 @@ start_server {tags {"external:skip needs:debug"}} {
             close_replication_stream $repl
         } {} {needs:repl}
 
+        test {HRANDFIELD delete expired fields and propagate DELs to replica} {
+            r flushall
+            set repl [attach_to_replication_stream]
+
+            r hset h4 f1 v1 f2 v2
+            r hpexpire h4 1 FIELDS 1 f1
+            r hpexpire h4 2 FIELDS 1 f2
+            after 100
+            assert_equal [r hrandfield h4 2] ""
+
+
+            assert_replication_stream $repl {
+                {select *}
+                {hset h4 f1 v1 f2 v2}
+                {hpexpireat h4 * FIELDS 1 f1}
+                {hpexpireat h4 * FIELDS 1 f2}
+                {hdel h4 f1}
+                {hdel h4 f2}
+            }
+            close_replication_stream $repl
+        } {} {needs:repl}
+
         # Start another server to test replication of TTLs
         start_server {tags {needs:repl external:skip}} {
             # Set the outer layer server as primary

From 5eac99c3128ab1e94bee61368c1929f0ca09497c Mon Sep 17 00:00:00 2001
From: Moti Cohen <moticless@gmail.com>
Date: Tue, 25 Jun 2024 13:15:09 +0300
Subject: [PATCH 11/14] Fix H(P)EXPIREAT command to propagate HDEL as well
 (#13364)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

H(P)EXPIREAT command might delete fields in case the absolute time is in the
past. Those HDELs need to be propagated as well.

In general, as we need to propagate H(P)EXPIRE(AT) command to the replica, each
field that is mentioned in the command should be categorized into one of the four
options:
1. Managed to update field’s expiration time - propagate it to replica as part
   of the HPEXPIREAT command.
2. Deleted the field because the time is in the past - propagate also HDEL command
   to delete the field and remove the field from the propagated HPEXPIREAT.
3. Condition not met for the field - Remove the field from the propagated
   HPEXPIREAT command.
4. Field does not exists - Remove the field from the propagated HPEXPIREAT command.

If none of the provided fields match option number 1, then avoid also propagating
the HPEXPIREAT command to the replica.

This approach is aligned with the EXPIRE command:
If a given key has already expired, then DEL will be propagated instead of
EXPIRE command. If condition not met, then command will be rejected. Otherwise,
EXPIRE command will be propagated for given key.
---
 src/t_hash.c                          | 85 +++++++++++++++++++++------
 tests/unit/type/hash-field-expire.tcl | 28 +++++++--
 2 files changed, 90 insertions(+), 23 deletions(-)

diff --git a/src/t_hash.c b/src/t_hash.c
index 208e6775b..b42e3c259 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -540,8 +540,10 @@ SetExRes hashTypeSetExpiryListpack(HashTypeSetEx *ex, sds field,
             ex->minExpireFields = prevExpire;
     }
 
-    /* if expiration time is in the past */
+    /* If expired, then delete the field and propagate the deletion.
+     * If replica, continue like the field is valid */
     if (unlikely(checkAlreadyExpired(expireAt))) {
+        propagateHashFieldDeletion(ex->db, ex->key->ptr, field, sdslen(field));
         hashTypeDelete(ex->hashObj, field, 1);
         ex->fieldDeleted++;
         return HSETEX_DELETED;
@@ -1034,8 +1036,11 @@ SetExRes hashTypeSetExpiryHT(HashTypeSetEx *exInfo, sds field, uint64_t expireAt
     dictSetKey(ht, existingEntry, hfNew);
 
 
-    /* if expiration time is in the past */
+    /* If expired, then delete the field and propagate the deletion.
+     * If replica, continue like the field is valid */
     if (unlikely(checkAlreadyExpired(expireAt))) {
+        /* replicas should not initiate deletion of fields */
+        propagateHashFieldDeletion(exInfo->db, exInfo->key->ptr, field, sdslen(field));
         hashTypeDelete(exInfo->hashObj, field, 1);
         exInfo->fieldDeleted++;
         return HSETEX_DELETED;
@@ -1101,12 +1106,7 @@ void initDictExpireMetadata(sds key, robj *o) {
     m->expireMeta.trash = 1; /* mark as trash (as long it wasn't ebAdd()) */
 }
 
-/*
- * Init HashTypeSetEx struct before calling hashTypeSetEx()
- *
- * Don't have to provide client and "cmd". If provided, then notification once
- * done by function hashTypeSetExDone().
- */
+/* Init HashTypeSetEx struct before calling hashTypeSetEx() */
 int hashTypeSetExInit(robj *key, robj *o, client *c, redisDb *db, const char *cmd,
                       ExpireSetCond expireSetCond, HashTypeSetEx *ex)
 {
@@ -1123,20 +1123,20 @@ int hashTypeSetExInit(robj *key, robj *o, client *c, redisDb *db, const char *cm
     ex->minExpireFields = EB_EXPIRE_TIME_INVALID;
 
     /* Take care that HASH support expiration */
-    if (ex->hashObj->encoding == OBJ_ENCODING_LISTPACK) {
-        hashTypeConvert(ex->hashObj, OBJ_ENCODING_LISTPACK_EX, &c->db->hexpires);
+    if (o->encoding == OBJ_ENCODING_LISTPACK) {
+        hashTypeConvert(o, OBJ_ENCODING_LISTPACK_EX, &c->db->hexpires);
 
-        listpackEx *lpt = ex->hashObj->ptr;
+        listpackEx *lpt = o->ptr;
         dictEntry *de = dbFind(c->db, key->ptr);
         serverAssert(de != NULL);
         lpt->key = dictGetKey(de);
-    } else if (ex->hashObj->encoding == OBJ_ENCODING_HT) {
+    } else if (o->encoding == OBJ_ENCODING_HT) {
         /* Take care dict has HFE metadata */
         if (!isDictWithMetaHFE(ht)) {
             /* Realloc (only header of dict) with metadata for hash-field expiration */
             dictTypeAddMeta(&ht, &mstrHashDictTypeWithHFE);
             dictExpireMetadata *m = (dictExpireMetadata *) dictMetadata(ht);
-            ex->hashObj->ptr = ht;
+            o->ptr = ht;
 
             /* Find the key in the keyspace. Need to keep reference to the key for
              * notifications or even removal of the hash */
@@ -1151,7 +1151,7 @@ int hashTypeSetExInit(robj *key, robj *o, client *c, redisDb *db, const char *cm
     }
 
     /* Read minExpire from attached ExpireMeta to the hash */
-    ex->minExpire = hashTypeGetMinExpire(ex->hashObj, 0);
+    ex->minExpire = hashTypeGetMinExpire(o, 0);
     return C_OK;
 }
 
@@ -3040,11 +3040,34 @@ static void httlGenericCommand(client *c, const char *cmd, long long basetime, i
  * unit is either UNIT_SECONDS or UNIT_MILLISECONDS, and is only used for
  * the argv[2] parameter. The basetime is always specified in milliseconds.
  *
- * Additional flags are supported and parsed via parseExtendedExpireArguments */
+ * PROPAGATE TO REPLICA:
+ *   The command will be translated into HPEXPIREAT and the expiration time will be
+ *   converted to absolute time in milliseconds.
+ *
+ *   As we need to propagate H(P)EXPIRE(AT) command to the replica, each field that
+ *   is mentioned in the command should be categorized into one of the four options:
+ *   1. Field’s expiration time updated successfully - Propagate it to replica as
+ *      part of the HPEXPIREAT command.
+ *   2. The field got deleted since the time is in the past - propagate also HDEL
+ *      command to delete the field. Also remove the field from the propagated
+ *      HPEXPIREAT command.
+ *   3. Condition not met for the field - Remove the field from the propagated
+ *      HPEXPIREAT command.
+ *   4. Field doesn't exists - Remove the field from propagated HPEXPIREAT command.
+ *
+ *   If none of the provided fields match option #1, that is provided time of the
+ *   command is in the past, then avoid propagating the HPEXPIREAT command to the
+ *   replica.
+ *
+ *   This approach is aligned with existing EXPIRE command. If a given key has already
+ *   expired, then DEL will be propagated instead of EXPIRE command. If condition
+ *   not met, then command will be rejected. Otherwise, EXPIRE command will be
+ *   propagated for given key.
+ */
 static void hexpireGenericCommand(client *c, const char *cmd, long long basetime, int unit) {
     long numFields = 0, numFieldsAt = 4;
     long long expire; /* unix time in msec */
-    int expireSetCond = 0;
+    int fieldAt, fieldsNotSet = 0, expireSetCond = 0;
     robj *hashObj, *keyArg = c->argv[1], *expireArg = c->argv[2];
 
     /* Read the hash object */
@@ -3117,14 +3140,38 @@ static void hexpireGenericCommand(client *c, const char *cmd, long long basetime
     hashTypeSetExInit(keyArg, hashObj, c, c->db, cmd, expireSetCond, &exCtx);
     addReplyArrayLen(c, numFields);
 
-    for (int i = 0 ; i < numFields ; i++) {
-        sds field = c->argv[numFieldsAt+i+1]->ptr;
+    fieldAt = numFieldsAt + 1;
+    while (fieldAt < c->argc) {
+        sds field = c->argv[fieldAt]->ptr;
         SetExRes res = hashTypeSetEx(hashObj, field, expire, &exCtx);
+
+        if (unlikely(res != HSETEX_OK)) {
+            /* If the field was not set, prevent field propagation */
+            rewriteClientCommandArgument(c, fieldAt, NULL);
+            fieldsNotSet = 1;
+        } else {
+            ++fieldAt;
+        }
+
         addReplyLongLong(c,res);
     }
+
     hashTypeSetExDone(&exCtx);
 
-    /* rewrite command for the replica sake */
+    /* Avoid propagating command if not even one field was updated (Either because
+     * the time is in the past, and corresponding HDELs were sent, or conditions
+     * not met) then it is useless and invalid to propagate command with no fields */
+    if (exCtx.fieldUpdated == 0) {
+        preventCommandPropagation(c);
+        return;
+    }
+
+    /* If some fields were dropped, rewrite the number of fields */
+    if (fieldsNotSet) {
+        robj *numFieldsObj = createStringObjectFromLongLong(exCtx.fieldUpdated);
+        rewriteClientCommandArgument(c, numFieldsAt, numFieldsObj);
+        decrRefCount(numFieldsObj);
+    }
 
     /* Propagate as HPEXPIREAT millisecond-timestamp. Rewrite only if not already */
     if (c->cmd->proc != hpexpireatCommand) {
diff --git a/tests/unit/type/hash-field-expire.tcl b/tests/unit/type/hash-field-expire.tcl
index cca616b61..8c71ebed2 100644
--- a/tests/unit/type/hash-field-expire.tcl
+++ b/tests/unit/type/hash-field-expire.tcl
@@ -914,14 +914,29 @@ start_server {tags {"external:skip needs:debug"}} {
             r config set hash-max-listpack-entries 512
         }
 
-        test "Command rewrite and expired hash fields are propagated to replica ($type)" {
+        test "Test Command propagated to replica as expected ($type)" {
             start_server {overrides {appendonly {yes} appendfsync always} tags {external:skip}} {
 
                 set aof [get_last_incr_aof_path r]
+
+                # Time is in the past so it should propagate HDELs to replica
+                # and delete the fields
+                r hset h0 x1 y1 x2 y2
+                r hexpireat h0 1 fields 3 x1 x2 non_exists_field
+
                 r hset h1 f1 v1 f2 v2
 
+                # Next command won't be propagated to replica
+                # because XX condition not met or field not exists
+                r hexpire h1 10 XX FIELDS 1 f1 f2 non_exists_field
+
                 r hpexpire h1 20 FIELDS 1 f1
-                r hpexpire h1 30 FIELDS 1 f2
+
+                # Next command will be propagate with only field 'f2'
+                # because NX condition not met for field 'f1'
+                r hpexpire h1 30 NX FIELDS 1 f1 f2
+
+                # Non exists field should be ignored
                 r hpexpire h1 30 FIELDS 1 non_exists_field
                 r hset h2 f1 v1 f2 v2 f3 v3 f4 v4
                 r hpexpire h2 40 FIELDS 2 f1 non_exists_field
@@ -938,11 +953,16 @@ start_server {tags {"external:skip needs:debug"}} {
                 # Assert that each TTL-related command are persisted with absolute timestamps in AOF
                 assert_aof_content $aof {
                     {select *}
+                    {hset h0 x1 y1 x2 y2}
+                    {multi}
+                        {hdel h0 x1}
+                        {hdel h0 x2}
+                    {exec}
                     {hset h1 f1 v1 f2 v2}
                     {hpexpireat h1 * FIELDS 1 f1}
                     {hpexpireat h1 * FIELDS 1 f2}
                     {hset h2 f1 v1 f2 v2 f3 v3 f4 v4}
-                    {hpexpireat h2 * FIELDS 2 f1 non_exists_field}
+                    {hpexpireat h2 * FIELDS 1 f1}
                     {hpexpireat h2 * FIELDS 1 f2}
                     {hpexpireat h2 * FIELDS 1 f3}
                     {hpexpireat h2 * FIELDS 1 f4}
@@ -1072,7 +1092,7 @@ start_server {tags {"external:skip needs:debug"}} {
                 {hset h2 f2 v2}
                 {hpexpireat h2 * NX FIELDS 1 f2}
                 {hset h3 f3 v3 f4 v4 f5 v5}
-                {hpexpireat h3 * FIELDS 3 f3 f4 non_exists_field}
+                {hpexpireat h3 * FIELDS 2 f3 f4}
                 {hpersist h3 FIELDS 1 f3}
             }
             close_replication_stream $repl

From 52e12d8bac139709c5b9791d90d791ae492a9244 Mon Sep 17 00:00:00 2001
From: "debing.sun" <debing.sun@redis.com>
Date: Wed, 26 Jun 2024 08:26:23 +0800
Subject: [PATCH 12/14] Don't keep global replication buffer reference for
 replicas marked CLIENT_CLOSE_ASAP (#13363)

In certain situations, we might generate a large number of propagates
(e.g., multi/exec, Lua script, or a single command generating tons of
propagations) within an event loop.
During the process of propagating to a replica, if the replica is
disconnected(marked as CLIENT_CLOSE_ASAP) due to exceeding the output
buffer limit, we should remove its reference to the global replication
buffer to avoid the global replication buffer being unable to be
properly trimmed due to being referenced.

---------

Co-authored-by: oranagra <oran@redislabs.com>
---
 src/networking.c                   |  3 +++
 src/replication.c                  |  3 +++
 tests/modules/propagate.c          | 20 ++++++++++++++++
 tests/unit/moduleapi/propagate.tcl | 38 ++++++++++++++++++++++++++++++
 4 files changed, 64 insertions(+)

diff --git a/src/networking.c b/src/networking.c
index 5098b3812..be5fa0694 100644
--- a/src/networking.c
+++ b/src/networking.c
@@ -1733,6 +1733,9 @@ void freeClientAsync(client *c) {
      * idle. */
     if (c->flags & CLIENT_CLOSE_ASAP || c->flags & CLIENT_SCRIPT) return;
     c->flags |= CLIENT_CLOSE_ASAP;
+    /* Replicas that was marked as CLIENT_CLOSE_ASAP should not keep the
+     * replication backlog from been trimmed. */
+    if (c->flags & CLIENT_SLAVE) freeReplicaReferencedReplBuffer(c);
     if (server.io_threads_num == 1) {
         /* no need to bother with locking if there's just one thread (the main thread) */
         listAddNodeTail(server.clients_to_close,c);
diff --git a/src/replication.c b/src/replication.c
index 1bfd11a16..a3d4eb15c 100644
--- a/src/replication.c
+++ b/src/replication.c
@@ -189,6 +189,9 @@ int canFeedReplicaReplBuffer(client *replica) {
     /* Don't feed replicas that are still waiting for BGSAVE to start. */
     if (replica->replstate == SLAVE_STATE_WAIT_BGSAVE_START) return 0;
 
+    /* Don't feed replicas that are going to be closed ASAP. */
+    if (replica->flags & CLIENT_CLOSE_ASAP) return 0;
+
     return 1;
 }
 
diff --git a/tests/modules/propagate.c b/tests/modules/propagate.c
index 879eebd2c..7e737589e 100644
--- a/tests/modules/propagate.c
+++ b/tests/modules/propagate.c
@@ -302,6 +302,21 @@ int propagateTestIncr(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
     return REDISMODULE_OK;
 }
 
+int propagateTestVerbatim(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
+    if (argc < 2){
+        RedisModule_WrongArity(ctx);
+        return REDISMODULE_OK;
+    }
+
+    long long replicate_num;
+    RedisModule_StringToLongLong(argv[1], &replicate_num);
+    /* Replicate the command verbatim for the specified number of times. */
+    for (long long i = 0; i < replicate_num; i++)
+        RedisModule_ReplicateVerbatim(ctx);
+    RedisModule_ReplyWithSimpleString(ctx,"OK");
+    return REDISMODULE_OK;
+}
+
 int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
     REDISMODULE_NOT_USED(argv);
     REDISMODULE_NOT_USED(argc);
@@ -368,6 +383,11 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
                 propagateTestIncr,
                 "write",1,1,1) == REDISMODULE_ERR)
             return REDISMODULE_ERR;
+        
+    if (RedisModule_CreateCommand(ctx,"propagate-test.verbatim",
+            propagateTestVerbatim,
+            "write",1,1,1) == REDISMODULE_ERR)
+        return REDISMODULE_ERR;
 
     return REDISMODULE_OK;
 }
diff --git a/tests/unit/moduleapi/propagate.tcl b/tests/unit/moduleapi/propagate.tcl
index 90a369da2..5e32e4e49 100644
--- a/tests/unit/moduleapi/propagate.tcl
+++ b/tests/unit/moduleapi/propagate.tcl
@@ -761,3 +761,41 @@ tags "modules aof" {
     }
     }
 }
+
+# This test does not really test module functionality, but rather uses a module
+# command to test Redis replication mechanisms.
+test {Replicas that was marked as CLIENT_CLOSE_ASAP should not keep the replication backlog from been trimmed} {
+    start_server [list overrides [list loadmodule "$testmodule"]] {
+        set replica [srv 0 client]
+        start_server [list overrides [list loadmodule "$testmodule"]] {
+            set master [srv 0 client]
+            set master_host [srv 0 host]
+            set master_port [srv 0 port]
+            $master config set client-output-buffer-limit "replica 10mb 5mb 0"
+
+            # Start the replication process...
+            $replica replicaof $master_host $master_port
+            wait_for_sync $replica
+
+            test {module propagates from timer} {
+                # Replicate large commands to make the replica disconnected.
+                $master write [format_command propagate-test.verbatim 100000 [string repeat "a" 1000]] ;# almost 100mb
+                # Execute this command together with module commands within the same
+                # event loop to prevent periodic cleanup of replication backlog.
+                $master write [format_command info memory]
+                $master flush
+                $master read ;# propagate-test.verbatim
+                set res [$master read] ;# info memory
+
+                # Wait for the replica to be disconnected.
+                wait_for_log_messages 0 {"*flags=S*scheduled to be closed ASAP for overcoming of output buffer limits*"} 0 1500 10
+                # Due to the replica reaching the soft limit (5MB), memory peaks should not significantly
+                # exceed the replica soft limit. Furthermore, as the replica release its reference to
+                # replication backlog, it should be properly trimmed, the memory usage of replication
+                # backlog should not significantly exceed repl-backlog-size (default 1MB). */
+                assert_lessthan [getInfoProperty $res used_memory_peak] 10000000;# less than 10mb
+                assert_lessthan [getInfoProperty $res mem_replication_backlog] 2000000;# less than 2mb
+            }
+        }
+    }
+}

From a9267137ee5cbd0908d3844e9d57284e93d85b72 Mon Sep 17 00:00:00 2001
From: Moti Cohen <moticless@gmail.com>
Date: Wed, 26 Jun 2024 14:12:06 +0300
Subject: [PATCH 13/14] HFE - count in command must match actual number of
 fields (#13369)

There was wrong preliminary assumption that we can optionally provide
vector of arguments more than count.
This is error-prone approach that leaded to actual error in that case.
This PR enforce that vector of argument match count.

Also fixed flaky HRANDFIELD test.
---
 src/t_hash.c                          | 13 ++---
 tests/unit/type/hash-field-expire.tcl | 70 ++++++++++++++++++---------
 2 files changed, 54 insertions(+), 29 deletions(-)

diff --git a/src/t_hash.c b/src/t_hash.c
index b42e3c259..45156f46e 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -2929,8 +2929,8 @@ static void httlGenericCommand(client *c, const char *cmd, long long basetime, i
         return;
 
     /* Verify `numFields` is consistent with number of arguments */
-    if (numFields > (c->argc - numFieldsAt - 1)) {
-        addReplyError(c, "Parameter `numFields` is more than number of arguments");
+    if (numFields != (c->argc - numFieldsAt - 1)) {
+        addReplyError(c, "The `numfields` parameter must match the number of arguments");
         return;
     }
 
@@ -3078,6 +3078,7 @@ static void hexpireGenericCommand(client *c, const char *cmd, long long basetime
     /* Read the expiry time from command */
     if (getLongLongFromObjectOrReply(c, expireArg, &expire, NULL) != C_OK)
         return;
+
     if (expire < 0) {
         addReplyError(c,"invalid expire time, must be >= 0");
         return;
@@ -3121,8 +3122,8 @@ static void hexpireGenericCommand(client *c, const char *cmd, long long basetime
         return;
 
     /* Verify `numFields` is consistent with number of arguments */
-    if (numFields > (c->argc - numFieldsAt - 1)) {
-        addReplyError(c, "Parameter `numFields` is more than number of arguments");
+    if (numFields != (c->argc - numFieldsAt - 1)) {
+        addReplyError(c, "The `numfields` parameter must match the number of arguments");
         return;
     }
 
@@ -3249,8 +3250,8 @@ void hpersistCommand(client *c) {
         return;
 
     /* Verify `numFields` is consistent with number of arguments */
-    if (numFields > (c->argc - numFieldsAt - 1)) {
-        addReplyError(c, "Parameter `numFields` is more than number of arguments");
+    if (numFields != (c->argc - numFieldsAt - 1)) {
+        addReplyError(c, "The `numfields` parameter must match the number of arguments");
         return;
     }
 
diff --git a/tests/unit/type/hash-field-expire.tcl b/tests/unit/type/hash-field-expire.tcl
index 8c71ebed2..b6dd58043 100644
--- a/tests/unit/type/hash-field-expire.tcl
+++ b/tests/unit/type/hash-field-expire.tcl
@@ -149,7 +149,9 @@ start_server {tags {"external:skip needs:debug"}} {
             r del myhash
             r hset myhash f1 v1
             assert_error {*Parameter `numFields` should be greater than 0} {r hpexpire myhash 1000 NX FIELDS 0 f1 f2 f3}
-            assert_error {*Parameter `numFields` is more than number of arguments} {r hpexpire myhash 1000 NX FIELDS 4 f1 f2 f3}
+            # <count> not match with actual number of fields
+            assert_error {*parameter must match the number*} {r hpexpire myhash 1000 NX FIELDS 4 f1 f2 f3}
+            assert_error {*parameter must match the number*} {r hpexpire myhash 1000 NX FIELDS 2 f1 f2 f3}
         }
 
         test "HPEXPIRE - parameter expire-time near limit of  2^46 ($type)" {
@@ -262,8 +264,9 @@ start_server {tags {"external:skip needs:debug"}} {
 
             foreach cmd {HTTL HPTTL} {
                 assert_equal [r $cmd myhash FIELDS 2 field2 non_exists_field] "$T_NO_EXPIRY $T_NO_FIELD"
-                # Set numFields less than actual number of fields. Fine.
-                assert_equal [r $cmd myhash FIELDS 1 non_exists_field1 non_exists_field2] "$T_NO_FIELD"
+                # <count> not match with actual number of fields
+                assert_error {*parameter must match the number*} {r $cmd myhash FIELDS 1 non_exists_field1 non_exists_field2}
+                assert_error {*parameter must match the number*} {r $cmd myhash FIELDS 3 non_exists_field1 non_exists_field2}
             }
         }
 
@@ -674,6 +677,9 @@ start_server {tags {"external:skip needs:debug"}} {
             assert_error {*wrong number of arguments*} {r hpersist myhash FIELDS 1}
             assert_equal [r hpersist myhash FIELDS 2 f1 not-exists-field] "$P_OK $P_NO_FIELD"
             assert_equal [r hpersist myhash FIELDS 1 f2] "$P_NO_EXPIRY"
+            # <count> not match with actual number of fields
+            assert_error {*parameter must match the number*} {r hpersist myhash FIELDS 2 f1 f2 f3}
+            assert_error {*parameter must match the number*} {r hpersist myhash FIELDS 4 f1 f2 f3}
         }
 
         test "HPERSIST - verify fields with TTL are persisted ($type)" {
@@ -928,13 +934,13 @@ start_server {tags {"external:skip needs:debug"}} {
 
                 # Next command won't be propagated to replica
                 # because XX condition not met or field not exists
-                r hexpire h1 10 XX FIELDS 1 f1 f2 non_exists_field
+                r hexpire h1 10 XX FIELDS 3 f1 f2 non_exists_field
 
                 r hpexpire h1 20 FIELDS 1 f1
 
                 # Next command will be propagate with only field 'f2'
                 # because NX condition not met for field 'f1'
-                r hpexpire h1 30 NX FIELDS 1 f1 f2
+                r hpexpire h1 30 NX FIELDS 2 f1 f2
 
                 # Non exists field should be ignored
                 r hpexpire h1 30 FIELDS 1 non_exists_field
@@ -1035,8 +1041,8 @@ start_server {tags {"external:skip needs:debug"}} {
 
                 # Verify HRANDFIELD deletes expired fields and propagates it
                 r hset h2 f1 v1 f2 v2
-                r hpexpire h2 1 FIELDS 1 f1
-                r hpexpire h2 50 FIELDS 1 f2
+                r hpexpire h2 1 FIELDS 2 f1 f2
+                after 5
                 assert_equal [r hrandfield h4 2] ""
                 after 200
 
@@ -1048,10 +1054,9 @@ start_server {tags {"external:skip needs:debug"}} {
                     {hpexpireat h1 * NX FIELDS 3 f3 f4 f5}
                     {hpexpireat h1 * FIELDS 1 f6}
                     {hset h2 f1 v1 f2 v2}
-                    {hpexpireat h2 * FIELDS 1 f1}
-                    {hpexpireat h2 * FIELDS 1 f2}
-                    {hdel h2 f1}
-                    {hdel h2 f2}
+                    {hpexpireat h2 * FIELDS 2 f1 f2}
+                    {hdel h2 *}
+                    {hdel h2 *}
                 }
 
                 array set keyAndFields1 [dumpAllHashes r]
@@ -1099,26 +1104,46 @@ start_server {tags {"external:skip needs:debug"}} {
         } {} {needs:repl}
 
         test {HRANDFIELD delete expired fields and propagate DELs to replica} {
+            r debug set-active-expire 0
             r flushall
             set repl [attach_to_replication_stream]
 
-            r hset h4 f1 v1 f2 v2
-            r hpexpire h4 1 FIELDS 1 f1
-            r hpexpire h4 2 FIELDS 1 f2
-            after 100
-            assert_equal [r hrandfield h4 2] ""
+            # HRANDFIELD delete expired fields and propagate MULTI-EXEC DELs. Reply none.
+            r hset h1 f1 v1 f2 v2
+            r hpexpire h1 1 FIELDS 2 f1 f2
+            after 5
+            assert_equal [r hrandfield h1 2] ""
 
+            # HRANDFIELD delete expired field and propagate DEL. Reply non-expired field.
+            r hset h2 f1 v1 f2 v2
+            r hpexpire h2 1 FIELDS 1 f1
+            after 5
+            assert_equal [r hrandfield h2 2] "f2"
+
+            # HRANDFIELD delete expired field and propagate DEL. Reply none.
+            r hset h3 f1 v1
+            r hpexpire h3 1 FIELDS 1 f1
+            after 5
+            assert_equal [r hrandfield h3 2] ""
 
             assert_replication_stream $repl {
                 {select *}
-                {hset h4 f1 v1 f2 v2}
-                {hpexpireat h4 * FIELDS 1 f1}
-                {hpexpireat h4 * FIELDS 1 f2}
-                {hdel h4 f1}
-                {hdel h4 f2}
+                {hset h1 f1 v1 f2 v2}
+                {hpexpireat h1 * FIELDS 2 f1 f2}
+                {multi}
+                {hdel h1 *}
+                {hdel h1 *}
+                {exec}
+                {hset h2 f1 v1 f2 v2}
+                {hpexpireat h2 * FIELDS 1 f1}
+                {hdel h2 f1}
+                {hset h3 f1 v1}
+                {hpexpireat h3 * FIELDS 1 f1}
+                {hdel h3 f1}
             }
             close_replication_stream $repl
-        } {} {needs:repl}
+            r debug set-active-expire 1
+        } {OK} {needs:repl}
 
         # Start another server to test replication of TTLs
         start_server {tags {needs:repl external:skip}} {
@@ -1163,4 +1188,3 @@ start_server {tags {"external:skip needs:debug"}} {
         }
     }
 }
-

From 4000bb2ee9c2630cd87cea2e450e58d8327a78e7 Mon Sep 17 00:00:00 2001
From: YaacovHazan <yaacov.hazan@redislabs.com>
Date: Thu, 27 Jun 2024 09:36:36 +0300
Subject: [PATCH 14/14] Redis 7.4 RC2

---
 00-RELEASENOTES | 22 +++++++++++++++++++++-
 src/version.h   |  4 ++--
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/00-RELEASENOTES b/00-RELEASENOTES
index a21c68f0a..f6c612df6 100644
--- a/00-RELEASENOTES
+++ b/00-RELEASENOTES
@@ -12,6 +12,26 @@ SECURITY: There are security fixes in the release.
 --------------------------------------------------------------------------------
 
 
+================================================================================
+Redis 7.4 RC2   Released Thu 27 Jun 2024 10:00:00 IST
+================================================================================
+
+Upgrade urgency LOW: This is the second Release Candidate for Redis 7.4.
+
+Performance and resource utilization improvements
+=================================================
+* #13296 Optimize CPU cache efficiency
+
+Changes to new 7.4 new features (compared to 7.4 RC1)
+=====================================================
+* #13343 Hash - expiration of individual fields: when key does not exist - reply with an array (nonexisting code for each field)
+* #13329 Hash - expiration of individual fields: new keyspace event: `hexpired`
+
+Modules API - Potentially breaking changes to new 7.4 features (compared to 7.4 RC1)
+====================================================================================
+* #13326 Hash - expiration of individual fields: avoid lazy expire when called from a Modules API function
+
+
 ================================================================================
 Redis 7.4 RC1   Released Thu 6 Jun 2024 10:00:00 IST
 ================================================================================
@@ -75,7 +95,7 @@ Other general improvements
 * #13020 Allow adjusting defrag configurations while active defragmentation is running
 * #12949 Increase the accuracy of avg_ttl (the average keyspace keys TTL)
 * #12977 Allow running `WAITAOF` in scripts
-* #12782 Implement TCP Keep-Alives across most Unix-like systems
+* #12782 Implement TCP keep-alive across most Unix-like systems
 * #12707 Improved error codes when rejecting scripts in cluster mode
 * #12596 Support `XREAD ... BLOCK` in scripts; rejected only if it ends up blocking
 
diff --git a/src/version.h b/src/version.h
index cf09654a4..d6e1fd4e8 100644
--- a/src/version.h
+++ b/src/version.h
@@ -1,2 +1,2 @@
-#define REDIS_VERSION "7.3.240"
-#define REDIS_VERSION_NUM 0x000703f0
+#define REDIS_VERSION "7.3.241"
+#define REDIS_VERSION_NUM 0x000703f1