Fix resize hash tables stuck on the last non-empty slot (#12802)
Introduced in #11695 . The tryResizeHashTables function gets stuck on the last non-empty slot while iterating through dictionaries. It does not restart from the beginning. The reason for this issue is a problem with the usage of dbIteratorNextDict: /* Returns next dictionary from the iterator, or NULL if iteration is complete. */ dict *dbIteratorNextDict(dbIterator *dbit) { if (dbit->next_slot == -1) return NULL; dbit->slot = dbit->next_slot; dbit->next_slot = dbGetNextNonEmptySlot(dbit->db, dbit->slot, dbit->keyType); return dbGetDictFromIterator(dbit); } When iterating to the last non-empty slot, next_slot is set to -1, causing it to loop indefinitely on that slot. We need to modify the code to ensure that after iterating to the last non-empty slot, it returns to the first non-empty slot. BTW, function tryResizeHashTables is actually iterating over slots that have keys. However, in its implementation, it leverages the dbIterator (which is a key iterator) to obtain slot and dictionary information. While this approach works fine, but it is not very intuitive. This PR also improves readability by changing the iteration to directly iterate over slots, thereby enhancing clarity.
This commit is contained in:
parent
095d05786f
commit
a1c5171c1d
17
src/db.c
17
src/db.c
@ -114,20 +114,7 @@ dbIterator *dbIteratorInit(redisDb *db, dbKeyType keyType) {
|
||||
return dbit;
|
||||
}
|
||||
|
||||
/* Returns DB iterator that can be used to iterate through sub-dictionaries.
|
||||
*
|
||||
* The caller should free the resulting dbit with dbReleaseIterator. */
|
||||
dbIterator *dbIteratorInitFromSlot(redisDb *db, dbKeyType keyType, int slot) {
|
||||
dbIterator *dbit = zmalloc(sizeof(*dbit));
|
||||
dbit->db = db;
|
||||
dbit->slot = slot;
|
||||
dbit->keyType = keyType;
|
||||
dbit->next_slot = dbGetNextNonEmptySlot(dbit->db, dbit->slot, dbit->keyType);
|
||||
dictInitSafeIterator(&dbit->di, NULL);
|
||||
return dbit;
|
||||
}
|
||||
|
||||
/* Free the dbit returned by dbIteratorInit or dbIteratorInitFromSlot. */
|
||||
/* Free the dbit returned by dbIteratorInit. */
|
||||
void dbReleaseIterator(dbIterator *dbit) {
|
||||
dictIterator *iter = &dbit->di;
|
||||
dictResetIterator(iter);
|
||||
@ -687,7 +674,7 @@ long long emptyDbStructure(redisDb *dbarray, int dbnum, int async,
|
||||
dbarray[j].expires_cursor = 0;
|
||||
for (dbKeyType subdict = DB_MAIN; subdict <= DB_EXPIRES; subdict++) {
|
||||
dbarray[j].sub_dict[subdict].key_count = 0;
|
||||
dbarray[j].sub_dict[subdict].resize_cursor = 0;
|
||||
dbarray[j].sub_dict[subdict].resize_cursor = -1;
|
||||
if (server.cluster_enabled) {
|
||||
if (dbarray[j].sub_dict[subdict].rehashing)
|
||||
listEmpty(dbarray[j].sub_dict[subdict].rehashing);
|
||||
|
21
src/server.c
21
src/server.c
@ -660,20 +660,19 @@ int htNeedsResize(dict *dict) {
|
||||
* In non cluster-enabled setup, it resize main/expires dictionary based on the same condition described above. */
|
||||
void tryResizeHashTables(int dbid) {
|
||||
redisDb *db = &server.db[dbid];
|
||||
int slot = 0;
|
||||
for (dbKeyType subdict = DB_MAIN; subdict <= DB_EXPIRES; subdict++) {
|
||||
dbIterator *dbit = dbIteratorInitFromSlot(db, subdict, db->sub_dict[subdict].resize_cursor);
|
||||
for (int i = 0; i < CRON_DBS_PER_CALL; i++) {
|
||||
dict *d = dbGetDictFromIterator(dbit);
|
||||
slot = dbIteratorGetCurrentSlot(dbit);
|
||||
dbIteratorNextDict(dbit);
|
||||
if (!d) break;
|
||||
if (dbSize(db, subdict) == 0) continue;
|
||||
|
||||
if (db->sub_dict[subdict].resize_cursor == -1)
|
||||
db->sub_dict[subdict].resize_cursor = findSlotByKeyIndex(db, 1, subdict);
|
||||
|
||||
for (int i = 0; i < CRON_DBS_PER_CALL && db->sub_dict[subdict].resize_cursor != -1; i++) {
|
||||
int slot = db->sub_dict[subdict].resize_cursor;
|
||||
dict *d = (subdict == DB_MAIN ? db->dict[slot] : db->expires[slot]);
|
||||
if (htNeedsResize(d))
|
||||
dictResize(d);
|
||||
db->sub_dict[subdict].resize_cursor = dbGetNextNonEmptySlot(db, slot, subdict);
|
||||
}
|
||||
/* Save current iterator position in the resize_cursor. */
|
||||
db->sub_dict[subdict].resize_cursor = slot;
|
||||
dbReleaseIterator(dbit);
|
||||
}
|
||||
}
|
||||
|
||||
@ -2656,7 +2655,7 @@ void initDbState(redisDb *db){
|
||||
for (dbKeyType subdict = DB_MAIN; subdict <= DB_EXPIRES; subdict++) {
|
||||
db->sub_dict[subdict].rehashing = listCreate();
|
||||
db->sub_dict[subdict].key_count = 0;
|
||||
db->sub_dict[subdict].resize_cursor = 0;
|
||||
db->sub_dict[subdict].resize_cursor = -1;
|
||||
db->sub_dict[subdict].slot_size_index = server.cluster_enabled ? zcalloc(sizeof(unsigned long long) * (CLUSTER_SLOTS + 1)) : NULL;
|
||||
db->sub_dict[subdict].bucket_count = 0;
|
||||
}
|
||||
|
@ -2436,7 +2436,6 @@ typedef struct dbIterator dbIterator;
|
||||
|
||||
/* DB iterator specific functions */
|
||||
dbIterator *dbIteratorInit(redisDb *db, dbKeyType keyType);
|
||||
dbIterator *dbIteratorInitFromSlot(redisDb *db, dbKeyType keyType, int slot);
|
||||
void dbReleaseIterator(dbIterator *dbit);
|
||||
dict *dbIteratorNextDict(dbIterator *dbit);
|
||||
dict *dbGetDictFromIterator(dbIterator *dbit);
|
||||
|
@ -426,3 +426,49 @@ start_server {tags {"other external:skip"}} {
|
||||
}
|
||||
}
|
||||
|
||||
start_cluster 1 0 {tags {"other external:skip cluster slow"}} {
|
||||
test "Redis can trigger resizing" {
|
||||
r flushall
|
||||
# hashslot(foo) is 12182
|
||||
for {set j 1} {$j <= 128} {incr j} {
|
||||
r set "{foo}$j" a
|
||||
}
|
||||
assert_match "*table size: 128*" [r debug HTSTATS 0]
|
||||
|
||||
# disable resizing
|
||||
r config set rdb-key-save-delay 10000000
|
||||
r bgsave
|
||||
|
||||
# delete data to have lot's (99%) of empty buckets
|
||||
for {set j 1} {$j <= 127} {incr j} {
|
||||
r del "{foo}$j"
|
||||
}
|
||||
assert_match "*table size: 128*" [r debug HTSTATS 0]
|
||||
|
||||
# enable resizing
|
||||
r config set rdb-key-save-delay 0
|
||||
catch {exec kill -9 [get_child_pid 0]}
|
||||
wait_for_condition 1000 10 {
|
||||
[s rdb_bgsave_in_progress] eq 0
|
||||
} else {
|
||||
fail "bgsave did not stop in time."
|
||||
}
|
||||
|
||||
after 200;# waiting for serverCron
|
||||
assert_match "*table size: 4*" [r debug HTSTATS 0]
|
||||
} {} {needs:debug}
|
||||
|
||||
test "Redis can rewind and trigger smaller slot resizing" {
|
||||
# hashslot(alice) is 749, smaller than hashslot(foo),
|
||||
# attempt to trigger a resize on it, see details in #12802.
|
||||
for {set j 1} {$j <= 128} {incr j} {
|
||||
r set "{alice}$j" a
|
||||
}
|
||||
for {set j 1} {$j <= 127} {incr j} {
|
||||
r del "{alice}$j"
|
||||
}
|
||||
|
||||
after 200;# waiting for serverCron
|
||||
assert_match "*table size: 8*" [r debug HTSTATS 0]
|
||||
} {} {needs:debug}
|
||||
}
|
||||
|
Loading…
x
Reference in New Issue
Block a user