Unverified Commit a84cc20a authored by Moti Cohen's avatar Moti Cohen Committed by GitHub
Browse files

HFE - Fix statistic to count also lazy expired and rename INFO params (#13372)

* INFO command : rename `hashes_with_expiry_fields` to `subexpiry`
* INFO command : rename `expired_hash_fields` to `expired_subkeys`
* Fix statistic of `expired_subkeys` to count also lazy expired
* Remove TODOs comments leftover in TCL
* Fix potential flaky test of rdb load of hash-field-expiration
parent 76d179c6
...@@ -2524,7 +2524,7 @@ void resetServerStats(void) { ...@@ -2524,7 +2524,7 @@ void resetServerStats(void) {
server.stat_numcommands = 0; server.stat_numcommands = 0;
server.stat_numconnections = 0; server.stat_numconnections = 0;
server.stat_expiredkeys = 0; server.stat_expiredkeys = 0;
server.stat_expired_hash_fields = 0; server.stat_expired_subkeys = 0;
server.stat_expired_stale_perc = 0; server.stat_expired_stale_perc = 0;
server.stat_expired_time_cap_reached_count = 0; server.stat_expired_time_cap_reached_count = 0;
server.stat_expire_cycle_time_used = 0; server.stat_expire_cycle_time_used = 0;
...@@ -5877,7 +5877,7 @@ sds genRedisInfoString(dict *section_dict, int all_sections, int everything) { ...@@ -5877,7 +5877,7 @@ sds genRedisInfoString(dict *section_dict, int all_sections, int everything) {
"sync_full:%lld\r\n", server.stat_sync_full, "sync_full:%lld\r\n", server.stat_sync_full,
"sync_partial_ok:%lld\r\n", server.stat_sync_partial_ok, "sync_partial_ok:%lld\r\n", server.stat_sync_partial_ok,
"sync_partial_err:%lld\r\n", server.stat_sync_partial_err, "sync_partial_err:%lld\r\n", server.stat_sync_partial_err,
"expired_hash_fields:%lld\r\n", server.stat_expired_hash_fields, "expired_subkeys:%lld\r\n", server.stat_expired_subkeys,
"expired_keys:%lld\r\n", server.stat_expiredkeys, "expired_keys:%lld\r\n", server.stat_expiredkeys,
"expired_stale_perc:%.2f\r\n", server.stat_expired_stale_perc*100, "expired_stale_perc:%.2f\r\n", server.stat_expired_stale_perc*100,
"expired_time_cap_reached_count:%lld\r\n", server.stat_expired_time_cap_reached_count, "expired_time_cap_reached_count:%lld\r\n", server.stat_expired_time_cap_reached_count,
...@@ -6124,7 +6124,7 @@ sds genRedisInfoString(dict *section_dict, int all_sections, int everything) { ...@@ -6124,7 +6124,7 @@ sds genRedisInfoString(dict *section_dict, int all_sections, int everything) {
if (keys || vkeys) { if (keys || vkeys) {
info = sdscatprintf(info, info = sdscatprintf(info,
"db%d:keys=%lld,expires=%lld,avg_ttl=%lld,hashes_with_expiry_fields=%lld\r\n", "db%d:keys=%lld,expires=%lld,avg_ttl=%lld,subexpiry=%lld\r\n",
j, keys, vkeys, server.db[j].avg_ttl, hexpires); j, keys, vkeys, server.db[j].avg_ttl, hexpires);
} }
} }
......
...@@ -1651,7 +1651,7 @@ struct redisServer { ...@@ -1651,7 +1651,7 @@ struct redisServer {
long long stat_numcommands; /* Number of processed commands */ long long stat_numcommands; /* Number of processed commands */
long long stat_numconnections; /* Number of connections received */ long long stat_numconnections; /* Number of connections received */
long long stat_expiredkeys; /* Number of expired keys */ long long stat_expiredkeys; /* Number of expired keys */
long long stat_expired_hash_fields; /* Number of expired hash-fields */ long long stat_expired_subkeys; /* Number of expired subkeys (Currently only hash-fields) */
double stat_expired_stale_perc; /* Percentage of keys probably expired */ double stat_expired_stale_perc; /* Percentage of keys probably expired */
long long stat_expired_time_cap_reached_count; /* Early expire cycle stops.*/ long long stat_expired_time_cap_reached_count; /* Early expire cycle stops.*/
long long stat_expire_cycle_time_used; /* Cumulative microseconds used. */ long long stat_expire_cycle_time_used; /* Cumulative microseconds used. */
......
...@@ -414,6 +414,7 @@ void listpackExExpire(redisDb *db, robj *o, ExpireInfo *info) { ...@@ -414,6 +414,7 @@ void listpackExExpire(redisDb *db, robj *o, ExpireInfo *info) {
break; break;
propagateHashFieldDeletion(db, ((listpackEx *) o->ptr)->key, (char *)((fref) ? fref : intbuf), flen); propagateHashFieldDeletion(db, ((listpackEx *) o->ptr)->key, (char *)((fref) ? fref : intbuf), flen);
server.stat_expired_subkeys++;
ptr = lpNext(lpt->lp, ptr); ptr = lpNext(lpt->lp, ptr);
...@@ -545,6 +546,7 @@ SetExRes hashTypeSetExpiryListpack(HashTypeSetEx *ex, sds field, ...@@ -545,6 +546,7 @@ SetExRes hashTypeSetExpiryListpack(HashTypeSetEx *ex, sds field,
if (unlikely(checkAlreadyExpired(expireAt))) { if (unlikely(checkAlreadyExpired(expireAt))) {
propagateHashFieldDeletion(ex->db, ex->key->ptr, field, sdslen(field)); propagateHashFieldDeletion(ex->db, ex->key->ptr, field, sdslen(field));
hashTypeDelete(ex->hashObj, field, 1); hashTypeDelete(ex->hashObj, field, 1);
server.stat_expired_subkeys++;
ex->fieldDeleted++; ex->fieldDeleted++;
return HSETEX_DELETED; return HSETEX_DELETED;
} }
...@@ -758,6 +760,7 @@ GetFieldRes hashTypeGetValue(redisDb *db, robj *o, sds field, unsigned char **vs ...@@ -758,6 +760,7 @@ GetFieldRes hashTypeGetValue(redisDb *db, robj *o, sds field, unsigned char **vs
/* delete the field and propagate the deletion */ /* delete the field and propagate the deletion */
serverAssert(hashTypeDelete(o, field, 1) == 1); serverAssert(hashTypeDelete(o, field, 1) == 1);
propagateHashFieldDeletion(db, key, field, sdslen(field)); propagateHashFieldDeletion(db, key, field, sdslen(field));
server.stat_expired_subkeys++;
/* If the field is the last one in the hash, then the hash will be deleted */ /* If the field is the last one in the hash, then the hash will be deleted */
res = GETF_EXPIRED; res = GETF_EXPIRED;
...@@ -1042,6 +1045,7 @@ SetExRes hashTypeSetExpiryHT(HashTypeSetEx *exInfo, sds field, uint64_t expireAt ...@@ -1042,6 +1045,7 @@ SetExRes hashTypeSetExpiryHT(HashTypeSetEx *exInfo, sds field, uint64_t expireAt
/* replicas should not initiate deletion of fields */ /* replicas should not initiate deletion of fields */
propagateHashFieldDeletion(exInfo->db, exInfo->key->ptr, field, sdslen(field)); propagateHashFieldDeletion(exInfo->db, exInfo->key->ptr, field, sdslen(field));
hashTypeDelete(exInfo->hashObj, field, 1); hashTypeDelete(exInfo->hashObj, field, 1);
server.stat_expired_subkeys++;
exInfo->fieldDeleted++; exInfo->fieldDeleted++;
return HSETEX_DELETED; return HSETEX_DELETED;
} }
...@@ -1833,7 +1837,6 @@ static uint64_t hashTypeExpire(robj *o, ExpireCtx *expireCtx, int updateGlobalHF ...@@ -1833,7 +1837,6 @@ static uint64_t hashTypeExpire(robj *o, ExpireCtx *expireCtx, int updateGlobalHF
.itemsExpired = 0}; .itemsExpired = 0};
listpackExExpire(db, o, &info); listpackExExpire(db, o, &info);
server.stat_expired_hash_fields += info.itemsExpired;
keystr = ((listpackEx*)o->ptr)->key; keystr = ((listpackEx*)o->ptr)->key;
} else { } else {
serverAssert(o->encoding == OBJ_ENCODING_HT); serverAssert(o->encoding == OBJ_ENCODING_HT);
...@@ -2887,7 +2890,7 @@ static ExpireAction onFieldExpire(eItem item, void *ctx) { ...@@ -2887,7 +2890,7 @@ static ExpireAction onFieldExpire(eItem item, void *ctx) {
dictExpireMetadata *dictExpireMeta = (dictExpireMetadata *) dictMetadata(d); dictExpireMetadata *dictExpireMeta = (dictExpireMetadata *) dictMetadata(d);
propagateHashFieldDeletion(expCtx->db, dictExpireMeta->key, hf, hfieldlen(hf)); propagateHashFieldDeletion(expCtx->db, dictExpireMeta->key, hf, hfieldlen(hf));
serverAssert(hashTypeDelete(expCtx->hashObj, hf, 0) == 1); serverAssert(hashTypeDelete(expCtx->hashObj, hf, 0) == 1);
server.stat_expired_hash_fields++; server.stat_expired_subkeys++;
return ACT_REMOVE_EXP_ITEM; return ACT_REMOVE_EXP_ITEM;
} }
......
...@@ -442,15 +442,21 @@ start_server [list overrides [list "dir" $server_path]] { ...@@ -442,15 +442,21 @@ start_server [list overrides [list "dir" $server_path]] {
restart_server 0 true false restart_server 0 true false
wait_done_loading r wait_done_loading r
assert_equal [lsort [r hgetall key]] "1 2 3 a b c" # Never be sure when active-expire kicks in into action
wait_for_condition 100 10 {
[lsort [r hgetall key]] == "1 2 3 a b c"
} else {
fail "hgetall of key is not as expected"
}
assert_equal [r hpexpiretime key FIELDS 3 a b c] {2524600800000 65755674080852 -1} assert_equal [r hpexpiretime key FIELDS 3 a b c] {2524600800000 65755674080852 -1}
assert_equal [s rdb_last_load_keys_loaded] 1 assert_equal [s rdb_last_load_keys_loaded] 1
# wait until expired_hash_fields equals 2 # wait until expired_subkeys equals 2
wait_for_condition 10 100 { wait_for_condition 10 100 {
[s expired_hash_fields] == 2 [s expired_subkeys] == 2
} else { } else {
fail "Value of expired_hash_fields is not as expected" fail "Value of expired_subkeys is not as expected"
} }
} }
} }
...@@ -562,9 +568,9 @@ foreach {type lp_entries} {listpack 512 dict 0} { ...@@ -562,9 +568,9 @@ foreach {type lp_entries} {listpack 512 dict 0} {
# wait at most 2 secs to make sure 'c' and 'd' will active-expire # wait at most 2 secs to make sure 'c' and 'd' will active-expire
wait_for_condition 20 100 { wait_for_condition 20 100 {
[s expired_hash_fields] == 2 [s expired_subkeys] == 2
} else { } else {
fail "expired hash fields is [s expired_hash_fields] != 2" fail "expired hash fields is [s expired_subkeys] != 2"
} }
assert_equal [s rdb_last_load_keys_loaded] 1 assert_equal [s rdb_last_load_keys_loaded] 1
...@@ -597,7 +603,7 @@ foreach {type lp_entries} {listpack 512 dict 0} { ...@@ -597,7 +603,7 @@ foreach {type lp_entries} {listpack 512 dict 0} {
after 500 after 500
assert_equal [s rdb_last_load_keys_loaded] 1 assert_equal [s rdb_last_load_keys_loaded] 1
assert_equal [s expired_hash_fields] 0 assert_equal [s expired_subkeys] 0
# hgetall will lazy expire fields, so it's only called after the stat asserts # hgetall will lazy expire fields, so it's only called after the stat asserts
assert_equal [lsort [r hgetall key]] "1 2 5 6 a b e f" assert_equal [lsort [r hgetall key]] "1 2 5 6 a b e f"
......
...@@ -293,7 +293,6 @@ proc findKeyWithType {r type} { ...@@ -293,7 +293,6 @@ proc findKeyWithType {r type} {
proc createComplexDataset {r ops {opt {}}} { proc createComplexDataset {r ops {opt {}}} {
set useexpire [expr {[lsearch -exact $opt useexpire] != -1}] set useexpire [expr {[lsearch -exact $opt useexpire] != -1}]
# TODO: Remove usehexpire on next commit, when RDB will support replication
set usehexpire [expr {[lsearch -exact $opt usehexpire] != -1}] set usehexpire [expr {[lsearch -exact $opt usehexpire] != -1}]
if {[lsearch -exact $opt usetag] != -1} { if {[lsearch -exact $opt usetag] != -1} {
......
...@@ -19,12 +19,12 @@ set P_OK 1 ...@@ -19,12 +19,12 @@ set P_OK 1
############################### AUX FUNCS ###################################### ############################### AUX FUNCS ######################################
proc get_hashes_with_expiry_fields {r} { proc get_stat_subexpiry {r} {
set input_string [r info keyspace] set input_string [r info keyspace]
set hash_count 0 set hash_count 0
foreach line [split $input_string \n] { foreach line [split $input_string \n] {
if {[regexp {hashes_with_expiry_fields=(\d+)} $line -> value]} { if {[regexp {subexpiry=(\d+)} $line -> value]} {
return $value return $value
} }
} }
...@@ -163,9 +163,6 @@ start_server {tags {"external:skip needs:debug"}} { ...@@ -163,9 +163,6 @@ start_server {tags {"external:skip needs:debug"}} {
} }
test "Lazy Expire - fields are lazy deleted ($type)" { test "Lazy Expire - fields are lazy deleted ($type)" {
# TODO remove the SELECT once dbid will be embedded inside dict/listpack
r select 0
r debug set-active-expire 0 r debug set-active-expire 0
r del myhash r del myhash
...@@ -349,10 +346,12 @@ start_server {tags {"external:skip needs:debug"}} { ...@@ -349,10 +346,12 @@ start_server {tags {"external:skip needs:debug"}} {
test "Test HRANDFIELD deletes all expired fields ($type)" { test "Test HRANDFIELD deletes all expired fields ($type)" {
r debug set-active-expire 0 r debug set-active-expire 0
r flushall r flushall
r config resetstat
r hset myhash f1 v1 f2 v2 f3 v3 f4 v4 f5 v5 r hset myhash f1 v1 f2 v2 f3 v3 f4 v4 f5 v5
r hpexpire myhash 1 FIELDS 2 f1 f2 r hpexpire myhash 1 FIELDS 2 f1 f2
after 5 after 5
assert_equal [lsort [r hrandfield myhash 5]] "f3 f4 f5" assert_equal [lsort [r hrandfield myhash 5]] "f3 f4 f5"
assert_equal [s expired_subkeys] 2
r hpexpire myhash 1 FIELDS 3 f3 f4 f5 r hpexpire myhash 1 FIELDS 3 f3 f4 f5
after 5 after 5
assert_equal [lsort [r hrandfield myhash 5]] "" assert_equal [lsort [r hrandfield myhash 5]] ""
...@@ -476,39 +475,48 @@ start_server {tags {"external:skip needs:debug"}} { ...@@ -476,39 +475,48 @@ start_server {tags {"external:skip needs:debug"}} {
r hset h5 1 1 2 22 3 333 4 4444 5 55555 r hset h5 1 1 2 22 3 333 4 4444 5 55555
r hset h6 01 01 02 02 03 03 04 04 05 05 06 06 r hset h6 01 01 02 02 03 03 04 04 05 05 06 06
r hset h18 01 01 02 02 03 03 04 04 05 05 06 06 07 07 08 08 09 09 10 10 11 11 12 12 13 13 14 14 15 15 16 16 17 17 18 18 r hset h18 01 01 02 02 03 03 04 04 05 05 06 06 07 07 08 08 09 09 10 10 11 11 12 12 13 13 14 14 15 15 16 16 17 17 18 18
r hpexpire h1 100 NX FIELDS 1 01 r hpexpire h1 1 NX FIELDS 1 01
r hpexpire h2 100 NX FIELDS 1 01 r hpexpire h2 1 NX FIELDS 1 01
r hpexpire h2 100 NX FIELDS 1 02 r hpexpire h2 1 NX FIELDS 1 02
r hpexpire h3 100 NX FIELDS 1 01 r hpexpire h3 1 NX FIELDS 1 01
r hpexpire h4 100 NX FIELDS 1 2 r hpexpire h4 1 NX FIELDS 1 2
r hpexpire h5 100 NX FIELDS 1 3 r hpexpire h5 1 NX FIELDS 1 3
r hpexpire h6 100 NX FIELDS 1 05 r hpexpire h6 1 NX FIELDS 1 05
r hpexpire h18 100 NX FIELDS 17 01 02 03 04 05 06 07 08 09 10 11 12 13 14 15 16 17 r hpexpire h18 1 NX FIELDS 17 01 02 03 04 05 06 07 08 09 10 11 12 13 14 15 16 17
after 150 after 5
# Verify HDEL not ignore expired field. It is too much overhead to check # Verify HDEL not ignore expired field. It is too much overhead to check
# if the field is expired before deletion. # if the field is expired before deletion.
assert_equal [r HDEL h1 01] "1" assert_equal [r HDEL h1 01] "1"
# Verify HGET ignore expired field # Verify HGET ignore expired field
r config resetstat
assert_equal [r HGET h2 01] "" assert_equal [r HGET h2 01] ""
assert_equal [s expired_subkeys] 1
assert_equal [r HGET h2 02] "" assert_equal [r HGET h2 02] ""
assert_equal [s expired_subkeys] 2
assert_equal [r HGET h3 01] "" assert_equal [r HGET h3 01] ""
assert_equal [r HGET h3 02] "02" assert_equal [r HGET h3 02] "02"
assert_equal [r HGET h3 03] "03" assert_equal [r HGET h3 03] "03"
assert_equal [s expired_subkeys] 3
# Verify HINCRBY ignore expired field # Verify HINCRBY ignore expired field
assert_equal [r HINCRBY h4 2 1] "1" assert_equal [r HINCRBY h4 2 1] "1"
assert_equal [s expired_subkeys] 4
assert_equal [r HINCRBY h4 3 1] "100" assert_equal [r HINCRBY h4 3 1] "100"
# Verify HSTRLEN ignore expired field # Verify HSTRLEN ignore expired field
assert_equal [r HSTRLEN h5 3] "0" assert_equal [r HSTRLEN h5 3] "0"
assert_equal [s expired_subkeys] 5
assert_equal [r HSTRLEN h5 4] "4" assert_equal [r HSTRLEN h5 4] "4"
assert_equal [lsort [r HKEYS h6]] "01 02 03 04 06" assert_equal [lsort [r HKEYS h6]] "01 02 03 04 06"
assert_equal [s expired_subkeys] 5
# Verify HEXISTS ignore expired field # Verify HEXISTS ignore expired field
assert_equal [r HEXISTS h18 07] "0" assert_equal [r HEXISTS h18 07] "0"
assert_equal [s expired_subkeys] 6
assert_equal [r HEXISTS h18 18] "1" assert_equal [r HEXISTS h18 18] "1"
# Verify HVALS ignore expired field # Verify HVALS ignore expired field
assert_equal [lsort [r HVALS h18]] "18" assert_equal [lsort [r HVALS h18]] "18"
assert_equal [s expired_subkeys] 6
# Restore to support active expire # Restore to support active expire
r debug set-active-expire 1 r debug set-active-expire 1
} }
...@@ -898,14 +906,14 @@ start_server {tags {"external:skip needs:debug"}} { ...@@ -898,14 +906,14 @@ start_server {tags {"external:skip needs:debug"}} {
r hset myhash f1 v1 f2 v2 f3 v3 f4 v4 f5 v5 r hset myhash f1 v1 f2 v2 f3 v3 f4 v4 f5 v5
r hpexpire myhash 100 FIELDS 3 f1 f2 f3 r hpexpire myhash 100 FIELDS 3 f1 f2 f3
assert_match [get_hashes_with_expiry_fields r] 1 assert_match [get_stat_subexpiry r] 1
r hset myhash2 f1 v1 f2 v2 f3 v3 f4 v4 f5 v5 r hset myhash2 f1 v1 f2 v2 f3 v3 f4 v4 f5 v5
assert_match [get_hashes_with_expiry_fields r] 1 assert_match [get_stat_subexpiry r] 1
r hpexpire myhash2 100 FIELDS 3 f1 f2 f3 r hpexpire myhash2 100 FIELDS 3 f1 f2 f3
assert_match [get_hashes_with_expiry_fields r] 2 assert_match [get_stat_subexpiry r] 2
wait_for_condition 50 50 { wait_for_condition 50 50 {
[get_hashes_with_expiry_fields r] == 0 [get_stat_subexpiry r] == 0
} else { } else {
fail "Hash field expiry statistics failed" fail "Hash field expiry statistics failed"
} }
...@@ -1039,11 +1047,8 @@ start_server {tags {"external:skip needs:debug"}} { ...@@ -1039,11 +1047,8 @@ start_server {tags {"external:skip needs:debug"}} {
r hpexpire h1 100000 NX FIELDS 3 f3 f4 f5 r hpexpire h1 100000 NX FIELDS 3 f3 f4 f5
r hexpire h1 100000 FIELDS 1 f6 r hexpire h1 100000 FIELDS 1 f6
# Verify HRANDFIELD deletes expired fields and propagates it
r hset h2 f1 v1 f2 v2 r hset h2 f1 v1 f2 v2
r hpexpire h2 1 FIELDS 2 f1 f2 r hpexpire h2 1 FIELDS 2 f1 f2
after 5
assert_equal [r hrandfield h4 2] ""
after 200 after 200
assert_aof_content $aof { assert_aof_content $aof {
...@@ -1060,8 +1065,6 @@ start_server {tags {"external:skip needs:debug"}} { ...@@ -1060,8 +1065,6 @@ start_server {tags {"external:skip needs:debug"}} {
} }
array set keyAndFields1 [dumpAllHashes r] array set keyAndFields1 [dumpAllHashes r]
# Let some time pass and reload data from AOF
after 2000
r debug loadaof r debug loadaof
array set keyAndFields2 [dumpAllHashes r] array set keyAndFields2 [dumpAllHashes r]
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment