Block ValkeyModule_Fork while a save is in progress - #4478
Conversation
A forkless save exists to avoid the copy-on-write memory spike of a fork-based save. But the save still writes to the pages of the data it walks (object refcounts, the iterator epoch in object metadata, and rehash state in collections), and the main thread keeps serving writes. If a module fork child is alive during the save, all of those writes trigger copy-on-write page copies. That brings back the exact memory spike forkless was made to avoid. Before forkless a module could not fork during a save anyway: a save was a child process, and serverFork() allows only one child at a time. A forkless save is a background thread, so that check no longer sees it and VM_Fork() slips through. Reject VM_Fork() while any save is in progress to restore the old behavior. 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❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## forkless #4478 +/- ##
============================================
- Coverage 79.69% 79.43% -0.26%
============================================
Files 175 175
Lines 94841 93225 -1616
============================================
- Hits 75581 74054 -1527
+ Misses 19260 19171 -89
🚀 New features to boost your workflow:
|
1 participant
A forkless save exists to avoid the CoW memory spike of a fork-based save. But the save still writes to the pages of the data it walks (object refcounts, the iterator epoch in object metadata, and rehash state in collections), and the main thread keeps serving writes. So if a fork child is alive during the save, all of those writes trigger CoW.
Before forkless a module could not fork during a save anyway: a save was a child process, and
serverFork()allows only one child at a time. Now a forkless save is a background thread, so that check no longer sees it andVM_Fork()slips through. RejectVM_Fork()while any save is in progress to restore the old behavior.