Forkless Snapshot (Threadsave)#3648
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## forkless-pre-threadsave #3648 +/- ##
===========================================================
+ Coverage 75.63% 77.32% +1.69%
===========================================================
Files 160 162 +2
Lines 80744 82386 +1642
===========================================================
+ Hits 61069 63707 +2638
+ Misses 19675 18679 -996
🚀 New features to boost your workflow:
|
a00a573 to
5482b96
Compare
|
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 |
1588575 to
79b3cc3
Compare
cca0dfe to
fa3301a
Compare
281d627 to
c0f533c
Compare
05a2f38 to
fa3301a
Compare
2634d89 to
c0f533c
Compare
00d57ed to
fb0c570
Compare
| # When disabled (default), automatic saves use fork. You can still trigger a | ||
| # thread-based snapshot explicitly with 'BGSAVE THREAD'. | ||
| # | ||
| # threadsave-enabled-for-backup no |
There was a problem hiding this comment.
For naming consistency, these should ALL be forkless or ALL be threadsave. Right now we're mixing terminology.
There was a problem hiding this comment.
forkless-options-supported is from the preceding bgIteration PRs - it's the umbrella infrastructure flag that enables per-key metadata allocation needed for the entire forkless family:
threadsave-to-disk (this PR), future threadsave-for-replication, and future forkless slot migration. This PR only references it as a prerequisite check. Everything introduced here uses "threadsave" (config, INFO fields, API, flags).
There was a problem hiding this comment.
I think it's ok to have a distinction between "forkless save" - a general strategy, and "Threadsave" - a specific implementation of that strategy. We might expect that someday, Threadsave could be replaced by a new-and-improved forkless implementation. In the code, I like calling it "Threadsave" - and there are contexts where talking about Threadsave as a "forkless save" makes sense.
However, I agree with Maddy that the external presentation should be consistent. I don't feel strongly regarding presentation as "forkless" or presentation as "threadsave". I can see reasons for each. The configs, and the bgsave option should match.
06a2531 to
1321109
Compare
Signed-off-by: Nitai Caro <caronita@amazon.com>
1321109 to
16bedb8
Compare
Thread-based RDB snapshot (Threadsave)
Motivation
Fork-based BGSAVE relies on
fork(), which on large datasets causes significant memory overhead from copy-on-write page duplication. On systems where memory is constrained, fork can fail entirely. Threadsave provides an alternative that avoids fork by performing the RDB snapshot on a background thread while the main thread continues serving clients.How it works
Threadsave builds on the bgIteration framework (introduced in #3553). bgIteration walks the keyspace on the main thread, delivering keys to a FIFO queue. A background thread consumes entries from the queue and serializes them to an RDB file using the standard
rdbSaveKeyValuePairencoding. The result is a normal RDB file - loadable by any Valkey instance.Usage
A thread-based snapshot can be triggered in three ways:
BGSAVE THREAD- explicit command (requiresforkless-options-supported yes)config set threadsave-enabled-for-backup yes- makes plainBGSAVEuse threadsavethreadsave-enabled-for-backupis enabledBGSAVE FORKexplicitly forces a fork-based snapshot regardless of configuration.BGSAVE SCHEDULE THREAD/BGSAVE SCHEDULE FORKschedule a snapshot with explicit type.