diff --git a/src/db.zig b/src/db.zig index 1a4015f..461d6f6 100644 --- a/src/db.zig +++ b/src/db.zig @@ -3146,8 +3146,19 @@ test "a checkpoint runs alongside writers on several collections" { var doc = make_doc(alloc, @intCast(i), "user") catch return error.Canceled; defer doc.deinit(); { - e.lock() catch return error.Canceled; - defer e.unlock(); + // The server's discipline, not the legacy whole-engine + // lock: catalog shared, then the target collection + // exclusive (commands.zig dispatch). `write_catalog` takes + // the same two in the same order, and that is the whole + // reason its walk of a collection's counters and extents is + // safe -- a writer that skipped the collection lock would + // not be excluded by it, and the test would be checking + // nothing. + e.lock_catalog(false) catch return error.Canceled; + defer e.unlock_catalog(false); + const coll = (e.lock_collection("app", name, true, true) catch + return error.Canceled) orelse return error.Canceled; + defer e.unlock_collection(coll, true); e.insert("app", name, &doc, undefined) catch return error.Canceled; } // As the dispatch epilogue does (commands.zig): the append bumps diff --git a/src/pager.zig b/src/pager.zig index 7435389..3642a35 100644 --- a/src/pager.zig +++ b/src/pager.zig @@ -321,6 +321,13 @@ pub const Pager = struct { /// See `track_dirty`. Pages written since the last sync. dirty: if (track_dirty) std.AutoHashMapUnmanaged(u32, void) else void, + /// Guards `dirty`, and only `dirty`. Writers to the file itself are kept + /// apart by the locks their *owners* hold -- a collection's, an index's -- + /// and the pages two of them touch never overlap. This set is the one thing + /// they share: two appenders on different collections record into the same + /// hash map at the same time, which is a torn map rather than a torn page. + /// Test-only instrumentation, so it costs the server nothing. + dirty_lock: if (track_dirty) std.Io.Mutex else void, /// Whether the last `protect_image` actually took effect. Checked by a test: /// an mprotect that silently fails would leave the belt looking present and /// doing nothing, which is worse than not having it. @@ -386,6 +393,7 @@ pub const Pager = struct { .free_hold = .empty, .free_pending = .empty, .dirty = if (track_dirty) .empty else {}, + .dirty_lock = if (track_dirty) .init else {}, .protect_ok = false, }; // The bit set is the one heap allocation `self` owns before `deinit` can @@ -467,7 +475,7 @@ pub const Pager = struct { /// recycling hands back. pub inline fn page_mut(self: *Pager, p: u32) *align(page_size) [page_size]u8 { assert_msg(p < self.mapped_pages, "write to a page past the mapped end of the data file"); - if (track_dirty) self.dirty.put(self.gpa, p, {}) catch {}; + self.note_dirty(p, p); return @ptrCast(@alignCast(self.reserve.ptr + (@as(usize, p) << page_shift))); } @@ -569,7 +577,7 @@ pub const Pager = struct { inline fn page_mut_slot(self: *Pager, p: u32) *align(page_size) [page_size]u8 { assert(p == page_watermark_a or p == page_watermark_b); assert_msg(p < self.mapped_pages, "write to a watermark slot past the mapped end"); - if (track_dirty) self.dirty.put(self.gpa, p, {}) catch {}; + self.note_dirty(p, p); return @ptrCast(@alignCast(self.reserve.ptr + (@as(usize, p) << page_shift))); } @@ -589,13 +597,21 @@ pub const Pager = struct { pub inline fn bytes_mut(self: *Pager, off: u64, len: usize) []u8 { assert_msg(off + len <= @as(u64, self.mapped_pages) << page_shift, "write past the mapped end of the data file"); if (track_dirty) { - var pg_i: u32 = @intCast(off >> page_shift); - const last: u32 = @intCast((off + len - 1) >> page_shift); - while (pg_i <= last) : (pg_i += 1) self.dirty.put(self.gpa, pg_i, {}) catch {}; + self.note_dirty(@intCast(off >> page_shift), @intCast((off + len - 1) >> page_shift)); } return self.reserve[@intCast(off)..][0..len]; } + /// Record pages `first..=last` as written since the last sync. See + /// `dirty_lock` for why this is the one shared structure on the write path. + inline fn note_dirty(self: *Pager, first: u32, last: u32) void { + if (!track_dirty) return; + self.dirty_lock.lockUncancelable(self.io); + defer self.dirty_lock.unlock(self.io); + var p = first; + while (p <= last) : (p += 1) self.dirty.put(self.gpa, p, {}) catch {}; + } + // -- allocation --------------------------------------------------------- /// Hand out `n` contiguous pages, growing the file if needed. For callers @@ -778,7 +794,11 @@ pub const Pager = struct { try std.posix.msync(self.reserve[0..len], std.posix.MSF.SYNC); } try self.file.sync(self.io); - if (track_dirty) self.dirty.clearRetainingCapacity(); + if (track_dirty) { + self.dirty_lock.lockUncancelable(self.io); + defer self.dirty_lock.unlock(self.io); + self.dirty.clearRetainingCapacity(); + } } /// Make the published image read-only at the hardware level. See