feat: feat(command): add FLUSHWAL to flush and fsync the RocksDB WAL - #3603
Closed
pipinstall1 wants to merge 1 commit into
Closed
feat: feat(command): add FLUSHWAL to flush and fsync the RocksDB WAL#3603pipinstall1 wants to merge 1 commit into
pipinstall1 wants to merge 1 commit into
Conversation
Expose rocksdb::DB::FlushWAL(/*sync=*/true) as a FLUSHWAL command. With rocksdb.write_options.sync=no, writes are appended to the WAL buffer per command but only made durable by background flushes; nothing in the command surface lets a client force that boundary. FLUSHWAL flushes the WAL buffer to the file and fsyncs it, making every write accepted before the call durable across a process or machine crash. Motivation: group-commit durability. An application tier running with sync=no can append fast, hold acks, and issue one FLUSHWAL per T ms (or per N writes), then release every ack accepted before the flush - ack=fsynced semantics at a fraction of the per-write fsync cost. On a disk with an ~12 ms fsync floor, a Lua append script doing 4 writes per call moves from ~20 appends/s (sync=yes, one fsync per write batch) to ~2,400 appends/s with a 5 ms flush window, with the loss window on power failure bounded by the flush interval (process crash alone loses nothing: the WAL is still written per command). The command rejects under rocksdb.write_options.disable_wal (there is no WAL to flush) and surfaces any RocksDB error. It performs no keyspace write and needs no exclusivity; registered read-only admin to match the other storage-maintenance commands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Hi @pipinstall1, Thank you for your pull request. Please review our Contributing Guide. Please make sure you understand your changes and explain your reasoning in this pull request. Low-quality pull requests may be closed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3602
Repo:
apache/kvrocks· Branch:pr/flushwal-command(in/home/dev/work/clipstream/kvrocks-upstream) · Base:unstable(1418e23d) · Files:src/commands/cmd_server.cc(+21)Motivation
With
rocksdb.write_options.sync yes, every command's write batch is fsynced individually. On a disk with an ~12 ms fsync floor (measured: 4 KiB rewrite, n=60, mean 12.0 ms, p50 11.0 ms), that caps a single connection at ~85 writes/s regardless of pipelining, and a Lua script issuing 4 writes per call at ~20 calls/s. Concurrency does not help because eachredis.callfsyncs on its own:sync yessync yessync nosync noWith
sync no, kvrocks leaves RocksDB'smanual_wal_flushat its default (false), so every WAL record iswrite()n before the command replies (akill -9loses nothing: 2000/2000 keys present after restart, with or without a flush). The exposure is power loss / kernel panic, and it is unbounded in time: anstrace -yover a 12 s append run counted 21,496 acked appends behind exactly onefdatasyncof the WAL (the creation sync at open); the next data sync is whenever the memtable flushes.RocksDB already has the primitive that bounds this,
DB::FlushWAL(sync=true), and kvrocks links it, but nothing in the command surface exposes it. This PR addsFLUSHWAL, which flushes the WAL buffer to the file and fsyncs it, making every write accepted before the call durable.That enables group-commit durability from the client side: run
sync no, append fast, and issue oneFLUSHWALper T ms (or per N writes), either holding acks until the flush returns (ack = fsynced) or accepting a bounded T-ms loss window on power loss. Measured with the same 4-write Lua append and one flusher process on the same disk (35 s windows, 4 client processes):Sustained 120 s at 200 writers / T=5 ms: 2470 appends/s at ack p99 189 ms with every ack behind an fsync, versus 21.6 appends/s at
sync yes: a ~114x throughput difference at the same durability guarantee. Thestracecount confirms coverage 1:1 (5FLUSHWAL-> 5fdatasync; 200SETs with noFLUSHWAL-> no sync syscalls).Change
srv->storage->GetDB()->FlushWAL(/*sync=*/true); replies+OK, or the RocksDB status string as an error.flushwal is meaningless with the disable_wal optionwhenrocksdb.write_options.disable_walis set (same pattern as the existingdisable_walcheck in this file).INFO("FLUSHWAL is triggered and executed successfully"), matchingFLUSHMEMTABLE/FLUSHBLOCKCACHE.MakeCmdAttr<CommandFlushWAL>("flushwal", 1, "read-only admin", NO_KEY): it performs no keyspace write and needs no exclusivity (it is safe to run concurrently with writers; that is its purpose).adminfollows fix(command): require admin permission for privileged commands #3570's convention for privileged storage-maintenance commands. Reviewer input welcome on whether it should also beno-script; the use case above benefits from being callable from a Lua append script (throttled by a timestamp key) to avoid a separate flusher process, and the command is deterministic from the script's point of view (no keyspace effect,+OK).The command class sits next to
CommandFlushBlockCacheinsrc/commands/cmd_server.cc.Testing
unstable(cmake -DDISABLE_JEMALLOC=ON, gcc 13.3, RelWithDebInfo); see the build note inINDEX.mdfor what was and was not run on this box.sync no): correctness cases above (2000 keys ->FLUSHWAL->kill -9-> 2000/2000;FLUSHWALreturns in 11-24 ms, i.e. a real fsync), syscall accounting viastrace, and the throughput/latency matrix.tests/gocase/unit/would be the natural addition (FLUSHWAL->OK;FLUSHWALunderdisable_wal-> error). Not included in this branch yet; happy to add it if the command shape is accepted.Notes
feat(command): ..., validated bypr-lint), single commit, no CLA required beyond the ASF ICLA where applicable. The repository's issue templates ask for an enhancement issue first ("Search before asking" / "willing to submit a PR"); the operator may want to open one and reference it from the PR. The commit carries aCo-Authored-Byline disclosing AI assistance per the project's AI-assisted contribution guidelines.