From 676f27acb0e2a61e96778bccfa1116e3f7eedaaf Mon Sep 17 00:00:00 2001 From: "debing.sun" Date: Sun, 11 Feb 2024 21:12:42 +0800 Subject: [PATCH] Fix the failure of defrag test under 32-bit (#13013) Fail CI: https://github.com/redis/redis/actions/runs/7837608438/job/21387609715 ## Why defragment tests only failed under 32-bit First of all, under 32-bit jemalloc will allocate more small bins and less large bins, which will also lead to more external fragmentation, therefore, the fragmentation ratio is higher in 32-bit than in 64-bit, so the defragment tests(`Active defrag eval scripts: cluster` and `Active defrag big keys: cluster`) always fails in 32-bit. ## Why defragment tests only failed with cluster The fowllowing is the result of `Active defrag eval scripts: cluster` test. 1) Before #11695, the fragmentation ratio is 3.11%. 2) After #11695, the fragmentation ratio grew to 4.58%. Since we are using per-slot dictionary to manage slots, we will only defragment the contents of these dictionaries (keys, values), but not the dictionaries' struct and ht_table, which means that frequent shrinking and expanding of the dictionaries, will make more fragments. 3) After #12850 and #12948, In cluster mode, a large number of cluster slot dicts will be shrunk, creating additional fragmention, and the dictionary will not be defragged. ## Solution * Add defragmentation of the per-slot dictionary's own structures, dict struct and ht_table. ## Other change * Increase floating point print precision of `frags` and `rss` in debug logs for defrag --------- Co-authored-by: Oran Agra --- src/defrag.c | 17 ++++++++++++++++- src/kvstore.c | 19 +++++++++++++++++++ src/kvstore.h | 2 ++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/defrag.c b/src/defrag.c index 7c6c06ced..61d9c6a04 100644 --- a/src/defrag.c +++ b/src/defrag.c @@ -150,6 +150,7 @@ luaScript *activeDefragLuaScript(luaScript *script) { void dictDefragTables(dict* d) { dictEntry **newtable; /* handle the first hash table */ + if (!d->ht_table[0]) return; /* created but unused */ newtable = activeDefragAlloc(d->ht_table[0]); if (newtable) d->ht_table[0] = newtable; @@ -760,6 +761,18 @@ void defragScanCallback(void *privdata, const dictEntry *de) { server.stat_active_defrag_scanned++; } +static void defragKvstoreDefragScanCallBack(dict **d) { + dict *newd; + /* handle the dict struct */ + if ((newd = activeDefragAlloc(*d))) + *d = newd; + dictDefragTables(*d); +} + +void activeDefragKvstore(kvstore *kvs) { + kvstoreDictLUTDefrag(kvs, defragKvstoreDefragScanCallBack); +} + /* Utility function to get the fragmentation ratio from jemalloc. * It is critical to do that by comparing only heap maps that belong to * jemalloc, and skip ones the jemalloc keeps as spare. Since we use this @@ -776,7 +789,7 @@ float getAllocatorFragmentation(size_t *out_frag_bytes) { if(out_frag_bytes) *out_frag_bytes = frag_bytes; serverLog(LL_DEBUG, - "allocated=%zu, active=%zu, resident=%zu, frag=%.0f%% (%.0f%% rss), frag_bytes=%zu (%zu rss)", + "allocated=%zu, active=%zu, resident=%zu, frag=%.2f%% (%.2f%% rss), frag_bytes=%zu (%zu rss)", allocated, active, resident, frag_pct, rss_pct, frag_bytes, rss_bytes); return frag_pct; } @@ -1032,6 +1045,8 @@ void activeDefragCycle(void) { } db = &server.db[current_db]; + activeDefragKvstore(db->keys); + activeDefragKvstore(db->expires); cursor = 0; expires_cursor = 0; slot = kvstoreFindDictIndexByKeyIndex(db->keys, 1); diff --git a/src/kvstore.c b/src/kvstore.c index 1daf8db92..16d34acbd 100644 --- a/src/kvstore.c +++ b/src/kvstore.c @@ -92,6 +92,10 @@ static dict *kvstoreGetDict(kvstore *kvs, int didx) { return kvs->dicts[didx]; } +static dict **kvstoreGetDictRef(kvstore *kvs, int didx) { + return &kvs->dicts[didx]; +} + /* Returns total (cumulative) number of keys up until given dict-index (inclusive). * Time complexity is O(log(kvs->num_dicts)). */ static unsigned long long cumulativeKeyCountRead(kvstore *kvs, int didx) { @@ -714,6 +718,21 @@ unsigned long kvstoreDictScanDefrag(kvstore *kvs, int didx, unsigned long v, dic return dictScanDefrag(d, v, fn, defragfns, privdata); } +/* Unlike kvstoreDictScanDefrag(), this method doesn't defrag the data(keys and values) + * within dict, it only reallocates the memory used by the dict structure itself using + * the provided allocation function. This feature was added for the active defrag feature. + * + * The 'defragfn' callback is called with a reference to the dict + * that callback can reallocate. */ +void kvstoreDictLUTDefrag(kvstore *kvs, kvstoreDictLUTDefragFunction *defragfn) { + for (int didx = 0; didx < kvs->num_dicts; didx++) { + dict **d = kvstoreGetDictRef(kvs, didx); + if (!*d) + continue; + defragfn(d); + } +} + uint64_t kvstoreGetHash(kvstore *kvs, const void *key) { return kvs->dtype.hashFunction(key); diff --git a/src/kvstore.h b/src/kvstore.h index d22c1f92e..71abc87ad 100644 --- a/src/kvstore.h +++ b/src/kvstore.h @@ -56,6 +56,8 @@ dictEntry *kvstoreDictFindEntryByPtrAndHash(kvstore *kvs, int didx, const void * unsigned int kvstoreDictGetSomeKeys(kvstore *kvs, int didx, dictEntry **des, unsigned int count); int kvstoreDictExpand(kvstore *kvs, int didx, unsigned long size); unsigned long kvstoreDictScanDefrag(kvstore *kvs, int didx, unsigned long v, dictScanFunction *fn, dictDefragFunctions *defragfns, void *privdata); +typedef void (kvstoreDictLUTDefragFunction)(dict **d); +void kvstoreDictLUTDefrag(kvstore *kvs, kvstoreDictLUTDefragFunction *defragfn); void *kvstoreDictFetchValue(kvstore *kvs, int didx, const void *key); dictEntry *kvstoreDictFind(kvstore *kvs, int didx, void *key); dictEntry *kvstoreDictAddRaw(kvstore *kvs, int didx, void *key, dictEntry **existing);