Write-behind log for async AOF-based durability - #3381
Conversation
Move the expensive AOF write+fsync off the main thread when IO threads are available. This prevents the main thread from blocking on disk I/O when appendfsync is set to 'always'. Add a generic trySendJobToIOThreads() API to io_threads with round-robin distribution, and an aof IO flush state machine (IDLE/PENDING/DONE/ERR) with atomic coordination between main and IO threads. The adjustIOThreadsByEventLoad() function gains a has_background_work parameter to ensure IO threads stay active when AOF fsync work is pending, even during low-traffic periods. Signed-off-by: jjuleslasarte <jules.lasarte@gmail.com> Signed-off-by: Jules Lasarte <jules.lasarte@gmail.com>
Introduce a provider registry that allows multiple durability backends (AOF fsync, replicas, etc.) to register and contribute to a consensus offset. The overall durability consensus is the MIN (AND) of all enabled providers' acknowledged offsets. Include the built-in AOF provider that tracks fsynced_reploff_pending when appendfsync=always, and transparently passes through when not. Add pause/resume support for providers (used via DEBUG commands) to enable deterministic testing by freezing a provider's acknowledged offset at a point-in-time snapshot. Signed-off-by: jjuleslasarte <jules.lasarte@gmail.com> Signed-off-by: Jules Lasarte <jules.lasarte@gmail.com>
Add a task registry that defers side-effects (keyspace notifications, key invalidations, flush invalidations) until durability providers acknowledge the associated write offset. Each task type registers create/destroy/execute/onClientDestroy handlers. Tasks are created during command execution with a deferred offset, then moved to an official waiting list once the replication offset is known. When the consensus offset advances past a task's offset, the task is executed and freed. Key invalidation tasks track the originating client pointer and properly handle client disconnection before task execution. Signed-off-by: jjuleslasarte <jules.lasarte@gmail.com> Signed-off-by: Jules Lasarte <jules.lasarte@gmail.com>
Track which keys have been modified but not yet acknowledged by durability providers using a per-database hashtable. This enables rejecting reads of uncommitted keys to ensure clients only see durable data (zero-data-loss semantics). Each uncommitted key stores the replication offset at which it was last modified. Keys are purged when the durability consensus offset advances past their stored offset. Include incremental cleanup via serverCron that scans databases round-robin with a configurable time limit, plus immediate purging on read access (lazy cleanup). Also handle database-level modifications (FLUSHDB, FLUSHALL, SWAPDB) and function store dirty tracking for transactions. Signed-off-by: jjuleslasarte <jules.lasarte@gmail.com> Signed-off-by: Jules Lasarte <jules.lasarte@gmail.com>
Add the core orchestration layer that blocks client responses in the client output buffer (COB) until durability providers confirm the write offset, then unblocks and flushes responses to clients. reply_blocking.c/h contains: - durabilityInit/Cleanup/Reset lifecycle management - beforeCommandTrackReplOffset/afterCommandTrackReplOffset for tracking which replication offsets each command produces - preCommandExec: rejects commands accessing uncommitted keys - postCommandExec: blocks client responses until providers acknowledge - notifyDurabilityProgress: called from beforeSleep to unblock clients whose offsets have been acknowledged - blockClientOnReplOffset/unblockResponsesWithAckOffset - Function store dirty tracking for FUNCTION LOAD/DELETE - INFO durability stats generation Integration points across the server: - server.c: init/cleanup in server lifecycle, pre/post command hooks in call() and processCommand(), notifyDurabilityProgress in beforeSleep, uncommitted keys cleanup in serverCron, per-DB init, INFO section - server.h: durable_t in server struct, clientDurabilityInfo in client, uncommitted_keys/dirty_repl_offset in serverDb, new client flag - config.c: 'durability' bool config with dynamic update callback - db.c: durabilitySignalModifiedKey/durabilitySignalFlushedDb hooks - networking.c: client durability init/reset, COB reply limiting - notify.c: defer keyspace notifications when durability is enabled - script.c/module.c: pre-script checks for uncommitted data access - replication.c: clear durability state on primary change - debug.c: durability-provider-pause/resume DEBUG subcommands - object.c: getIntFromObject utility Signed-off-by: jjuleslasarte <jules.lasarte@gmail.com> Signed-off-by: Jules Lasarte <jules.lasarte@gmail.com>
Add reply_blocking.c, durable_task.c, durability_provider.c, and uncommitted_keys.c to the build system (both Makefile and CMake). Also fix a clang compatibility issue in unit test CMakeLists.txt: -fno-var-tracking-assignments is GCC-only, so guard it with a compiler ID check. Signed-off-by: jjuleslasarte <jules.lasarte@gmail.com> Signed-off-by: Jules Lasarte <jules.lasarte@gmail.com>
Add comprehensive gtest-based unit tests covering the reply blocking subsystem including: - Client output buffer blocking and unblocking mechanics - Offset tracking through command execution - Multi-command transaction (MULTI/EXEC) offset handling - Durability provider consensus calculations - Deferred task lifecycle (create, execute, cleanup) - Uncommitted key tracking and purging - Edge cases: client disconnection, provider pause/resume Signed-off-by: jjuleslasarte <jules.lasarte@gmail.com> Signed-off-by: Jules Lasarte <jules.lasarte@gmail.com>
Add Tcl-based integration tests (1,051 lines) covering end-to-end durability behavior including: - AOF-based response blocking with appendfsync=always - Provider pause/resume via DEBUG commands for deterministic testing - Uncommitted key rejection (reads return error for dirty keys) - MULTI/EXEC transaction durability semantics - Lua script and FCALL durability checks - Function store (FUNCTION LOAD/DELETE) durability blocking - Client disconnection during blocked state - Multiple concurrent clients with interleaved blocking/unblocking - INFO durability stats verification Signed-off-by: jjuleslasarte <jules.lasarte@gmail.com> Signed-off-by: Jules Lasarte <jules.lasarte@gmail.com>
Do you think we need a separate config for this? If you set up fsync always, can we imply that |
Should we factor in available memory before executing the command to avoid the over-buffering which may introduce OOM risk? |
Yeah, I went back and forth on this. I had the flag left over from the initial draft and figured it might be useful to not enable this since I wasn't sure whether we'd do a major version or minor with this change. I can remove |
Good point, we should have a mechanism for this. Let me think through the options -- a proactive one might be harder (as we need to estimate the output before execution) but we can probably track the ammount of pending responses (or pending writes to the durability providers) and start throttling (rejecting) writes after a certain threshold? |
|
Regarding the |
Yeah, proactive would be challenging but a reactive approach might be good enough. We could track the total consumed output buffer and initiate throttling once a predefined threshold is reached. Valkey’s existing In addition, pointing out that this client suspension should be conditioned on the ability to zero-copy responses (e.g., the requested key is not robj based). |
Signed-off-by: Jules Lasarte <jules.lasarte@gmail.com>
Yeah, makes sense to me. I will remove it in the next commit, along with other feedback! |
Signed-off-by: Jules Lasarte <jules.lasarte@gmail.com>
|
Just regarding the title and description of the PR. This is not really changing the AOF durability. It just makes it possible to do fsync in the background so it gets faster. I think we can name the PR something like Write-behind log for async AOF durability. The main benefit for the future is that it introduces the WBL into the code base. Also it isn't doing anything to sync replication, other than preparing for introducing it in the future, so I don't think it should be highlighted that much. |
Ack! yeah, my brain is focused on the long term but you are correct. I will address some of the other feedback and change the description. |
murphyjacob4
left a comment
murphyjacob4
left a comment
There was a problem hiding this comment.
Just some first pass comments, not a complete review
zuiderkwast
changed the title
Remove the standalone 'durability' / 'sync-replication' bool config. Durability is now implied by the combination of appendonly + appendfsync always — there is no need for a separate knob. - isDurabilityEnabled() now checks aof_state != AOF_OFF && aof_fsync == AOF_FSYNC_ALWAYS - Removed 'enabled' field from durable_t struct - AOF durability provider isEnabled() now requires appendfsync always (instead of a pass-through returning primary_repl_offset) - updateAppendFsync() and updateAppendOnly() call durabilityReset() - Updated integration and unit tests to use appendfsync always/everysec Signed-off-by: jjuleslasarte <jules.lasarte@gmail.com> Signed-off-by: Jules Lasarte <jules.lasarte@gmail.com>
… queue Separate the two concerns in uncommitted key tracking: 1. Is a key dirty? Mark it immediately when mutated, even inside MULTI/EXEC (using LLONG_MAX as placeholder offset so reads block). 2. When can we clean it? Use a FIFO offset tracker queue in the durability system. After a transaction completes, update the placeholder to the real offset and enqueue for cleanup. drainCommittedKeys() pops from the queue head when the committed offset advances, replacing the old periodic scan-based clearUncommittedKeysAcknowledged() cron job entirely. - Removed keys_cleanup_time_limit_ms and curr_db_scan_idx from durable_t - Removed getUncommittedKeysCleanupTimeLimit() - drainCommittedKeys() called from notifyDurabilityProgress() - Removed periodic clearUncommittedKeysAcknowledged() from serverCron Signed-off-by: jjuleslasarte <jules.lasarte@gmail.com> Signed-off-by: Jules Lasarte <jules.lasarte@gmail.com>
Resolve conflict in src/debug.c by keeping DEBUG subcommands from both sides: reply-blocking-pause/resume and set-io-last-written (feature branch) alongside force-free-primary-async and protect-client (unstable). Signed-off-by: Chris Li <1070743423@qq.com>
The open threads mentioned above are addressed or tracked as follow-ups of separate issues. |
…#3381) Per the maintainers' decision, reply-blocking observability is internal and test-only, not customer-facing. Move the reply_blocking_* fields out of the standalone (all/everything-visible) INFO section and into the hidden Debug section, which is emitted only for an explicit INFO debug -- excluded from INFO, INFO all, and INFO everything. Update the durability tests to read INFO debug. Further field-level changes (dropping reply_blocking_enabled and the offset fields, and relocating the committed/uncommitted offsets to the Replication section) are tracked separately pending TSC approval. Signed-off-by: Chris Li <1070743423@qq.com>
|
Hey @CPUmaker - have some cycles to spend reviewing this now. Sorry for the delay |
The ProcessingEventsWhileBlocked branch of beforeSleep skipped handleClientsWithPendingWrites() while an offloaded AOF fsync was in flight, withholding even durable replies from every client, and only on the re-entrant path (busy scripts, module yield, loading). Durability is already enforced per client by the reply-blocking boundary and the write cap in _writeToClient()/writevToClient(), so flush unconditionally like the main beforeSleep path. Add a regression test that drives the re-entrant path with a busy Lua script. Also address review nits on the AOF offload path: - Rewrite processAofBioFlushResult() as a switch over the flush states, with an explicit panic on an unknown state. - Document the forced vs non-forced drain in flushAppendOnlyFile(). - Extract bioProcessAofFlushOffload() from the bio job loop to reduce nesting. Signed-off-by: Chris Li <1070743423@qq.com>
…y-io#3381) Rename CMD_KEYSPACE_INFORMATIONAL to CMD_KEYSPACE_GLOBAL and keep it only on the commands that read the entire keyspace with no key argument (KEYS, SCAN, RANDOMKEY, DBSIZE, CLUSTERSCAN). Drop it from the key-targeting commands (EXISTS, DUMP, TYPE, TTL, PTTL, EXPIRETIME, PEXPIRETIME, TOUCH, OBJECT ENCODING/FREQ/IDLETIME/REFCOUNT) and from the static OBJECT HELP. Previously all of these blocked on the global replication offset whenever any key was dirty. Key-targeting commands carry their keys in argv, so they now fall through to the per-key path and block only on their own key's offset -- correctness is unchanged (a dirty key still blocks, and whole-keyspace scans still block globally), but reads of clean keys are no longer stalled behind unrelated in-flight writes. OBJECT HELP touches no keys and is no longer tracked or blocked. The flag remains a tracking trigger in clientEligibleForResponseTracking so a future whole-keyspace command that is not tagged READONLY/WRITE is still tracked and blocked correctly. Add durability tests covering per-key blocking for key-targeting reads, global blocking for whole-keyspace commands, and OBJECT HELP never blocking. Signed-off-by: Chris Li <1070743423@qq.com>
…alkey-io#3381) Active expiry, eviction, and lazy expiry deleted keys through a handleUncommittedKeyForClient() call placed by hand next to each signalModifiedKey(NULL, ...) callsite. Route these background writes through signalModifiedKey instead: replyBlockingSignalModifiedKey() now records the key when the client is NULL, and postExecutionUnitOperations() drains the set with the final replication offset. Any future background write path that calls signalModifiedKey is covered automatically, with no per-function callsite. Client-command marking is unchanged. This also tightens the durability boundary for background deletions. The old callsites marked the key with primary_repl_offset before the deletion was propagated (propagateDeletion defers via alsoPropagate), so a read could unblock one flush before the deletion was durable. Draining after propagatePendingCommands marks the key with the offset that includes the deletion, matching how client-command writes are already tracked. Add a durability test that a lazy-expired key's deletion is tracked and blocks later reads until it is durable. Signed-off-by: Chris Li <1070743423@qq.com>
Reply-blocking enforces zero-data-loss reads by withholding user-facing replies in the client output buffer until the write is durably acked. A module, however, consumes a VM_Call reply synchronously in C at call time, so once call() returns there is nothing left to hold. The only options for a dirty read are to reject the call or to let the module observe the value. Modules can already observe dirty reads under stock Valkey with appendfsync always, since fsync only defers the user reply, not the reply returned immediately to a VM_Call. This is therefore not a new hole, and gating module reads on durability is a separate opt-in feature to add later (see the unused module_cmd_blocking_offset hook). Remove the dead validateScriptForReplyBlocking rejection scaffold from the VM_Call path so the code reflects the intended behavior: module dirty reads are neither rejected nor blocked. Signed-off-by: Chris Li <1070743423@qq.com>
notifyKeyspaceEvent() distinguishes a first-pass notification (notify modules inline, defer the client pub/sub message) from the re-fired client notification at ack time. This context was encoded in a NOTIFY_IN_POST_COMMIT_TASK bit OR'd into the notification type, threaded through the deferred task payload, and masked back out before the class check. Model it as execution context instead: a reply_blocking.in_post_commit_task_execution flag raised only while executeDeferredTasksForAck() runs deferred tasks. This removes the OR-in/mask-out dance, prevents a stray high bit from ever polluting the server.notify_keyspace_events & type check, and frees bit 30 of the notification type space. Save/restore the flag around the drain loop so a future re-entrant task type cannot clear it prematurely. No behavior change: nothing other than the re-fired notifications generates keyspace events during the drain, so modules are still notified once, inline, on the first pass. Signed-off-by: Chris Li <1070743423@qq.com>
notifyKeyspaceEvent() distinguishes a first-pass notification (notify modules inline, defer the client pub/sub message) from the re-fired client notification at ack time. This context was encoded in a NOTIFY_IN_POST_COMMIT_TASK bit OR'd into the notification type, threaded through the deferred task payload, and masked back out before the class check. Model it as execution context instead: a reply_blocking.in_post_commit_task_execution flag raised only while executeDeferredTasksForAck() runs deferred tasks. This removes the OR-in/mask-out dance, prevents a stray high bit from ever polluting the server.notify_keyspace_events & type check, and frees bit 30 of the notification type space. Save/restore the flag around the drain loop so a future re-entrant task type cannot clear it prematurely. No behavior change: nothing other than the re-fired notifications generates keyspace events during the drain, so modules are still notified once, inline, on the first pass. Signed-off-by: Chris Li <1070743423@qq.com>
441b121 to
e484af5
Compare
jjuleslasarte
added
the
run-extra-tests
clearAllUncommittedKeys() logged at LL_NOTICE on every shutdown and reply-blocking reset, even when the feature is disabled (it runs unconditionally from replyBlockingCleanup()). This is an internal lifecycle breadcrumb with no operator value, and printing it on every shutdown broke the fragile tail -9 scan in the module hooks.tcl shutdown test by pushing the module-event-shutdown line out of the window (seen in the tls-module CI jobs). Lower it to LL_DEBUG, matching the sibling "Initializing reply-blocking subsystem" log, and drop the stale "for sync replication" wording (the feature is AOF durability). Signed-off-by: Chris Li <1070743423@qq.com>
| #define CMD_MODULE_GETCHANNELS (1ULL << 27) /* Use the modules getchannels interface. */ | ||
| #define CMD_TOUCHES_ARBITRARY_KEYS (1ULL << 28) | ||
| #define CMD_ALL_DBS (1ULL << 29) | ||
| #define CMD_KEYSPACE_GLOBAL (1ULL << 30) /* Command reads the whole keyspace with no key argument (KEYS, SCAN, RANDOMKEY, DBSIZE, CLUSTERSCAN). Used by reply-blocking. */ |
There was a problem hiding this comment.
Wouldn't info also be here if DBSIZE is here?
There was a problem hiding this comment.
Good catch — you're right that INFO's Keyspace section reports kvstoreSize(db->keys), so it exposes the same not-yet-durable key count.
That said, I don't think INFO should get KEYSPACE_GLOBAL: every KEYSPACE_GLOBAL command is READONLY — a real keyspace data read on the normal read path. INFO isn't READONLY and it's STALE | LOADING. Those flags mean INFO is contractually allowed to answer on a replica disconnected from its primary (serving stale, possibly-inconsistent data) and while the dataset is still loading — i.e. it already operates outside the data-consistency guarantee. Holding it to the stricter zero-data-loss reply-blocking guarantee would contradict its own semantics, and its keyspace line sits alongside plenty of other non-durable live counters (rdb_changes_since_last_save, ops/sec, etc.).
There's also a real blast-radius cost: KEYSPACE_GLOBAL blocks the whole reply whenever any data is dirty, which under appendfsync always is ~always. INFO is on every health-check/monitoring path, so gating it on the next fsync ack would stall probes broadly to make one coarse stat durable.
WDYT?
9 participants
AOF-based Durability
fixes: #3994
Summary
This PR optimizes
appendfsync alwaysby moving the AOF flush+fsync off themain thread (a write-behind log), so synchronous-durability workloads aren't
bottlenecked on the event loop. It preserves the existing zero-data-loss
guarantee — and extends it to reads of not-yet-durable keys — by holding
client replies in the client output buffer (COB) until the write is fsynced,
rather than blocking the main thread. It is milestone one in the durability
plan (here).
So a client writing
SET foo barstill won't receive+OKuntil the data isfsynced to disk (no application-level
WAITAOFneeded), but the main thread isfree to keep serving other clients while the fsync is in flight.
How it's enabled
The feature is opt-in and gated behind a hidden feature flag. All of the
following must hold for reply-blocking to engage on a node:
With the default
bio-aof-offload-enabled no, none of the reply-blocking coderuns, so stock Valkey is unaffected. There is no separate
durabilityconfig (an earlier bool was removed); behavior derives from the flag + AOF
settings. Changing
appendfsync/appendonlytriggers a reply-blocking reset.Design decisions
are never paused before executing — the command runs immediately and its
reply is buffered in the COB. There is no command-rejection path;
consistency is enforced entirely by holding the reply. A byte-precise
"disallowed byte offset" boundary in the COB prevents the networking layer
from flushing past it until the durably-committed offset catches up. A single
client can hold a list of boundaries (deeper pipelines unblock
incrementally), and
_writeToClient()/writevToClient()cap each syscall atthe boundary so a write physically cannot push blocked bytes out.
per-database with the replication offset they were dirtied at. Read commands
touching an uncommitted key are response-blocked the same as writes: their
replies are held until the offset at which the key was last modified is
durably committed. This holds even for non-replicating replies (e.g.
SETNXreturning
0because of a not-yet-durable write) — any reply can encodenot-yet-durable state. Committed keys are cleaned up by iterating the per-DB
hashtable when the committed offset advances — no separate tracking queue.
client-side-cache (tracking) invalidations are deferred until the write is
acknowledged, so subscribers and caches don't learn about changes before
they're durable. Module keyspace notifications are delivered inline at
change time (not deferred) — modules filter on their own
event_maskindependent of
notify-keyspace-events, and inline delivery is the only modelthat (a) preserves read-after-write for change-time consumers and (b) composes
with module client-blocking (Fix engine crash on module client blocking during keyspace events #1819). The deferred client notification
re-enters
notifyKeyspaceEvent()at ack time withNOTIFY_IN_POST_COMMIT_TASKset.
every fsync under
appendfsync always, the write+fsync runs as a background(BIO) job that advances the AOF-acked offset on completion. (This was
initially built on IO threads, then corrected to the BIO path.)
How to Review
The branch isn't squashed; below is a subsystem-oriented guide (roughly the
recommended reading order). Reviewers are also welcome to review the whole thing
at once.
aof.c,bio.c,bio.hBIO_AOF_FSYNCjob (buf != NULL⇒ write+fsync+notify path),bioCreateAofFsyncNotifyJob, and the acked-offset tracking. Standalone perf improvement.reply_blocking.c/.hcomputeCommandBlockingOffset(replicating vs non-replicating paths), the COB boundary mechanism,getDurablyCommittedOffset(), andnotifyReplyBlockingProgressunblocking frombeforeSleep.uncommitted_keys.c/.hFLUSHDB/FLUSHALL/SWAPDB+ function-store handling.post_commit_task.c/.hserver.c,db.c,networking.c,notify.c,script.c,module.c,object.c,evict.cclientHasPendingRepliessame-buffer invariant, inline-module / deferred-client notification split, deferred-reply boundary-on-commit hooks.commands/*.json,commands.defCMD_KEYSPACE_INFORMATIONALflag on read-only introspection commands (KEYS,SCAN,EXISTS,TTL,TYPE,OBJECT *,DUMP,RANDOMKEY, …) so they're tracked for reply-blocking.config.c,server.h,src/Makefile,cmake/Modules/SourceFiles.cmake,src/unit/Makefile,src/unit/CMakeLists.txtbio-aof-offload-enabledflag, struct fields, source wiring.src/unit/test_reply_blocking.cpp,tests/durability/reply_blocking.tcl,tests/unit/moduleapi/block_keyspace_notification_durability.tclNew INFO section
INFO reply_blockingreports:reply_blocking_enabled—1when reply-blocking is active on this node(hidden flag on +
appendonly yes+appendfsync always+ primary), else0.When
0, no other fields are emitted.reply_blocking_read_blocked_count— cumulative number of read commandreplies that have been held in the COB (because they touched a not-yet-durable
key). Counts since reply-blocking was last (re)enabled; not a live gauge.
reply_blocking_write_blocked_count— cumulative number of write commandreplies that have been held in the COB awaiting their durability ack. Same
reset semantics as the read counter.
reply_blocking_clients_waiting_ack— live gauge: how many clientscurrently have at least one reply held in the COB waiting for the committed
offset to advance.
reply_blocking_uncommitted_keys— live gauge: number of keys currentlytracked as modified-but-not-yet-durable across all databases.
reply_blocking_previous_acked_offset— the most recent durably-committedreplication offset the node has advanced to (the offset up to which replies
have been released).
reply_blocking_primary_repl_offset— the node's current primaryreplication offset. The gap between this and
reply_blocking_previous_acked_offsetis the amount of written-but-not-yet-durabledata — i.e. how far ahead execution is of the fsync.
(plus internal cumulative timing counters — per-type blocked-time totals in
microseconds — used for latency measurement)
New DEBUG subcommands (testing levers)
DEBUG reply-blocking-pause aof— freeze the AOF-acked offset todeterministically hold blocked replies.
DEBUG reply-blocking-resume aof— resume and let the offset catch up.DEBUG client-enforce-reply-list <0|1>— force replies to skip the staticbuffer and go straight to
c->reply, to exercise the reply-list boundarybranch.
DEBUG set-io-last-written <client-id> <bufpos> <data_len>— inject apost-partial-write
io_last_writtenstate to probe the boundary comparison.Changes since initial review round
Addressed feedback from @murphyjacob4, @zuiderkwast, @sushilpaneru1,
@sumitk163, @QuChen88 (and CodeRabbit):
durability_provider.{c,h}) —inlined AOF logic directly since only one provider exists; renamed
getDurabilityConsensusOffset→getDurablyCommittedOffset,anyDurabilityProviderEnabled→isAofDurabilityEnabled(per @zuiderkwast).unified
BIO_AOF_ALWAYS_FLUSHintoBIO_AOF_FSYNC, distinguishing thewrite+fsync+notify path by
buf != NULL.durabilityconfig; gated the whole subsystem behindthe hidden
bio-aof-offload-enabledflag derived alongsideappendfsync always(per @murphyjacob4, @zuiderkwast).uncommitted-key cleanup — simpler, less memory, better hot-key behavior (per
@sumitk163).
durability*→replyBlocking*/postCommit*;durable_task.*→post_commit_task.*; INFOdurability_*→reply_blocking_*;DEBUG durability-provider-pause→reply-blocking-pause.delivered inline and unconditionally (independent of
notify-keyspace-eventsand foreground/background origin); only client pub/sub is deferred. Removed the
redundant
moduleNotifyKeyspaceEvent()call at certification time.boundary when the deferred reply is committed into the COB, with a fresh COB
snapshot taken at commit (avoids a use-after-free on a stale reply-block
pointer across the module-blocked window).
getIntFromObjectonlyrange-checks
INT_MIN/INT_MAX, so a rejectedMOVE key <dbnum..INT_MAX>read
server.db[dest_dbid]out of bounds; both MOVE branches now bounddest_dbidto[0, dbnum).createClient()(zmallocdoesn't zero).uncommitted_keysentries whenFLUSHALLsetsall_dbs_dirtyinside a transaction.enable-debug-assert) guarding theclientHasPendingRepliescursor-vs-boundary comparison._FILE_OFFSET_BITS=64), macOS clang CI fixes,_AtomicC++ compatibility inserver.h, dangling event-string fix in thekeyspace-notify task,
bufpos-vs-data_lencopy-avoidance fix,getKeysUsingKeySpecsfix, CMake source-list rename(
durable_task.c→post_commit_task.c), and removal ofsync-replication / replica-rejection leftovers.
Known follow-ups (out of scope for this PR)
(#4027): expose
module-facing APIs so modules can observe/interact with the durability
(reply-blocking) state rather than relying on internal hooks. Tracked
separately.
per the review-bot summary): module keyspace notifications fire inline at
change time by default. Durability-sensitive modules (external forwarding,
audit/CDC) may want delivery gated on the write becoming durable. A follow-up
should add a module option (mirroring the internal
DELAY_KEYSPACE_NOTIFICATION_FOR_ZDLflag) to opt notifications intodurable/delayed delivery while the default stays inline. Not in this PR.
replies are held in the COB until durable rather than pausing the client
before execution, a sustained stream of not-yet-acked writes can grow the COB
unboundedly and risk OOM. There is no pre-execution memory gate today; the
likely mitigation is reactive throttling — suspend command ingestion for a
client once its buffered output crosses a threshold, resuming when it drops
back below — rather than the connection-closing
client-output-buffer-limit.Not addressed in this PR.
PR ships only the AOF path. The broader durability plan adds replica-based
acknowledgement as additional providers, at which point the committed offset
becomes a consensus across providers. Future milestone.
(#4029): throughput/latency
of the BIO offload and the reply-blocking overhead are benchmarked separately;
results are posted in Performance testing of improvements #4029.
DEBUG set-io-last-writteninput validation (CodeRabbit, minor): thesubcommand has no debug-build guard and does not range-check its
bufpos/data_lenarguments, so a negative/oversized cast is reachable. It isa test-only DEBUG path; hardening is a low-priority follow-up.