From 1814020df9284f50459870a9918b961aa982ef7b Mon Sep 17 00:00:00 2001 From: Aleksey Shakhmatov Date: Tue, 4 Aug 2026 00:39:44 +0300 Subject: [PATCH] db/pager: an append resumes inside its extent after a checkpoint `slab_reserve` and `reserve_overflow` abandoned the rest of their extent whenever a checkpoint froze the page the tail pointed into, and took a fresh 8 MiB one. The comment called the waste "bounded by one extent per collection per checkpoint", which is true per checkpoint and says nothing about the sum: nothing reclaims it except a rebuild, and a rebuild only runs when there is garbage. A pure-insert workload produces none. Measured, 40 collections of inserts with incompressible payloads so the log actually reaches the checkpoint threshold: live data file log 29 MB 340 MB 29 MB 38 MB 542 MB 5 MB <- checkpoint 67 MB 681 MB 33 MB 76 MB 1076 MB 9 MB <- checkpoint 115 MB 1357 MB 14 MB <- checkpoint 11.8x the live data and climbing by ~335 MB per checkpoint (40 x 8 MiB), which would exhaust the 64 GB address-space reservation after roughly 6 GB of real data -- and after ~1.2 GB with 200 collections. `DatabaseTooLarge` on a database that is nowhere near too large. The fix is what the plan called for and never got: round the cursor up to the next *system* page and keep the extent. Only the page holding the live tail is in the published image; the rest of the extent holds nothing referenced by the image or by an index, so `Pager.mark_appendable` hands it back for appending (and unprotects it, since it may sit below the stable mark where `protect_image` made it read-only). System pages rather than 4 KiB ones because writeback tears at the granularity the kernel manages: a 4 KiB store dirties a whole 16 KiB page on Apple Silicon, and tearing there would take out the published bytes sharing it. Same 40 collections after: 340 MB -> 352 MB across three checkpoints, the ratio falling monotonically toward the 8 MiB-per-collection floor. 64,000 documents across 8 collections verified byte-for-byte and after a kill -9. The churn gate is unchanged at 1.65x, big.js at 4 GB unchanged (4.32 GB file, reopen 0.5 s, RSS after reopen 130 MB). Two mutations, verified red: dropping the resume branch (a fresh extent per checkpoint), and rounding to `page_size` instead of `map_align` (the resumed append then shares a system page with the published image). --- src/db.zig | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/index.zig | 17 +++++---- src/pager.zig | 38 ++++++++++++++++++++ 3 files changed, 146 insertions(+), 6 deletions(-) diff --git a/src/db.zig b/src/db.zig index 1049ab9..cf74735 100644 --- a/src/db.zig +++ b/src/db.zig @@ -139,6 +139,25 @@ pub const Collection = struct { // still writable. Asking the mark meant every recycled extent was thrown // away after one document, so churn never reused anything. if (self.pager.is_unpublished_at(self.slab_tail) and self.slab_tail + len <= self.slab_end) return; + // The page holding the tail is frozen, but the *rest* of the extent is + // not: nothing above the live cursor is referenced by the image or by an + // index. So skip to the next system page and keep the extent, instead of + // throwing away what is left of 8 MiB. + // + // This is what the plan called for ("append cursors are rounded up to the + // system page size at each checkpoint") and it matters more than it + // sounds: abandoning the extent costs ~8 MiB per collection per + // checkpoint, and a pure-insert workload generates no garbage, so + // compaction never fires and nothing ever gives it back. Measured at 40 + // collections: the data file reached 11.8x the live data and grew by + // ~335 MB per checkpoint, heading for DatabaseTooLarge at around 6 GB of + // real data. + const resumed = std.mem.alignForward(u64, self.slab_tail, pgr.map_align); + if (resumed + len <= self.slab_end) { + self.pager.mark_appendable(resumed, self.slab_end); + self.slab_tail = resumed; + return; + } // A document larger than the standard extent gets one of its own; BSON // reaches 16 MB and the extent is 8 MiB. const want_pages: u32 = @intCast(@max( @@ -2087,6 +2106,84 @@ test "compaction reclaims garbage but leaves a garbage-free log alone" { try testing.expect(engine.log.data_bytes < after_insert * 2); } +test "an append after a checkpoint keeps its extent instead of abandoning it" { + // Mutation check: delete the `resumed` branch in `slab_reserve`. Red on the + // extent count -- every checkpoint would take a fresh 8 MiB extent per + // collection and leave the old one's remaining space stranded, reclaimable + // only by a rebuild. A pure-insert workload produces no garbage, so no + // rebuild is ever triggered and nothing gives it back: measured at 40 + // collections, the data file reached 11.8x the live data and grew ~335 MB per + // checkpoint, on course for DatabaseTooLarge at ~6 GB of real data. + // + // Second mutation: round `resumed` to `pgr.page_size` instead of + // `pgr.map_align`. Red on the frozen-page assertion below on any host whose + // system page is larger than 4 KiB (16 KiB on Apple Silicon) -- a 4 KiB store + // dirties the whole system page, so a torn writeback would take the published + // bytes sharing it. + var threaded: std.Io.Threaded = .init_single_threaded; + defer threaded.deinit(); + var env = test_env(&threaded); + const io = env.io; + const gpa = testing.allocator; + + var tmp = try TmpLog.init(gpa); + defer tmp.deinit(gpa); + var engine = try Engine.open(gpa, io, tmp.path); + defer engine.deinit(); + engine.compact_threshold = std.math.maxInt(u64); // no rebuild may intervene + try engine.lock(); + defer engine.unlock(); + + var first = try make_doc(gpa, 1, "alice"); + defer first.deinit(); + try engine.insert("app", "users", &first, &env.gen); + try engine.commit(); + + const coll = engine.get_collection("app", "users").?; + try testing.expectEqual(@as(usize, 1), coll.slab_extents.items.len); + const extent_start = @as(u64, coll.slab_extents.items[0].first) << pgr.page_shift; + + try engine.checkpoint(); + const tail_at_checkpoint = coll.slab_tail; + try testing.expect(tail_at_checkpoint > extent_start); + const tail_before = engine.pager.alloc_tail; + + // The next write must land in the same extent, past the frozen page. + var second = try make_doc(gpa, 2, "bob"); + defer second.deinit(); + try engine.insert("app", "users", &second, &env.gen); + try engine.commit(); + + try testing.expectEqual(@as(usize, 1), coll.slab_extents.items.len); + // A page or two for the tree's copy-on-write is expected; a whole slab + // extent is the regression this guards against. + try testing.expect(engine.pager.alloc_tail < tail_before + slab_extent_pages); + try testing.expect(coll.slab_tail > tail_at_checkpoint); + + // The document itself landed on the next system-page boundary past the + // frozen tail -- checked at the offset the index recorded, since `slab_tail` + // has already advanced past it by the document's length. + const bob_enc = try id_key_for(gpa, bson.Value{ .int32 = 2 }); + defer gpa.free(bob_enc); + const bob_off = coll.id_index.lookup_exact(bob_enc).?; + try testing.expectEqual(std.mem.alignForward(u64, tail_at_checkpoint, pgr.map_align), bob_off); + + // And the page holding the last published byte is still frozen, so the + // resumed append cannot have shared a page with the durable image. + try testing.expect(!engine.pager.is_unpublished_at(tail_at_checkpoint - 1)); + + // Both documents readable, and the first one -- which lives below the + // checkpoint's tail -- unharmed. + try testing.expectEqual(@as(u64, 2), engine.live_docs); + for ([_]i32{ 1, 2 }) |id| { + const id_enc = try id_key_for(gpa, bson.Value{ .int32 = id }); + defer gpa.free(id_enc); + const off = coll.id_index.lookup_exact(id_enc).?; + const name = try bson.get_at(gpa, coll.doc_bytes(off), "name"); + try testing.expectEqualStrings(if (id == 1) "alice" else "bob", name.?.string); + } +} + test "a replace that changes nothing is not a write" { // Mutation check: delete the byte comparison in `upsert`'s `.replace` arm. // Red on all three: the log grows, the document is superseded so the engine diff --git a/src/index.zig b/src/index.zig index c143cf2..715b8d0 100644 --- a/src/index.zig +++ b/src/index.zig @@ -458,13 +458,18 @@ pub const Index = struct { if (rec_len > inline_limit) overflow_bytes += rec_len; } if (overflow_bytes == 0) return; - // Same rule as the document slab: a checkpoint freezes the page the tail - // points into, so a frozen tail means starting a fresh extent rather - // than writing inside the durable image. - // Same reasoning as the document slab: a recycled extent is below the - // stable mark and still writable, so ask whether these bytes are in the - // published image rather than where they sit. + // Same rules as the document slab, for the same reasons. Ask whether the + // bytes are in the published image rather than where they sit, because a + // recycled extent is below the stable mark and still writable; and when + // the tail's page is frozen, skip to the next system page and keep the + // extent rather than abandoning what is left of it. if (self.pager.is_unpublished_at(self.ovf_tail) and self.ovf_tail + overflow_bytes <= self.ovf_end) return; + const resumed = std.mem.alignForward(u64, self.ovf_tail, pgr.map_align); + if (self.ovf_tail != 0 and resumed + overflow_bytes <= self.ovf_end) { + self.pager.mark_appendable(resumed, self.ovf_end); + self.ovf_tail = resumed; + return; + } // One extent for the whole batch, or a bespoke one when a single // record is larger than the standard extent (a BSON string reaches // 16 MB). diff --git a/src/pager.zig b/src/pager.zig index 9c35d3d..738f08f 100644 --- a/src/pager.zig +++ b/src/pager.zig @@ -425,6 +425,44 @@ pub const Pager = struct { return p < self.unpublished.bit_length and self.unpublished.isSet(p); } + /// Reclaim the unwritten tail of an extent for appending again after a + /// checkpoint, from `off` (which must already be clear of every byte the + /// published image references) to the end of the extent. + /// + /// The append cursors need this because `publish` clears `unpublished` + /// wholesale, which makes an extent the appender still owns read as part of + /// the image. Without it the only safe move was to abandon the rest of the + /// extent and take a fresh one -- ~8 MiB per collection at every checkpoint, + /// never reclaimed when the workload produces no garbage for compaction to + /// find. Measured: 40 collections of pure inserts put the data file at 11.8x + /// the live data and rising by ~335 MB per checkpoint, on course to exhaust + /// the address-space reservation after about 6 GB of real data. + /// + /// The caller's contract, which is what makes this sound: `off` is rounded up + /// past the *live* append cursor to a system-page boundary, so no page in + /// `[off, end)` holds a byte referenced by the image or by the live indexes. + /// System pages rather than 4 KiB ones because writeback tears at the + /// granularity the kernel manages -- a 4 KiB store dirties the whole 16 KiB + /// page on Apple Silicon, and a torn writeback there would take out the image + /// bytes sharing it. + pub fn mark_appendable(self: *Pager, off: u64, end: u64) void { + assert(off <= end); + assert(off % map_align == 0); + const first: u32 = @intCast(off >> page_shift); + const last: u32 = @intCast(end >> page_shift); // exclusive + if (last <= first) return; + self.alloc_lock.lockUncancelable(self.io); + defer self.alloc_lock.unlock(self.io); + assert_msg( + last <= self.mapped_pages, + "marking pages appendable past the mapped end of the data file", + ); + self.unpublished.setRangeValue(.{ .start = first, .end = last }, true); + // These pages may sit below the stable mark, where `protect_image` has + // made them hardware read-only. + self.unprotect(first, last - first); + } + /// The same question for the byte-offset consumers: may an append at `off` /// land in place, or does its page belong to the durable image? Used by the /// document slab and the overflow slab, which would otherwise have to guess