Skip to content

Commit

Permalink
Optimize RDB load performance and fix cluster mode resizing on replic…
Browse files Browse the repository at this point in the history
…a side (#1199)

This PR addresses two issues:

1. Performance Degradation Fix - Resolves a significant performance
issue during RDB load on replica nodes.
- The problem was causing replicas to rehash multiple times during the
load process. Local testing demonstrated up to 50% degradation in BGSAVE
time.
- The problem occurs when the replica tries to expand pre-created slot
dictionaries. This operation fails quietly, resulting in undetected
performance issues.
- This fix aims to optimize the RDB load process and restore expected
performance levels.

2. Bug fix when reading `RDB_OPCODE_RESIZEDB` in Valkey 8.0 cluster
mode-
- Use the shard's master slots count when processing this opcode, as
`clusterNodeCoversSlot` is not initialized for the currently syncing
replica.
- Previously, this problem went unnoticed because `RDB_OPCODE_RESIZEDB`
had no practical impact (due to 1).

These improvements will enhance overall system performance and ensure
smoother upgrades to Valkey 8.0 in the future.

Testing:
- Conducted local tests to verify the performance improvement during RDB
load.
- Verified that ignoring `RDB_OPCODE_RESIZEDB` does not negatively
impact functionality in the current version.

Signed-off-by: naglera <[email protected]>
Co-authored-by: Binbin <[email protected]>
  • Loading branch information
naglera and enjoy-binbin authored Nov 18, 2024
1 parent d07674f commit c5012cc
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/db.c
Original file line number Diff line number Diff line change
Expand Up @@ -1884,7 +1884,7 @@ keyStatus expireIfNeeded(serverDb *db, robj *key, int flags) {
* The purpose is to skip expansion of unused dicts in cluster mode (all
* dicts not mapped to *my* slots) */
static int dbExpandSkipSlot(int slot) {
return !clusterNodeCoversSlot(getMyClusterNode(), slot);
return !clusterNodeCoversSlot(clusterNodeGetPrimary(getMyClusterNode()), slot);
}

/*
Expand Down
6 changes: 4 additions & 2 deletions src/kvstore.c
Original file line number Diff line number Diff line change
Expand Up @@ -423,9 +423,11 @@ unsigned long long kvstoreScan(kvstore *kvs,
* `dictTryExpand` call and in case of `dictExpand` call it signifies no expansion was performed.
*/
int kvstoreExpand(kvstore *kvs, uint64_t newsize, int try_expand, kvstoreExpandShouldSkipDictIndex *skip_cb) {
if (newsize == 0) return 1;
for (int i = 0; i < kvs->num_dicts; i++) {
dict *d = kvstoreGetDict(kvs, i);
if (!d || (skip_cb && skip_cb(i))) continue;
if (skip_cb && skip_cb(i)) continue;
/* If the dictionary doesn't exist, create it */
dict *d = createDictIfNeeded(kvs, i);
int result = try_expand ? dictTryExpand(d, newsize) : dictExpand(d, newsize);
if (try_expand && result == DICT_ERR) return 0;
}
Expand Down

0 comments on commit c5012cc

Please sign in to comment.