From 62419c01db4499e52ddcb689726cfa732a9736a6 Mon Sep 17 00:00:00 2001 From: Binbin Date: Sun, 10 Dec 2023 23:30:20 +0800 Subject: [PATCH] Handle missing fields in dbSwapDatabases and swapMainDbWithTempDb (#12763) The change in dbSwapDatabases seems harmless. Because in non-clustered mode, dbBuckets calculations are strictly accurate and in cluster mode, we only have one DB. Modify it for uniformity (just like resize_cursor). The change in swapMainDbWithTempDb is needed in case we swap with the temp db, otherwise the overhead memory usage of db can be miscalculated. In addition we will swap all fields (including rehashing list), just for completeness (and reduce the chance of surprises in the future). Introduced in #12697. --- src/db.c | 14 ++++++++++++-- src/server.c | 1 + src/server.h | 1 + 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/db.c b/src/db.c index 0059af78a..5ab8d32c1 100644 --- a/src/db.c +++ b/src/db.c @@ -757,6 +757,7 @@ redisDb *initTempDb(void) { tempDb[i].dict = dictCreateMultiple(&dbDictType, tempDb[i].dict_count); tempDb[i].expires = dictCreateMultiple(&dbExpiresDictType, tempDb[i].dict_count); for (dbKeyType subdict = DB_MAIN; subdict <= DB_EXPIRES; subdict++) { + tempDb[i].sub_dict[subdict].rehashing = listCreate(); tempDb[i].sub_dict[subdict].slot_size_index = server.cluster_enabled ? zcalloc(sizeof(unsigned long long) * (CLUSTER_SLOTS + 1)) : NULL; } } @@ -777,8 +778,9 @@ void discardTempDb(redisDb *tempDb, void(callback)(dict*)) { } zfree(tempDb[i].dict); zfree(tempDb[i].expires); - if (server.cluster_enabled) { - for (dbKeyType subdict = DB_MAIN; subdict <= DB_EXPIRES; subdict++) { + for (dbKeyType subdict = DB_MAIN; subdict <= DB_EXPIRES; subdict++) { + listRelease(tempDb[i].sub_dict[subdict].rehashing); + if (server.cluster_enabled) { zfree(tempDb[i].sub_dict[subdict].slot_size_index); } } @@ -1888,7 +1890,9 @@ int dbSwapDatabases(int id1, int id2) { db1->expires_cursor = db2->expires_cursor; db1->dict_count = db2->dict_count; for (dbKeyType subdict = DB_MAIN; subdict <= DB_EXPIRES; subdict++) { + db1->sub_dict[subdict].rehashing = db2->sub_dict[subdict].rehashing; db1->sub_dict[subdict].key_count = db2->sub_dict[subdict].key_count; + db1->sub_dict[subdict].bucket_count = db2->sub_dict[subdict].bucket_count; db1->sub_dict[subdict].non_empty_slots = db2->sub_dict[subdict].non_empty_slots; db1->sub_dict[subdict].resize_cursor = db2->sub_dict[subdict].resize_cursor; db1->sub_dict[subdict].slot_size_index = db2->sub_dict[subdict].slot_size_index; @@ -1900,7 +1904,9 @@ int dbSwapDatabases(int id1, int id2) { db2->expires_cursor = aux.expires_cursor; db2->dict_count = aux.dict_count; for (dbKeyType subdict = DB_MAIN; subdict <= DB_EXPIRES; subdict++) { + db2->sub_dict[subdict].rehashing = aux.sub_dict[subdict].rehashing; db2->sub_dict[subdict].key_count = aux.sub_dict[subdict].key_count; + db2->sub_dict[subdict].bucket_count = aux.sub_dict[subdict].bucket_count; db2->sub_dict[subdict].non_empty_slots = aux.sub_dict[subdict].non_empty_slots; db2->sub_dict[subdict].resize_cursor = aux.sub_dict[subdict].resize_cursor; db2->sub_dict[subdict].slot_size_index = aux.sub_dict[subdict].slot_size_index; @@ -1944,7 +1950,9 @@ void swapMainDbWithTempDb(redisDb *tempDb) { activedb->expires_cursor = newdb->expires_cursor; activedb->dict_count = newdb->dict_count; for (dbKeyType subdict = DB_MAIN; subdict <= DB_EXPIRES; subdict++) { + activedb->sub_dict[subdict].rehashing = newdb->sub_dict[subdict].rehashing; activedb->sub_dict[subdict].key_count = newdb->sub_dict[subdict].key_count; + activedb->sub_dict[subdict].bucket_count = newdb->sub_dict[subdict].bucket_count; activedb->sub_dict[subdict].non_empty_slots = newdb->sub_dict[subdict].non_empty_slots; activedb->sub_dict[subdict].resize_cursor = newdb->sub_dict[subdict].resize_cursor; activedb->sub_dict[subdict].slot_size_index = newdb->sub_dict[subdict].slot_size_index; @@ -1956,7 +1964,9 @@ void swapMainDbWithTempDb(redisDb *tempDb) { newdb->expires_cursor = aux.expires_cursor; newdb->dict_count = aux.dict_count; for (dbKeyType subdict = DB_MAIN; subdict <= DB_EXPIRES; subdict++) { + newdb->sub_dict[subdict].rehashing = aux.sub_dict[subdict].rehashing; newdb->sub_dict[subdict].key_count = aux.sub_dict[subdict].key_count; + newdb->sub_dict[subdict].bucket_count = aux.sub_dict[subdict].bucket_count; newdb->sub_dict[subdict].non_empty_slots = aux.sub_dict[subdict].non_empty_slots; newdb->sub_dict[subdict].resize_cursor = aux.sub_dict[subdict].resize_cursor; newdb->sub_dict[subdict].slot_size_index = aux.sub_dict[subdict].slot_size_index; diff --git a/src/server.c b/src/server.c index 7bc0ffa51..9205cf6d4 100644 --- a/src/server.c +++ b/src/server.c @@ -2651,6 +2651,7 @@ void makeThreadKillable(void) { pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL); } +/* When adding fields, please check the initTempDb related logic. */ void initDbState(redisDb *db){ for (dbKeyType subdict = DB_MAIN; subdict <= DB_EXPIRES; subdict++) { db->sub_dict[subdict].rehashing = listCreate(); diff --git a/src/server.h b/src/server.h index ee408fb24..77ebb0f5b 100644 --- a/src/server.h +++ b/src/server.h @@ -969,6 +969,7 @@ typedef struct replBufBlock { char buf[]; } replBufBlock; +/* When adding fields, please check the swap db related logic. */ typedef struct dbDictState { list *rehashing; /* List of dictionaries in this DB that are currently rehashing. */ int resize_cursor; /* Cron job uses this cursor to gradually resize dictionaries (only used for cluster-enabled). */