From c8d547fef59a9be80767a08806fa2365e1c2c262 Mon Sep 17 00:00:00 2001 From: Aleksey Shakhmatov Date: Mon, 3 Aug 2026 00:09:11 +0300 Subject: [PATCH] db/commands: acknowledged writes reach the disk again Three defects, each of which made the database lose data that had already been acknowledged, or answer a client with a malformed reply. - Engine.commit decided a writer was "already covered" by comparing log.end_pos with the position of the last completed commit. Under block framing an append leaves its bytes in the log's open in-memory block and does not move end_pos -- only sealing does. So once the first commit had set committed_end = end_pos, every later write command found itself covered and returned without sealing or syncing anything. A no-op deleteMany followed by insertMany(50) was acknowledged with the file still 16 bytes (its header) and lost all 50 documents on kill -9, which is precisely what e2e2's crash pair does. Coverage is now decided by sequence number, which counts records rather than bytes on disk. - Compaction read the new log's end position before syncing it, but the sync is what seals the open block, and the seal is what moves end_pos past it. Appends after a compaction therefore started inside the compacted file's last block and overwrote it, so those documents were gone at the next replay: e2e6's phase 2 ended with 1000 documents in memory and 996 after a graceful restart. - cmd_find returned early on a missing namespace without putting anything in the reply, so a find on an unknown collection arrived at the driver as a response with no `ok` field ("MongoServerError: n/a") instead of an empty cursor. The other commands' missing-namespace paths were fine. Verified with the unit suite in ReleaseFast/ReleaseSafe/Debug, the split fuzzer, all six e2e suites (e2e6 back to 72/72) and the kill -9 crash pair -- none of which passed beforehand -- plus 13 kill -9 runs over 1/2/8 connections with 1200 acknowledged inserts each and nothing lost. tests/e2e/results/phase7.txt records the benchmark with the fixes in place: no regression against phase6 (bulk 739 -> 753 MB/s, updateMany 1.9 -> 2.0 ms, RSS 547 -> 546 MB), and concurrent durable writes now measurable at 7.1k/15.0k/21.8k docs/s over 1/8/32 connections. --- src/commands.zig | 9 +++++- src/db.zig | 32 ++++++++++++++------ tests/e2e/results/phase7.txt | 58 ++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 10 deletions(-) create mode 100644 tests/e2e/results/phase7.txt diff --git a/src/commands.zig b/src/commands.zig index 52e5f8e..db158ed 100644 --- a/src/commands.zig +++ b/src/commands.zig @@ -624,7 +624,14 @@ fn cmd_find(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !void { // Sorting and emitting need the documents as trees; materialize the // matched page into the reply arena (the slab itself is never copied). - const coll = ctx.engine.get_collection(db_name, coll_name) orelse return; + const coll = ctx.engine.get_collection(db_name, coll_name) orelse { + // A find on a namespace that does not exist is an empty cursor, not + // an error -- and above all not a reply with no `ok` at all, which + // is what returning here without one sends. + const none: []const *const bson.Document = &.{}; + try emit_docs_tree(reply, db_name, coll_name, proj_pairs, none); + return reply.put_ok(); + }; // Lives in the reply arena; freed with it. var tree_docs: std.ArrayListUnmanaged(*const bson.Document) = .empty; const arena = reply.arena_alloc(); diff --git a/src/db.zig b/src/db.zig index 647de36..02266a6 100644 --- a/src/db.zig +++ b/src/db.zig @@ -137,8 +137,11 @@ pub const Engine = struct { /// Serializes commit decisions; the group-commit leader holds it while /// sealing and syncing. commit_lock: std.Io.Mutex = .init, - /// Log end position covered by the last completed commit. - committed_end: u64 = 0, + /// Sequence number covered by the last completed commit. A seq rather + /// than a file position: an append leaves its bytes in the log's open + /// block without moving end_pos, so a position comparison would call + /// buffered-but-unwritten records durable. + committed_seq: u64 = 0, /// Writers increment before appending and decrement after; the commit /// leader waits for this to reach zero so its seal covers every append /// in flight, coalescing many writers' fsyncs into one. @@ -327,13 +330,16 @@ pub const Engine = struct { pub fn commit(self: *Engine) !void { try self.commit_lock.lock(self.io); defer self.commit_lock.unlock(self.io); + // Everything this command appended is at or below the current seq. + // Read it before waiting, so a leader that sealed before this + // command's appends cannot be mistaken for one that covered them. + try self.log_lock.lock(self.io); + const want = self.seq; + self.log_lock.unlock(self.io); // A commit is in flight; wait for it, then check whether the // leader's seal covered this writer's append. while (self.committing) self.commit_done.wait(self.io, &self.commit_lock) catch return; - try self.log_lock.lock(self.io); - const covered = self.log.end_pos == self.committed_end; - self.log_lock.unlock(self.io); - if (covered) { + if (self.committed_seq >= want) { return; // a concurrent commit already synced this writer's records } // Become the leader: the flag is set before the wait below, so any @@ -351,7 +357,9 @@ pub const Engine = struct { try self.log_lock.lock(self.io); defer self.log_lock.unlock(self.io); try self.log.sync(); - self.committed_end = self.log.end_pos; + // Appends drained above, so the seal covered every record written so + // far -- including any that arrived while this leader waited. + self.committed_seq = self.seq; done = true; self.committing = false; self.commit_done.signal(self.io); @@ -837,9 +845,13 @@ pub const Engine = struct { continue; // a writer appended during the snapshot; retry } errdefer self.log_lock.unlock(self.io); - const new_end_pos = new_log.end_pos; // Durable before the rename makes it the database. try new_log.sync(); + // After the sync, not before: sync seals the open block, and + // that seal is what moves end_pos past it. Reading the position + // first leaves appends writing over the compacted file's last + // block, which then vanishes on the next replay. + const new_end_pos = new_log.end_pos; try std.Io.Dir.renameAbsolute(tmp_path, self.log.path, self.io); // Persist the rename: fsync the parent directory so the new @@ -855,7 +867,9 @@ pub const Engine = struct { // Log.open starts at end_pos 0 and does not replay; continue // appending where the compacted file actually ends. self.log.end_pos = new_end_pos; - self.committed_end = new_end_pos; + // The rewritten file was synced before the rename, so everything + // applied so far is durable. + self.committed_seq = self.seq; // The rewritten log holds only live documents. self.dead_docs = 0; self.gpa.free(old_path); diff --git a/tests/e2e/results/phase7.txt b/tests/e2e/results/phase7.txt new file mode 100644 index 0000000..b0ac620 --- /dev/null +++ b/tests/e2e/results/phase7.txt @@ -0,0 +1,58 @@ +# Phase 7 gate — mongo-lite vs MongoDB 8.3.7, 1g dataset / ~16k docs +# Ratio < 1.0 = mongo-lite faster. Reproduce: bash tests/e2e/compare-run.sh 1g 16k +# All five roadmap items plus the three durability/correctness fixes below. +# Compare: tests/e2e/results/phase6.txt (same code, broken commit path) and +# tests/e2e/results/phase1.txt (pre-tree baseline). + +benchmark mongo-lite mongodb ratio +insertOne (sequential) ×200 0.19 ms 4.9 ms 0.0x +bulk insert throughput 753.5 MB/s 704.6 MB/s 1.1x +docs loaded 65,536 65,536 1.0x +createIndex({k: 1}) 75.7 ms 75.5 ms 1.0x +countDocuments({}) 3.4 ms 11.2 ms 0.3x +findOne({_id: }) 0.61 ms 0.63 ms 1.0x +findOne({k: 500}) (indexed) 0.57 ms 2.3 ms 0.2x +find({p: {$gte,$lt}}).count() (scan) 13.5 ms 14.6 ms 0.9x +find({}).sort({_id:-1}).limit(20) 2.3 ms 2.5 ms 0.9x +find({}, {proj}).limit(1000) 3.7 ms 4.4 ms 0.8x +aggregate $group by k 8.3 ms 15.1 ms 0.5x +updateOne({_id}) ×50 0.16 ms 0.20 ms 0.8x +updateMany({k: 7}, {$inc}) 2.0 ms 5.3 ms 0.4x +deleteOne({_id}) + insertOne 0.58 ms 5.0 ms 0.1x +node client RSS 153 MB 154 MB 1.0x +server RSS 546 MB 1424 MB +kill -9 reopen 0.8s 1.3s +db on disk 97MB 95MB + +# No regression from the fixes. Against phase6 (which measured a commit path +# that mostly skipped its fsync): bulk 739 -> 753.5 MB/s, insertOne 0.20 -> +# 0.19 ms, updateMany 1.9 -> 2.0 ms, RSS 547 -> 546 MB, disk and reopen +# unchanged — all within run noise. The real fsync per commit does not show +# here because this benchmark is single-connection and writes through large +# insertMany batches, so one commit is amortized over the whole batch. +# +# What phase6 could not have measured, because the crash pair did not pass: +# - kill -9 durability: 13 runs over 1/2/8 connections, 1200 acknowledged +# inserts each, zero lost. +# - concurrent durable writes (sequential insertOne per client): +# 1 client 7.1k docs/s | 8 clients 15.0k | 32 clients 21.8k. +# +# Fixes in this commit (each reproduced before it was fixed): +# 1. Engine.commit compared log.end_pos with the last committed position to +# decide a writer was already covered. Under block framing an append +# leaves its bytes in the log's open block without moving end_pos, so +# after the first commit every later write command returned without +# sealing or syncing. A no-op deleteMany followed by insertMany(50) was +# acknowledged with the file still 16 bytes (header only) and lost +# everything on kill -9. Coverage is now by sequence number. +# 2. Compaction read new_end_pos before new_log.sync(), but the sync is +# what seals the open block and moves end_pos past it, so later appends +# overwrote the compacted file's last block. e2e6's phase 2 ended with +# 1000 documents in memory and 996 after a graceful restart. +# 3. cmd_find returned without a reply on a missing namespace, so find on +# an unknown collection reached the driver as a malformed response +# ("MongoServerError: n/a") instead of an empty cursor. +# +# Verification: unit suite in ReleaseFast/ReleaseSafe/Debug, the split fuzzer +# (src/fuzz_split.zig), all six e2e suites (e2e6 72/72), and the kill -9 +# crash pair — none of which passed before these fixes.