diff --git a/src/t_hash.c b/src/t_hash.c index 4752568a3..66a3083c6 100644 --- a/src/t_hash.c +++ b/src/t_hash.c @@ -1985,6 +1985,21 @@ uint64_t hashTypeRemoveFromExpires(ebuckets *hexpires, robj *o) { return expireTime; } +int hashTypeIsFieldsWithExpire(robj *o) { + if (o->encoding == OBJ_ENCODING_LISTPACK) { + return 0; + } else if (o->encoding == OBJ_ENCODING_LISTPACK_EX) { + return EB_EXPIRE_TIME_INVALID != listpackExGetMinExpire(o); + } else { /* o->encoding == OBJ_ENCODING_HT */ + dict *d = o->ptr; + /* If dict doesn't holds HFE metadata */ + if (!isDictWithMetaHFE(d)) + return 0; + dictExpireMetadata *meta = (dictExpireMetadata *) dictMetadata(d); + return ebGetTotalItems(meta->hfe, &hashFieldExpireBucketsType) != 0; + } +} + /* Add hash to global HFE DS and update key for notifications. * * key - must be the same key instance that is persisted in db->dict @@ -2315,6 +2330,14 @@ void hdelCommand(client *c) { if ((o = lookupKeyWriteOrReply(c,c->argv[1],shared.czero)) == NULL || checkType(c,o,OBJ_HASH)) return; + /* Hash field expiration is optimized to avoid frequent update global HFE DS for + * each field deletion. Eventually active-expiration will run and update or remove + * the hash from global HFE DS gracefully. Nevertheless, statistic "subexpiry" + * might reflect wrong number of hashes with HFE to the user if it is the last + * field with expiration. The following logic checks if this is indeed the last + * field with expiration and removes it from global HFE DS. */ + int isHFE = hashTypeIsFieldsWithExpire(o); + for (j = 2; j < c->argc; j++) { if (hashTypeDelete(o,c->argv[j]->ptr,1)) { deleted++; @@ -2328,9 +2351,13 @@ void hdelCommand(client *c) { if (deleted) { signalModifiedKey(c,c->db,c->argv[1]); notifyKeyspaceEvent(NOTIFY_HASH,"hdel",c->argv[1],c->db->id); - if (keyremoved) - notifyKeyspaceEvent(NOTIFY_GENERIC,"del",c->argv[1], - c->db->id); + if (keyremoved) { + notifyKeyspaceEvent(NOTIFY_GENERIC, "del", c->argv[1], c->db->id); + } else { + if (isHFE && (hashTypeIsFieldsWithExpire(o) == 0)) /* is it last HFE */ + ebRemove(&c->db->hexpires, &hashExpireBucketsType, o); + } + server.dirty += deleted; } addReplyLongLong(c,deleted); diff --git a/tests/unit/type/hash-field-expire.tcl b/tests/unit/type/hash-field-expire.tcl index d753ca8b8..2c0ff8abd 100644 --- a/tests/unit/type/hash-field-expire.tcl +++ b/tests/unit/type/hash-field-expire.tcl @@ -786,6 +786,36 @@ start_server {tags {"external:skip needs:debug"}} { } } + test "Statistics - Hashes with HFEs ($type)" { + r config resetstat + r flushall + + # hash1: 5 fields, 3 with TTL. subexpiry incr +1 + r hset myhash f1 v1 f2 v2 f3 v3 f4 v4 f5 v5 + r hpexpire myhash 150 FIELDS 3 f1 f2 f3 + assert_match [get_stat_subexpiry r] 1 + + # hash2: 5 fields, 3 with TTL. subexpiry incr +1 + r hset myhash2 f1 v1 f2 v2 f3 v3 f4 v4 f5 v5 + assert_match [get_stat_subexpiry r] 1 + r hpexpire myhash2 100 FIELDS 3 f1 f2 f3 + assert_match [get_stat_subexpiry r] 2 + + # hash3: 2 fields, 1 with TTL. HDEL field with TTL. subexpiry decr -1 + r hset myhash3 f1 v1 f2 v2 + r hpexpire myhash3 100 FIELDS 1 f2 + assert_match [get_stat_subexpiry r] 3 + r hdel myhash3 f2 + assert_match [get_stat_subexpiry r] 2 + + # Expired fields of hash1 and hash2. subexpiry decr -2 + wait_for_condition 50 50 { + [get_stat_subexpiry r] == 0 + } else { + fail "Hash field expiry statistics failed" + } + } + r config set hash-max-listpack-entries 512 } @@ -899,25 +929,6 @@ start_server {tags {"external:skip needs:debug"}} { r config set hash-max-listpack-value 64 } - - test "Statistics - Hashes with HFEs" { - r config resetstat - r del myhash - r hset myhash f1 v1 f2 v2 f3 v3 f4 v4 f5 v5 - r hpexpire myhash 100 FIELDS 3 f1 f2 f3 - - assert_match [get_stat_subexpiry r] 1 - r hset myhash2 f1 v1 f2 v2 f3 v3 f4 v4 f5 v5 - assert_match [get_stat_subexpiry r] 1 - r hpexpire myhash2 100 FIELDS 3 f1 f2 f3 - assert_match [get_stat_subexpiry r] 2 - - wait_for_condition 50 50 { - [get_stat_subexpiry r] == 0 - } else { - fail "Hash field expiry statistics failed" - } - } } start_server {tags {"external:skip needs:debug"}} {