Set cur_bgsave_type on the diskless replication path - #4482
Conversation
The diskless full sync path set rdb_write_target but never set cur_bgsave_type, so while a socket-target RDB child was running INFO reported rdb_bgsave_in_progress:1 together with rdb_current_bgsave_type: none. A diskless full sync still forks a child, so report it as a fork save. It is cleared on completion by rdbClearSaveState() as before. Signed-off-by: Nitai Caro <caronita@amazon.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forkless #4482 +/- ##
============================================
- Coverage 79.82% 79.45% -0.38%
============================================
Files 175 175
Lines 94843 93223 -1620
============================================
- Hits 75712 74070 -1642
- Misses 19131 19153 +22
🚀 New features to boost your workflow:
|
xhon-pelushi
left a comment
There was a problem hiding this comment.
I verified the diskless replication save state and this fixes the observable INFO inconsistency.
Using a local primary/replica pair with repl-diskless-sync yes, repl-diskless-sync-delay 0, and 500k debug-populated keys, the parent commit 50ec936 reported rdb_bgsave_in_progress:1 together with rdb_current_bgsave_type:none while the socket-target RDB child was active. At head 7af928c, the same diskless full-sync window reports rdb_current_bgsave_type:fork, and after the transfer completes it returns to none.
I traced the path from startBgsaveForReplication() into rdbSaveToReplicasSockets(), then through backgroundSaveDoneHandler() and rdbClearSaveState(). The added assignment matches the disk-target rdbRecordStartMetrics(RDB_BGSAVE_TYPE_FORK) state, and the shared cleanup still clears both rdb_write_target and cur_bgsave_type after socket transfer completion.
Local verification: make -j$(nproc) and ./runtest --single integration/replication --verbose --dump-logs (72 passed, 0 failed). I also checked the red test-ubuntu-latest-cmake-tls CI job log; it failed in tests/unit/cluster/replica-priority.tcl on a missing Start of election...rank #1...replica priority 10 log pattern, not in the RDB/diskless replication path touched here.
The diskless full sync path set
rdb_write_targetbut never setcur_bgsave_type.