diff --git a/.gitignore b/.gitignore index cda1338..0a5e98e 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ compile_commands.json *.db *.wal .claude +next_steps.txt diff --git a/.obsidian/appearance.json b/.obsidian/appearance.json index a8b1e37..4be7969 100644 --- a/.obsidian/appearance.json +++ b/.obsidian/appearance.json @@ -1,3 +1,3 @@ { - "theme": "moonstone" + "theme": "obsidian" } \ No newline at end of file diff --git a/.obsidian/graph.json b/.obsidian/graph.json new file mode 100644 index 0000000..8771672 --- /dev/null +++ b/.obsidian/graph.json @@ -0,0 +1,22 @@ +{ + "collapse-filter": false, + "search": "design-docs", + "showTags": false, + "showAttachments": false, + "hideUnresolved": false, + "showOrphans": true, + "collapse-color-groups": false, + "colorGroups": [], + "collapse-display": true, + "showArrow": false, + "textFadeMultiplier": 0, + "nodeSizeMultiplier": 1, + "lineSizeMultiplier": 1, + "collapse-forces": true, + "centerStrength": 0.518713248970312, + "repelStrength": 10, + "linkStrength": 1, + "linkDistance": 250, + "scale": 0.8914715268792586, + "close": false +} \ No newline at end of file diff --git a/.obsidian/workspace.json b/.obsidian/workspace.json index 8f00694..58a8978 100644 --- a/.obsidian/workspace.json +++ b/.obsidian/workspace.json @@ -4,21 +4,21 @@ "type": "split", "children": [ { - "id": "19615ae5e5dc0062", + "id": "f8b803ce0789e6ab", "type": "tabs", "children": [ { - "id": "b27174596c7c49e7", + "id": "213624f19b44c94b", "type": "leaf", "state": { "type": "markdown", "state": { - "file": "design-docs/DD-001-storage-file-layout.md", + "file": "design-docs/DD-002-buffer-pool-manager.md", "mode": "preview", "source": false }, "icon": "lucide-file", - "title": "DD-001-storage-file-layout" + "title": "DD-002-buffer-pool-manager" } } ] @@ -41,7 +41,9 @@ "type": "file-explorer", "state": { "sortOrder": "alphabetical", - "autoReveal": false + "autoReveal": false, + "showSearch": false, + "searchQuery": "" }, "icon": "lucide-folder-closed", "title": "Files" @@ -78,8 +80,7 @@ } ], "direction": "horizontal", - "width": 300, - "collapsed": true + "width": 300 }, "right": { "id": "4ce9e65978c028c9", @@ -185,23 +186,30 @@ "bases:Create new base": false } }, - "active": "b27174596c7c49e7", + "active": "213624f19b44c94b", "lastOpenFiles": [ - "build/debug/Testing/Temporary/CTestCheckpoint.txt", - "build/debug/Testing/Temporary/LastTest.log.tmp", - "build/debug/CMakeFiles/disk_manager_test.dir/link.d", - "build/debug/CMakeFiles/disk_manager_test.dir/test/storage/disk_manager_test.cpp.o.d", - "test/storage/disk_manager_test.cpp.tmp.9657.43c7f8735f29", - "test/storage/disk_manager_test.cpp.tmp.9657.bdd712030b93", "build/debug/CMakeFiles/kernsql.dir/link.d", - "build/debug/stu9JUzF", - "build/debug/st9efrCo", - "build/debug/CMakeFiles/kernsql_lib.dir/src/storage/disk_manager.cpp.o.d", - "build/debug/CMakeCache.txt.tmp00436", - "assets/slotted_page.png", - "assets/storage_file_layout.png", + "build/debug/stt8jRT2", + "build/debug/stTk6Uc9", + "build/debug/CMakeFiles/kernsql_lib.dir/src/buffer/buffer_pool_manager.cpp.o.d", + "src/buffer/buffer_pool_manager.cpp.tmp.16764.7acecd0a7cef", + "src/buffer/buffer_pool_manager.cpp.tmp.16764.6d12f137211c", + "build/debug/stB1ka1L", + "build/debug/stMIkJFI", + "build/debug/stCuaJp3", + "build/debug/stI9cO3C", + "build/debug/CMakeFiles/kernsql_lib.dir/src/buffer/page_guard.cpp.o.d", + "design-docs/DD-003-threading-model.md", + "design-docs/DD-002-buffer-pool-manager.md", + "design-docs/DD-002-buffer-manager.md", + "design-docs/notes/Buffer-Pool-Manager.md", + "design-docs.md", + "build/asan/_deps/googletest-src/docs/reference/assertions.md", + "design-docs/notes/Concurrency-Control.md", "design-docs/notes/Indexes.md", "design-docs/DD-001-storage-file-layout.md", + "assets/slotted_page.png", + "assets/storage_file_layout.png", "Untitled.canvas", "README.md" ] diff --git a/CMakeLists.txt b/CMakeLists.txt index d9ca251..98769b0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -6,9 +6,14 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_EXTENSIONS OFF) set(CMAKE_EXPORT_COMPILE_COMMANDS ON) # for clangd -add_library(kernsql_lib STATIC src/common/logger.cpp src/storage/disk_manager.cpp) +add_library(kernsql_lib STATIC + src/common/logger.cpp + src/storage/disk_manager.cpp + src/buffer/buffer_pool_manager.cpp + src/buffer/page_guard.cpp) target_include_directories(kernsql_lib PUBLIC src) -target_compile_options(kernsql_lib PUBLIC -Wall -Wextra -Wpedantic -Wshadow -Wconversion) +target_compile_options(kernsql_lib PUBLIC -Wall -Wextra -Wpedantic -Wshadow -Wconversion + -Werror=format) add_executable(kernsql src/shell/main.cpp) target_link_libraries(kernsql PRIVATE kernsql_lib) @@ -26,5 +31,7 @@ foreach(t ${TEST_SRCS}) get_filename_component(name ${t} NAME_WE) add_executable(${name} ${t}) target_link_libraries(${name} PRIVATE kernsql_lib GTest::gtest_main) - gtest_discover_tests(${name}) + # A dropped condvar notify makes a concurrency test HANG rather than fail, and a hung test + # blocks the run forever instead of reporting. Generous because TSan is roughly 10x slower. + gtest_discover_tests(${name} PROPERTIES TIMEOUT 120) endforeach() diff --git a/README.md b/README.md index 5451034..2def979 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@

- A minimal SQL database built from scratch in C++20 on Linux — + A minimal SQL database built from scratch in C++23 on Linux — storage engine, transactions, and query engine, no shortcuts.

@@ -19,51 +19,109 @@ It is a learning-in-public project. The goal is not to compete with SQLite; it i understand how real databases (Postgres in particular) work by building one, and to leave behind a codebase clean enough that others can learn from it. -## Architecture at a glance +The emphasis is **depth over surface area**. The SQL dialect is deliberately tiny; the +concurrency underneath it is not. Every design decision is recorded in +[`design-docs/`](design-docs/) with the alternatives that were rejected and why — those +documents are the most useful thing in this repository. -KernSQL follows a Postgres-flavored design: +## Status + +| Component | State | +|---|---| +| `DiskManager` — single file, 4 KB pages, persistent free list, thread-safe | done, tested | +| `PageHeader` — 32-byte self-describing header (self page id, format version) | done | +| `Replacer` — CLOCK sweep with capped usage counts | done, tested | +| `BufferPoolManager` — sharded page table, frame state machine, RAII page guards | code complete, **tests pending** | +| Shell — REPL over the buffer pool, one command per operation | done | +| Slotted pages + heap file | next | +| B+tree with latch crabbing | planned | +| Catalog | planned | +| Parser, binder, executor | planned | +| Two-phase locking + deadlock detection | planned | + +## Architecture | Layer | Design | |---|---| -| Storage | Single-file, page-based (4 KB), slotted pages, heap files | -| Caching | Buffer pool with LRU-K replacement, pin/latch semantics | -| Indexing | B+tree with latch crabbing for concurrent access | -| Concurrency control | MVCC with snapshot isolation — in-heap version chains (`xmin`/`xmax`), first-updater-wins | -| Durability | Redo-only write-ahead log, checkpoints, crash recovery, vacuum | +| Storage | Single-file, page-based (4 KB), self-describing page headers, slotted pages, heap files | +| Caching | Buffer pool: 16-way sharded page table, CLOCK-sweep eviction, pin/latch separation, RAII guards | +| Indexing | B+tree with latch crabbing, fixed-width integer keys | +| Concurrency control | Strict two-phase locking — row-level shared/exclusive locks, wait-for-graph deadlock detection | +| Durability | Explicit `Shutdown()` — flush, then `fsync`. No write-ahead log (see non-goals) | | SQL front-end | Hand-written lexer and recursive-descent parser, binder with type checking | -| Execution | Volcano (iterator) model, rule-based planner with predicate pushdown and index selection | +| Execution | Volcano (iterator) model, fixed execution strategy — no cost model | | Interface | Interactive shell, thread-per-session concurrency | -A key consequence of this combination: **rollback writes nothing to the heap.** Aborted -transactions simply become invisible — the same property that makes redo-only logging -sufficient in Postgres. +Two decisions shape everything else: + +**Thread-per-session.** One thread carries a statement down the entire stack and back; layers +are a code decomposition, never a scheduling one. Every layer below is therefore synchronous +and blocking by construction ([DD-003](design-docs/DD-003-threading-model.md)). + +**Latches are not locks.** Latches protect physical structures for nanoseconds and are ordered +to prevent deadlock; locks protect logical database state for the length of a transaction and +deadlock is detected and resolved. Conflating them is the classic error, and the distinction is +load-bearing throughout the codebase. -## Features (v1 scope) +## Scope (v1) **SQL surface** -- `CREATE TABLE` / `DROP TABLE` +- `CREATE TABLE` - `INSERT`, `UPDATE`, `DELETE` -- `SELECT` with `WHERE`, `ORDER BY`, `LIMIT`, `INNER JOIN` -- `GROUP BY` with `COUNT` / `SUM` / `AVG` (stretch) +- `SELECT` with `WHERE` and a single `INNER JOIN` - `BEGIN` / `COMMIT` / `ROLLBACK` -- Types: `BIGINT`, `VARCHAR(n)`, `BOOLEAN` — with `NULL` support +- Types: `INT`, `VARCHAR(n)`, with `NULL` support **Engine guarantees** -- Snapshot isolation for all transactions; readers never block writers -- Crash safety: committed data survives `kill -9` (WAL replay on restart) -- Concurrent sessions with correct latching throughout — the full test suite runs under +- Serializable isolation via strict 2PL; deadlocks are detected and one transaction is aborted +- Concurrent sessions with correct latching throughout — the test suite runs under ThreadSanitizer and AddressSanitizer in CI -- System catalog stored in the database itself, as regular tables +- Durability on clean shutdown + +## Non-goals + +Deliberately out of scope. These are decisions, not omissions — each is recorded with its +reasoning in the relevant design doc. + +**No write-ahead log, and therefore no crash recovery.** Durability comes from an explicit +`Shutdown()` that flushes and `fsync`s. A `kill -9` loses every dirty page in the buffer pool, +and because a transaction's pages are not flushed atomically, a crash can leave the database +*structurally* inconsistent — a half-applied B+tree split, not merely missing recent writes. +This is the single largest simplification in the project. It is deferred rather than unexamined: +adding a WAL would change the buffer pool's flush sequence (a page could no longer be written +before the log record describing it) and would demote the shutdown flush from the durability +mechanism to a restart-time optimization. + +**No MVCC.** Concurrency control is 2PL, so readers block writers and writers block readers. +MVCC was rejected on scope: it is a tuple-format decision (version chains or an undo log) plus +a mandatory garbage collector, and it reaches into layers this project builds later. 2PL is +additive to what already exists. -## Non-goals (v1) +**No query optimizer.** No cost model, no statistics, no join ordering. An index is used if and +only if the predicate is on the primary key. -Deliberately out of scope, so the core stays finishable and understandable: +**No variable-length index keys**, no prefix compression, no hash join, no overflow pages — +tuples larger than a page are rejected rather than split. -distributed anything · cost-based optimization · subqueries, CTEs, views, triggers, -foreign keys · `ALTER TABLE` · floating-point types · authentication · network wire -protocol. +**No page checksums.** Space is reserved in the page header; the self page id catches +misdirected reads, but not bit rot. + +**Not portable across architectures.** Page headers are `memcpy`'d, so the file format is +host-endian and host-ABI. + +Also out of scope: distributed anything · subqueries, CTEs, views, triggers, foreign keys · +`ALTER TABLE`, `DROP TABLE` · aggregates, `GROUP BY`, `ORDER BY` · floating-point types · +authentication · a network wire protocol. + +## Design docs + +The reasoning behind each component, including rejected alternatives: + +- [DD-001 — Storage file layout](design-docs/DD-001-storage-file-layout.md) +- [DD-002 — Buffer pool: concurrency and latching](design-docs/DD-002-buffer-pool-manager.md) +- [DD-003 — Threading and execution model](design-docs/DD-003-threading-model.md) ## Project layout @@ -76,7 +134,7 @@ assets/ logo and branding ## Building -Requires GCC 13+ (or Clang 17+), CMake ≥ 3.25, Ninja. +Requires Clang 17+ with libc++ (the sanitizer presets pin it), CMake ≥ 3.25, Ninja. ```bash cmake --preset debug && cmake --build --preset debug # fast development build @@ -89,10 +147,17 @@ cmake --preset tsan && cmake --build --preset tsan && ctest --preset tsan # ra CI runs the ASan and TSan suites plus a clang-format check on every pull request; `main` only moves through green pipelines. -## Status +Then drive the engine by hand: -🚧 Early days — foundation phase. Progress, design decisions, and write-ups are tracked in -[`design-docs/`](design-docs/) as each component lands, feature by feature, branch by branch. +```bash +./build/debug/kernsql mydb.db +kernsql> new +new: allocated page 2 +kernsql> write 2 hello +kernsql> read 2 +read: page 2 -> "hello" +kernsql> quit +``` ## References diff --git a/design-docs/DD-002-buffer-pool-manager.md b/design-docs/DD-002-buffer-pool-manager.md new file mode 100644 index 0000000..5b7e6cf --- /dev/null +++ b/design-docs/DD-002-buffer-pool-manager.md @@ -0,0 +1,374 @@ +# Buffer Pool Manager: Concurrency & Latching + +**Status:** Accepted +**Component:** `src/buffer` (`BufferPoolManager`) + +Caches a fixed number of `PAGE_SIZE` pages from `DiskManager` ([DD-001](./DD-001-storage-file-layout.md)) +as **frames**, and is what the catalog, heap and index use instead of `DiskManager` directly. +`DiskManager`'s `ReadPage`/`WritePage` are safe to call concurrently but provide no exclusion +between two callers on the *same* page; that is this layer's job. + +## The rule everything follows + +> **No lock is held across an I/O operation, and a frame's identity — which page it holds — +> changes only inside a single critical section that performs no I/O.** + +## Frame lifecycle + +One `enum class FrameState` per frame, under the frame's metadata mutex. + +| State | Mapped | Contents | Pin count | In replacer | +|------------|--------|-------------|-----------|----------------------| +| `Free` | no | meaningless | 0 | no (on free list) | +| `Loading` | yes | meaningless | ≥ 1 | no | +| `Resident` | yes | valid | ≥ 0 | iff `pin_count == 0` | +| `Failed` | no | meaningless | ≥ 1 | no | + +| Transition | Owner | +|----------------------|------------------------------------------------------------| +| `Free → Loading` | a fetch miss, under the new page's shard lock | +| `Loading → Resident` | the loader, on a successful read; notifies waiters | +| `Loading → Failed` | the loader, on a read error; erases the mapping first | +| `Failed → Free` | the loader only, once waiters have dropped their pins | +| `Free → Resident` | `NewPage` — nothing on disk to read, so no `Loading` phase | +| `Resident → Free` | reclaim or `DeletePage`; requires unpinned and clean | + +There is no `Reclaiming` state: reclaim performs no I/O, so it is one critical section and nothing +can need to cancel it. + +## Frame fields + +`page_id`, `state`, `pin_count`, the inline `PAGE_SIZE` buffer, `dirty_epoch`/`flushed_epoch`, a +metadata mutex + condition variable, and a content latch. + +The pool is one contiguous allocation made once at construction — a frame owns a mutex and condvar, +so it is neither copyable nor movable. + +Not stored: `frame_id` (it is the array index), `page_lsn` (a WAL concern), `usage_count` (the +replacer's). + +## Locks + +1. **Content latch** (per frame, reader/writer) — the page bytes. Held as long as a caller's access + needs. `std::shared_mutex` behind `Frame::AcquireRead`/`AcquireWrite`, which return `auto`-bound + locks so the type stays swappable. A replacement lock type must provide **move construction, + move assignment, `unlock()`, and a destructor that releases only if still held** — move + assignment is required by the guards' own move-assignment operator, not just by the fetch paths. +2. **Metadata mutex** (per frame) — `state`, `pin_count`, `page_id`. Must be a real mutex, not an + atomic word: `Loading` needs a condition variable to wait on. +3. **Page-table shard locks** — the `page_id → frame_id` map split into 16 shards, keyed + `page_id & (N-1)`. Page ids are sequential, so low bits already spread evenly. +4. **Free-list lock** — one mutex, one push or pop. +5. **Replacer lock** — clock hand and candidate set only. `usage_count` is `std::atomic[]` + outside it, bumped by a capped relaxed CAS, so the per-access path is lock-free. + +I/O coordination is not a lock: it is the `Loading` state plus the frame's condvar. + +## The pin invariant + +> **No thread may read, write, wait on, or flush a frame's contents without holding a pin. Every +> lock here is momentary; the pin is what spans.** + +- A miss waiter **pins before** it sleeps on `Loading`, so `page_id` is provably unchanged across + the wait — no re-validation, no restart. +- A flush **pins the frame it flushes**, or a concurrent miss repurposes it mid-writeback. +- A reclaimer flushing a dirty victim **pins across the writeback** and re-validates after. + +## Dirtiness + +Two monotonic `std::atomic` counters outside every lock. `dirty_epoch` is bumped by each +`WritePageGuard` release; `flushed_epoch` is raised by each completed flush. Dirty iff +`dirty_epoch > flushed_epoch`. This keeps the content latch and metadata mutex from ever being held +together. + +**Flush sequence**, used by every flush path: + +1. Acquire the shared content latch. +2. Sample `D = dirty_epoch`. +3. `DiskManager::WritePage`. +4. Release the content latch. +5. Raise `flushed_epoch` to `D` (CAS upward only). + +A writer that dirties after step 4 bumps past `D`, so the frame stays dirty; one cannot dirty +between 1 and 4, which needs the exclusive latch. Never falsely clean. + +## Free list and replacer + +- **Free list** — frame_ids in state `Free`. A LIFO stack behind a mutex, fully populated at + construction. No condvar, no exhausted flag, no watermarks; an empty list sends the caller to the + replacer. +- **Replacer** — frames in `Resident` with `pin_count == 0`. `RecordAccess` bumps a capped + `usage_count`; `SetEvictable` adds/removes a candidate; `Evict()` sweeps and returns the first + candidate at zero, `nullopt` if none are evictable. + +Frozen contracts: + +- `SetEvictable` is **idempotent** — it adjusts the evictable count only when the flag actually + flips. +- `Evict` **removes its victim from the candidate set itself** and resets its `usage_count`, under + the replacer lock. +- A frame `Evict` returns that the caller declines to reclaim is **normal, not an error**; it stays + out of the candidate set until its next unpin. + +**Clock parameters:** `usage_count` caps at 3. One global clock hand. `Evict` is bounded by +construction — `nullopt` immediately when nothing is evictable, otherwise at most +`(cap + 1) × capacity` positions, after which it takes the first evictable frame regardless of +count. + +## Reclaiming a frame + +A thread that needs a frame and finds the free list empty reclaims one inline, holding **no other +lock** on entry. + +1. `Evict()` → victim, already out of the candidate set. `nullopt` → return `BufferPoolFull`. +2. **Clean victim.** Take the victim's shard lock, then its metadata mutex. Re-validate: + `Resident`, `pin_count == 0`, `!IsDirty()`, mapping still points here. If so — erase the + mapping, set `page_id = INVALID_PAGE`, `state = Free`, reset epochs, release both. The frame is + now private to this thread. **No I/O in this section.** +3. **Any check fails** — drop both locks, back to step 1 for another victim. Do not return it to + the replacer; whoever pinned it owes a `SetEvictable(true)` on release. +4. **Dirty victim.** Under the metadata mutex verify `Resident` and `pin_count == 0`, take a pin, + note `page_id`, release everything, run the flush sequence, then retry from step 2 with + `pin_count == 1` (our own) as the condition. Drop the pin either way. + +Step 4 spans I/O and is safe because identity never changes during the flush: the mapping stays +live, so a concurrent fetcher gets a cache hit and pins, and the reclaimer sees `pin_count > 1` at +re-validation and moves on. + +The clean-check in step 2 is exact, not conservative: nothing can newly dirty the frame (that needs +a guard, which needs the page table we hold the shard lock for), and nothing already dirtied is +missed (a guard's release bumps `dirty_epoch` strictly before decrementing `pin_count`). + +## Lock ordering + +> **The only two locks ever held at once are a page-table shard lock and a frame metadata mutex, in +> that order. Enforce it as: never acquire a shard lock while holding a frame metadata mutex.** + +Everything else is a leaf. The replacer lock is taken and dropped inside `Evict()`. The free-list +lock covers one push or pop. Content latches are taken only after both others are released. No path +needs two shard locks. + +Standing rules: + +- **Never block on I/O or a condvar while holding a shard lock, with one named exception.** Both + waits here — a miss waiter on `Loading`, the loader on the failed-load path — drop it first. + + **The exception: `DeletePage` holds the shard lock across `DiskManager::DeallocatePage`.** That is + the whole guarantee against the stale-mapping race in "Known behaviour" — no new mapping for the + page can be published until its header reads `FREE`, at which point the miss-path validation + rejects the fetch. Affordable because the I/O is bounded and the path is not hot: + `DeallocatePage` is three syscalls with no `fsync`, and it already serializes every deleter and + allocator in the process on `DiskManager`'s global `meta_latch_`, so the shard lock adds only the + other pages in that one shard for a span the call already spends. New ordering edge, + shard → `meta_latch_`, closing no cycle: `DiskManager` knows nothing about the page table. The + alternatives — a tombstone entry in the shard, or keeping the frame mapped as `Loading` across + the deallocation — both put a new case into the hot fetch path, and are the answer only if + `DeletePage` ever shows up in a profile. +- **Never call into the replacer or free list while holding a frame's metadata mutex, with one + named exception.** Note the 0→1 transition under the mutex, release it, *then* call + `RecordAccess`/`SetEvictable(false)`. This admits a benign race (a frame briefly evictable while + about to be re-pinned), harmless because reclaim re-validates `pin_count == 0` before repurposing. + + **The exception: `UnpinPage`'s `SetEvictable(true)` is made under the metadata mutex.** The two + directions are not symmetric. Deferring the 1→0 publication splits the decision from the act, and + `DeletePage` fits in the gap — it sees the `pin_count == 0` the unpinner just produced, vacates + the frame, calls `SetEvictable(false)` (which removes nothing, the membership having never been + published) and pushes to the free list; the straggling `true` then lands on free stock, leaving + one frame on the free list *and* in the candidate set. See "Known behaviour". The 0→1 direction + has no such hazard: the only thing that adds a frame to the candidate set is an unpin, and in + that window the caller holds the pin, so no unpin can occur. The exception is safe because + `Replacer` is a strict leaf — every method takes only its own mutex and never reaches back into a + frame, a shard, or the free list — so the added `metadata → replacer` edge closes no cycle. The + cost is that an unpinner can now hold a frame's metadata mutex while waiting behind `Evict()`'s + sweep, which sharpens the sweep-under-one-lock issue noted below. +- Any operation needing two content latches takes them in a globally fixed order, not a + caller-dependent one. State the order where that operation is designed. + +## Page guards + +RAII guards, never raw frames. + +``` +Result FetchPageRead(page_id_t page_id); +Result FetchPageWrite(page_id_t page_id); +Result NewPage(); +``` + +Latched from birth, in the mode the caller named. **No pin-only guard and no upgrade path**; a +future optimistic B+tree descent must drop the guard, re-fetch for write, and re-validate. + +`ReadPageGuard` exposes `span`; `WritePageGuard` also exposes a mutable +span. `PageId()` returns a value **stored in the guard**, never read back from `Frame::page_id` — +that field is under the metadata mutex. + +**Release sequence (frozen):** + +1. `WritePageGuard` only: bump `dirty_epoch`. +2. Release the content latch. +3. Metadata mutex; decrement `pin_count`; note a 1→0 transition; if it transitioned, + `SetEvictable(frame_id, true)` **under that same mutex** (the named exception in "Lock ordering"). +4. Release the metadata mutex. + +Step 1 precedes 2 so a flusher sees an epoch covering every write it is about to capture, and +precedes 3 so a reclaimer's clean-check is exact. The frame cannot be reclaimed between 2 and 3 — +the pin, not the latch, protects identity. `WritePageGuard` bumps the epoch **unconditionally**. + +**Value semantics:** non-copyable; movable, with the moved-from guard's manager pointer nulled. +`Drop()` is explicit and idempotent; the destructor calls it. + +Move-assignment **releases before taking over**, and checks self-assignment **by address** +(`this == &other`) — guards have no `operator==`, and the question is identity, not equality. +Without that check, `g = std::move(g)` drops the pin and then assigns the nulled members to +themselves, silently leaving a live-looking guard that holds nothing. Transfer is an explicit +`Drop()`-then-steal, **not** a swap: swapping hands the old pin to the moved-from object, deferring +its release to that object's lifetime instead of now. + +`UnpinPage` is **private**; the guards are friends. Guards live in `page_guard.{hpp,cpp}` with +`BufferPoolManager` forward-declared. + +## Control flow + +### FetchPage — hit + +1. Shard lock for `page_id`; look up `frame_id`. +2. Metadata mutex. **Increment `pin_count` immediately**, before any wait. Note a 0→1 transition. +3. Release the shard lock. If `Loading`, wait on the condvar. If `Failed`, decrement the pin, + notify, return the error. +4. Release the metadata mutex, then `RecordAccess`, and `SetEvictable(false)` if it went 0→1. +5. Acquire the content latch in the requested mode; return the guard. + +### FetchPage — miss + +**Release the shard lock first** — acquiring a frame may reclaim one, which needs the *victim's* +shard lock. + +1. `free_list_.TryPop()`, else reclaim inline. `BufferPoolFull` propagates from there. +2. Re-acquire the shard lock and **look up again**. If present: release, push our frame back to the + free list, restart from the hit path. +3. Still under the shard lock: insert the mapping, and under the metadata mutex stamp `page_id`, + `Loading`, `pin_count = 1`, epochs zeroed. Release both. The mapping goes in before the read so + a concurrent fetcher of the same page waits on `Loading` instead of racing. +4. `DiskManager::ReadPage`, under no lock. +5. Success: metadata mutex, `Resident`, notify all, release. Then `RecordAccess`, content latch, + return the guard. +6. Failure: the failed-load path, then return the error. + +### Failed load + +The loader owns disposal. + +1. Loader takes the shard lock, erases the mapping, releases it. +2. Metadata mutex: `state = Failed`, notify all. +3. Each waiter wakes, sees `Failed`, and drops its pin **through `UnpinPage`** — which notifies on + `Failed` precisely so this path has no second decrement site. No disposal. +4. Loader waits until `pin_count == 1` (its own), then resets to `Free`, drops its pin, and pushes + to the free list — not the replacer. + +`UnpinPage` is the single place a pin is ever decremented. Any other decrement is a bug: it would +be a second site that has to remember the `Failed` notify, and forgetting it parks the loader on +the condvar forever. + +### NewPage / DeletePage / UnpinPage / FlushPage + +- **NewPage** — get a frame as in miss step 1, then `DiskManager::AllocatePage` outside every lock. + No `Loading` phase and no re-lookup race. Shard lock, insert mapping, stamp `Resident` and + `pin_count = 1`, release, zero the buffer under the content latch, return a `WritePageGuard`. +- **DeletePage** — shard lock, then metadata mutex. Reject if `pin_count != 0`. Dirty contents are + discarded without a flush. Set `INVALID_PAGE`, `Free`, reset epochs, erase the mapping, release + the metadata mutex. Then, **still holding the shard lock**, `SetEvictable(false)`, + `DiskManager::DeallocatePage`, push to the free list. Vacating before deallocating satisfies + `DeallocatePage`'s not-resident contract; keeping the shard lock across the deallocation is the + named exception in "Lock ordering", and without it a racing miss can republish the page. +- **UnpinPage** — private, guard-only. Under the metadata mutex: notify all if `Failed`, note + whether this is a **1→0 transition on a `Resident` frame**, decrement, and — still under that + mutex — `SetEvictable(true)` if it transitioned. Holding the mutex across that call is the one + exception in "Lock ordering"; releasing first is what produced the lost-`SetEvictable` bug below. + + Both halves of that condition are load-bearing. Testing the post-decrement value instead of the + transition would mark a frame evictable on a spurious unpin at `pin_count == 0`; testing the pin + without the state would admit a `Free` frame — one already on the free list — into the candidate + set, so `TryPop` and `Evict` could hand the same frame to two threads. + + **Underflow is a caller bug, not a condition to absorb.** Assert in debug; in release the count + goes negative and the frame is permanently stuck (never 1→0 again, so never re-evictable, and + `DeletePage` rejects it forever), which is a capacity leak rather than corruption. Log it at a + level that survives `NDEBUG`. +- **FlushPage / FlushAllPages** — pin first, run the flush sequence, unpin. `FlushAllPages` is a + sequential loop, **best-effort, not atomic across frames**. + +### Shutdown and destruction + +Flushing is **not** the destructor's job. `Status Shutdown()` is the durable operation; the +destructor is a backstop that reports the caller's mistake. + +**`Shutdown()`** — idempotent. Verify quiescence (every frame `pin_count == 0` and not `Loading`), +then `FlushAllPages()`, then **`DiskManager::Sync()`**. Both steps: `FlushAllPages` moves page bytes +into the OS page cache via `pwrite`, which is not durability. Returns `Status`, so the caller can +decide what a failure means — exit non-zero, refuse to mark the database cleanly closed. + +**Quiescence is a precondition, not something the pool arranges.** Destroying an object while +another thread uses it is undefined regardless of what the destructor does: a live `WritePageGuard` +holds a `BufferPoolManager*` that dangles the instant `~BufferPoolManager` returns, and its later +`Drop()` calls `UnpinPage` on freed memory. No locking inside the destructor can prevent that. The +owner establishes quiescence by **joining every worker thread** — guards are stack-scoped, so an +unwound stack has released every pin, and `join()` supplies the synchronizes-with edge. See +[DD-003](./DD-003-threading-model.md). + +**A non-zero pin at destruction is therefore proof that a guard outlived the pool.** Assert on it +rather than working around it — this is a use-after-free you want to fail loudly in the test suite. + +**`~BufferPoolManager`** — if `Shutdown()` already ran, nothing to do. If it did not, that is a +caller bug: log it, best-effort flush, and abort rather than silently discarding dirty pages. A +destructor cannot return a `Status` or throw, so it is structurally the wrong place to be deciding +what a failed flush means. + +This mirrors `DiskManager`, which does not `fsync` in `~DiskManager` and lists implicit durability +as an explicit non-goal ([DD-001](./DD-001-storage-file-layout.md)). It also removes a destruction- +order hazard: the pool holds `DiskManager&`, so a destructor-driven flush is only correct while +that reference outlives the pool — an ordering constraint that disappears entirely once the flush +happens before either destructor runs. + +Once a WAL exists, the shutdown flush becomes a restart-time optimization rather than the +durability mechanism. It is load-bearing today only because there is no log yet. + +## Known behaviour + +- **A fetch racing a delete of the same page left the pool caching a deallocated page. FIXED + 2026-08-30.** `DeletePage` erased the mapping, dropped both locks, and only then called + `DiskManager::DeallocatePage`. A fetcher that misses inside that window reads a header still + stamped `ALLOCATED`, passes the miss-path validation, and republishes a `Resident` mapping for a + page that is about to join the disk free list. Consequences, in increasing severity: a later + fetch of that page id is a cache hit returning stale bytes with no error; `AllocatePage` hands + the id out again and the new owner's fetch hits the stale frame, so a freshly allocated page + arrives holding a dead page's contents; a flush of that frame then writes those bytes over the + new owner's. Measured by `ConcurrentDeleteAndFetchStayConsistent` at ~1–5% of successful fetches + and ~6% of rounds (`stale_after_delete`). Fixed by holding the shard lock across + `DeallocatePage` — the named exception in "Lock ordering" — so no new mapping can be published + before the header reads `FREE`. Both counters have been zero over 10,000 rounds since. The pool + no longer leans on the caller to exclude this, though callers generally do anyway: InnoDB frees + pages under the index X-latch, and Postgres does not recycle page ids outside `VACUUM`'s + `AccessExclusiveLock`. +- **Lost `SetEvictable(true)`, leaving a frame on the free list and in the candidate set at once. + FIXED 2026-08-30.** `UnpinPage` published evictability after releasing the metadata mutex; + `DeletePage` could vacate the frame in that gap and revoke a membership that had not yet been + created. Symptom: `evictable > resident_frames` at quiescence, reproduced by + `ConcurrentDeleteAndFetchStayConsistent` in roughly 8 runs in 20. Not corruption in practice — + `ReclaimFrame` re-validates `state == Resident` and declines the victim — but the census lied and + the free-list/candidate-set exclusion was broken. Fixed by the "Lock ordering" exception above. +- `DeletePage` can spuriously fail while a reclaimer holds a transient pin across a dirty victim's + writeback. It rejects rather than waiting, so deleting a page you still hold a guard on is an + error rather than a deadlock. +- A reclaimer can flush a victim and then lose it to a fetcher that pins during the writeback. + Wasted work, nothing corrupted. +- A miss that picks a dirty victim pays the write inline. + +## Non-goals + +- **Durability without an explicit `Shutdown()`**, mirroring `DiskManager`'s stance on `Sync()`. +- A background cleaner thread (bgwriter / page cleaner). Add it when measurement shows misses + stalling on writeback. +- Packing `state` and `pin_count` into one CAS'd atomic word (LeanStore/Umbra style). +- Lock-free/RCU buffer pool; async I/O (io_uring); NUMA-aware placement. +- Read→write latch upgrade, and the optimistic B+tree descent that would want it. +- Sharding the free list or replacer; a per-shard clock hand. +- A spin-then-block content latch replacing `std::shared_mutex`. diff --git a/design-docs/DD-003-threading-model.md b/design-docs/DD-003-threading-model.md new file mode 100644 index 0000000..13928f2 --- /dev/null +++ b/design-docs/DD-003-threading-model.md @@ -0,0 +1,150 @@ +# Threading & Execution Model + +**Status:** Accepted +**Component:** engine-wide — constrains every layer from the shell down to `src/storage` + +## Context + +A statement descends shell → concurrency control (MVCC) → planning → execution → `BufferPoolManager` +([DD-002](./DD-002-buffer-pool-manager.md)) → `DiskManager` ([DD-001](./DD-001-storage-file-layout.md)). +This doc fixes **which thread runs each of those layers**, because the answer constrains every layer +above storage and is expensive to change once the MVCC layer starts making assumptions. + +## The rule everything follows + +> **One thread carries a statement down the entire stack and back. Layers are a code decomposition, +> never a scheduling decomposition.** + +## Thread-per-session + +One OS thread per client connection, owned by that connection for its lifetime: + +``` +accept() -> spawn thread -> loop { read SQL; parse; plan; execute; send results } -> disconnect -> exit +``` + +The thread *is* the session's execution context. Its call stack holds the statement's state; session +state — current transaction, prepared statements, session settings — lives with it. 200 connected +clients means 200 threads, most blocked in `read()`. + +This is Postgres (process-per-connection), MySQL/InnoDB (thread-per-connection), and SQLite +(caller's thread). It is already what the README commits to. + +### Why not a thread pool per layer + +The staged/SEDA shape — an MVCC pool handing off to a buffer-pool pool — was tried in the DB +literature (StagedDB, Harizopoulos & Ailamaki) and abandoned. Threads should be sized to **hardware +parallelism** and switched at **blocking points**, not at architectural boundaries. + +A handoff between layers gains no parallelism: the caller has nothing to do but wait for the callee. +It costs a queue push/pop, a context switch, and the request's entire L1/L2 working set. A +`FetchPageRead` **hit** is on the order of 100ns of pointer chasing; a thread handoff is a few +microseconds. Staging would make the common case ~20x slower to solve a problem we do not have. + +The same reasoning rejects an I/O thread pool under `DiskManager`: offloading a blocking `pread` and +then blocking on the result is strictly worse than blocking directly. That trade only pays when the +caller has other work, which under thread-per-session it does not. + +### Consequences already baked in + +DD-002 is synchronous and blocking by construction — `cv_.wait` on `Loading`, `shared_mutex` +content latches, blocking `pread`, and `FetchPageRead` returning a `Result` by value +rather than a future. **The thread that calls into the buffer pool must be able to park in the +kernel.** That is a deliberate choice, not an oversight. + +## Threads vs. transactions + +> **Concurrency control exists because transactions overlap, not because threads overlap.** + +The two are unrelated, and the 1:1 correspondence in this model — one thread runs one transaction at +a time — is coincidental, not causal. One thread interleaving statements from two open transactions +still needs MVCC; a thousand threads with only one active transaction would need none. + +| | Latch | Lock / MVCC | +|---|---|---| +| Protects | a physical in-memory structure | logical database state | +| Held for | ns–µs | the whole transaction, possibly minutes | +| Scope | one page, one structure | many pages, many statements | +| On conflict | wait | wait, or read an older version | +| Deadlock | avoided by fixed ordering | detected, resolved by rollback | + +`Frame::mtx_` and `Frame::latch_` are latches. They make *reading the bytes* safe. They provide no +isolation whatsoever: + +- A `WritePageGuard` releases its latch when the statement ends — long before `COMMIT`. In that gap + another session's `FetchPageRead` legally reads uncommitted data. Every mutex was respected; + ThreadSanitizer is silent; the read is still wrong. +- A scan touching 10,000 pages needs them **as of one instant**, or a concurrent transfer is counted + twice. Each page read is individually correct. Snapshot isolation is a guarantee across pages and + across time; a latch is a guarantee about one structure at one instant, and no quantity of + latching composes into the former. + +**Layering:** `BufferPoolManager` has no notion of a transaction and must keep it that way. It hands +out page bytes; it has no opinion on which tuples inside them a caller may see. Every tuple access is +two questions at two layers — the guard makes reading the bytes safe, the visibility check against +the caller's snapshot decides whether the tuple is theirs to see. + +## Disciplines (these keep the model swappable) + +The migration path, if concurrency ever demands it, is to replace *one thread per session* with *one +task per query on a fixed pool sized to cores*. That works only if the layers below were never +thread-aware. Three rules, enforced from now: + +1. **No `thread_local` engine state.** Transaction context, resource ownership, and snapshots are + passed explicitly down the stack. A `thread_local` current-transaction pointer welds the engine to + thread-per-session permanently — this is exactly what makes Postgres's `MyProc` / + `CurrentResourceOwner` globals so hard to move off processes. +2. **A page guard never outlives the function that acquired it.** Short, stack-scoped, released + before returning up a layer. A guard held across an unbounded call is a latch held across an + unbounded call. +3. **A pin never spans a client round-trip.** An open cursor holding pinned pages while the client + decides whether to fetch more rows is pool exhaustion by a slow network. Materialize a batch or + re-fetch on resume. + +## Where separate threads do belong + +Not per layer — per **background activity with a different cadence**. These are off the request path, +so they are not handoffs: + +- **WAL writer / group commit.** Not optional once a log exists: batching many transactions' fsyncs + into one fundamentally requires a thread that is none of them. +- **Checkpointer.** +- **MVCC garbage collection** — reclaiming versions no snapshot can see. MVCC without GC is a + slow-motion disk leak. +- **Background page cleaner.** A DD-002 non-goal today; the first candidate once write throughput + matters, since a foreground miss stalling on a dirty victim's writeback is the obvious stall. + +## Shutdown + +Shutdown is a phase the owner drives, not an operation workers participate in — a worker cannot know +it is the last one, and a flush failure is a decision (exit code, clean-shutdown marker) that no +worker can make or propagate. + +1. **Stop accepting work.** Close the request queue; workers finish their current unit and exit. +2. **Join every worker.** This is what produces the quiescence `Shutdown()` requires: guards are + stack-scoped, so an unwound stack has released every pin, and `join()` supplies the + synchronizes-with edge. +3. **`BufferPoolManager::Shutdown()`**, then `DiskManager::Sync()`, single-threaded, checked. +4. Destroy. + +Postgres does exactly this — backends never checkpoint on exit; the postmaster reaps every child and +only then runs the shutdown checkpoint. InnoDB's phased `srv_shutdown` is the same shape. + +**Stopping work is not the pool's business.** A `stopping_` flag inside `BufferPoolManager` that +`FetchPage` checks would put an atomic load and a branch on the hottest path in the engine to handle +a condition that occurs once per process lifetime. + +## Non-goals + +- Async/coroutine execution. The cost is not plumbing, it is guard lifetime: a coroutine suspended + while holding a pinned, latched page can resume after an arbitrary delay, and two such tasks + pinning pages in different orders is a deadlock this design has no detector for. Under + thread-per-session, latch hold times are bounded by function duration; under async, by the + scheduler. Revisit only with a measured reason. +- Intra-query parallelism (parallel scan/join, DuckDB-style morsel scheduling). A separate axis from + the connection model. Deferred, but not precluded: it requires only that the buffer pool be safe + under concurrent access from several threads on behalf of one transaction, which DD-002 already is + — nothing there assumes a pin's owner is a particular thread. +- Scaling to very high connection counts. That is a connection-pooler problem (pgbouncer), not an + engine-architecture problem. +- Thread-per-core / shared-nothing partitioning (ScyllaDB, Umbra). diff --git a/design-docs/notes/Buffer-Pool-Manager.md b/design-docs/notes/Buffer-Pool-Manager.md new file mode 100644 index 0000000..7e93107 --- /dev/null +++ b/design-docs/notes/Buffer-Pool-Manager.md @@ -0,0 +1,116 @@ +Concepts behind [DD-002](../DD-002-buffer-pool-manager.md). See [[Concurrency-Control]] for the +locks/latches/sharding background this leans on. + +## What a buffer pool actually is +- Disk is slow, memory is fast — a buffer pool is just a fixed-size in-memory cache of disk + pages, so repeated access to the same page doesn't repeat the I/O. +- **Frame**: a fixed-size slot in the buffer pool that currently holds (or could hold) one + page's worth of bytes, plus bookkeeping about that page. "Frame" and "page" are often + conflated casually, but the distinction matters: a page is a disk-format concept + (`PAGE_SIZE` bytes, identified by `page_id`), a frame is the in-memory *container* for one, + identified by its `frame_id` (just its index in the pool array). +- The whole pool is typically one contiguous allocation (`capacity × sizeof(Frame)`) rather than + one allocation per frame — better cache locality, and often required anyway once frames own + non-movable things like mutexes/condvars. + +## Page table +- A map `page_id → frame_id`: "if this page is currently cached, which frame is it in." +- Distinct from the **page directory** (on-disk `page_id → file offset`, DiskManager's concern) + — the page table is purely in-memory and only exists to make cache lookups fast. +- Sharded in kernSQL (see [[Concurrency-Control]]) so lookups for unrelated pages don't + serialize against each other. + +## Pin count +- A page is "pinned" while some caller is actively using it — pin count is a reference count. +- ==Core invariant: a frame is only eligible for eviction when its pin count is zero.== Eviction + candidacy is a direct function of this count, not a separate flag someone remembers to set. +- Pinning/unpinning is the mechanism that prevents "the buffer pool evicted a page while I still + had a pointer into it" — a classic use-after-free class of bug in naive implementations. + +## Dirty flag +- Set when a page's content has been modified since it was last written back to disk (or since + it was loaded, if never written). +- Only dirty pages need a disk write on eviction/flush — clean pages can just be dropped, since + disk already has an identical copy. +- Concurrency wrinkle: if two threads both hold a pin on the same frame and one modifies it, the + dirty flag has to be **OR'd**, not overwritten — you can't let a clean unpinner race with a + dirty unpinner and accidentally clear the flag on a page that's actually dirty because of the + *other* pinner's write. + +## Replacement policy (which page to evict on a miss with a full pool) +- **LRU (least recently used)**: evict the page that hasn't been touched longest. Great hit + ratio in practice, but naive implementations need a doubly-linked list reordered on *every* + access, which itself becomes a point of contention under concurrency — the exact convoy + problem this whole design is trying to avoid. +- **Clock / second-chance** (what kernSQL uses): approximates LRU without reordering on every + access. Each frame has a `usage_count` (or a single reference bit in the classic version); a + "clock hand" sweeps candidate frames, decrementing/clearing the bit as it passes, and evicts + the first frame it finds already at zero. A frame that was accessed gets its bit set again on + next access, giving it a "second chance" before eviction. + - Why this over strict LRU here: O(1) amortized, no reordering of a shared list on the hot + read path — accesses only need to bump a counter (or set a bit) on *their own* frame, not + touch a shared structure. That's a much smaller, more local piece of contention. +- **LRU-K / LRU-2**: tracks the *k*-th most recent access, not just the most recent — resists + "one-off scan pollutes the cache" better than plain LRU (a single sequential scan touches many + pages exactly once, which plain LRU would otherwise treat as "recently used" and protect from + eviction ahead of genuinely hot pages). Not used here; noted as the thing you'd reach for if + clock-sweep hit ratio proves insufficient in practice. + +## Free list vs. Replacer — two different populations of frames +- **Free list**: frames that have *never* held a page yet, or were just vacated by an explicit + delete. Nothing worth evicting — just an empty slot. Simple LIFO stack, fully populated at + construction time. +- **Replacer**: frames that *currently hold a page* but are unpinned — i.e. real eviction + candidates, tracked by the replacement policy above. +- A frame moves into the replacer's candidate set the instant its pin count drops to zero, and + out of it the instant it's pinned again. +- ==Why bother with two structures instead of just running the replacement policy on + everything?== Popping an empty free-list slot is unconditionally O(1) and touches nothing else + — during pool warm-up (before every frame has held a page at least once), this avoids running + a clock sweep at all. The miss path checks free list first, only consults the replacer once + it's empty. + +## Why frame metadata and frame *content* need separate protection +- Two very different operations touch a frame: "is this pinned, is it dirty, is it still + loading" (nanoseconds) vs "read/write the actual page bytes" (can be held across a whole scan, + potentially milliseconds). +- Protecting both with one latch means the fast metadata check can get stuck behind a slow + content access — a convoy (see [[Concurrency-Control]]). Splitting them into a metadata + mutex and a content R/W latch means a pin-count check never blocks on someone else's page + scan. + +## Loading flag — coordinating a cache miss without holding a latch across I/O +- ==A spin-or-short-block latch must never be held across a blocking syscall== — that would + turn every other thread waiting on that latch into a thread effectively blocked on disk I/O, + defeating the entire point of having fine-grained latches. +- Instead: a `loading` bool + condition variable per frame. The thread that causes the miss sets + `loading = true`, releases all latches, performs the disk read *unlocked*, then re-acquires the + metadata mutex, sets `loading = false`, and notifies waiters. +- Any other thread that looks up the same page while it's loading sees `loading == true`, and + waits on the condvar instead of either (a) racing to issue a redundant read, or (b) reading + stale/uninitialized content. + +## What's deliberately *not* on the frame +- `frame_id` — purely positional (its own index in the pool array); storing a copy risks it + drifting out of sync with reality for zero benefit. +- `page_lsn` — belongs to WAL/recovery, which doesn't exist yet in kernSQL. Adding the field + now would just be dead weight until recovery design actually starts. +- `usage_count` — belongs to the *replacer's* bookkeeping, not the frame itself. Keeps "what + page is this and how do I access it" (Frame's job) separate from "how do we decide what to + evict" (Replacer's job) — lets the replacer be swapped out (LRU-K instead of clock, say) + without touching Frame at all. + +## Control flow shape (general pattern, not kernSQL-specific wording) +- **Hit path**: find frame via page table → bump pin count → tell the replacer this frame was + just accessed / is no longer evictable → hand back a frame handle. Caller separately takes the + content latch for whatever it actually wants to do. +- **Miss path**: get a free frame (free list, else evict via replacer) → if the victim is dirty, + flush it first → update the page table (remove old mapping, insert new) → mark `loading` → + read from disk *without* holding the page-table lock → clear `loading`, notify waiters. +- Inserting the new page-table mapping *before* releasing the lock (but before the read + completes) matters: it's what makes a second concurrent fetcher of the *same* new page find + the in-progress entry and wait on `loading`, instead of independently also triggering a second + read of the same page. +- **Eviction of a dirty victim only needs a *shared* content latch**, not exclusive — because + pin count already being zero means there's no active writer to race with; a flush is "just + another reader" of the content. diff --git a/design-docs/notes/Concurrency-Control.md b/design-docs/notes/Concurrency-Control.md new file mode 100644 index 0000000..e88386f --- /dev/null +++ b/design-docs/notes/Concurrency-Control.md @@ -0,0 +1,84 @@ +## Locks vs Latches +- **Locks**: high-level, protect *logical/transactional* state (a row, a table, the DB) for the + duration of a transaction. Have deadlock detection (wait-for graphs, timeouts) because + transactions are allowed to block on each other for a while — that's expected. +- **Latches**: low-level, protect *physical in-memory* structures (a frame's bytes, a pin count, + a linked-list pointer). Short-duration by design. ==No deadlock detection== — a latch acquire + is supposed to be so fast that you just avoid deadlock structurally (global ordering) instead + of detecting and breaking it after the fact. +- Rule of thumb: if it could plausibly be held while waiting on a disk I/O or another + transaction, it's a lock. If it's only ever held across a few instructions of pointer/counter + manipulation, it's a latch. + +## Why not just one mutex protecting everything +- An OS mutex (`std::mutex`, ultimately a futex) is nearly free when uncontended, but on + contention the losing thread **sleeps** and is woken by the kernel in scheduler order — with + zero awareness of whether the critical section it's waiting on is 3 instructions or 3ms. +- If one lock protects things with very different hold times (e.g. "check a pin count" vs "scan + a page"), the cheap operation gets stuck behind the expensive one purely because they share a + lock. This pile-up is called a **convoy**. +- Fix: don't use one lock per shared structure — split into *multiple independent mechanisms, + each sized to what it actually protects*, so a fast path never queues behind a slow one just + because they happen to touch the same object. +- ==This is the actual reason commercial engines (Postgres, InnoDB) have such elaborate + locking hierarchies — it's not over-engineering, it's convoy avoidance at scale.== + +## Latch implementation flavors +- **Spinlock**: busy-wait loop, no syscall, no context switch. Cheap only if the hold time is + shorter than a context-switch would cost — otherwise it just burns CPU while achieving + nothing. Never spin across anything that can block (syscalls, I/O, another lock acquire). +- **OS mutex / futex**: sleeps on contention, cheap uncontended, but pays a syscall + context + switch + scheduler-order wakeup on contention. Good for hold times that are unpredictable or + can be long. +- **Hybrid spin-then-block** (Postgres `LWLock` style): spin briefly first (covers the common + case where the holder releases almost immediately), fall back to a real block if that fails. + Gets uncontended-cheap *and* avoids CPU-burning under real contention. This is what kernSQL's + frame content latch uses instead of a raw `pthread_rwlock_t` — using a hand-rolled wrapper + buys control over the exact wait/wake path. + +## Reader/Writer latches +- Shared (read) mode: many readers concurrently. Exclusive (write) mode: one writer, no + readers. Standard trade-off — read-heavy workloads (most page accesses are reads) benefit a + lot from not serializing readers against each other. +- Can be legitimately held for a *relatively* long span (e.g. across a whole page scan) as + long as it's a **content** latch and not a metadata latch — this is why kernSQL splits these + into two separate primitives per frame rather than one (see Buffer Pool Manager notes). + +## Condition variables need a real mutex +- An atomic word (flag + spin) cannot implement wait/notify — there's no way for a thread to + "sleep until notified" on a bare atomic; it can only poll, which burns CPU. +- `std::condition_variable` requires pairing with an actual `std::mutex` to avoid the + classic missed-wakeup race (check condition → mutex unlocked → notify happens → then you + wait forever). This is the concrete reason a "just use an atomic" scheme for coordinating + "is this resource ready yet" waiters doesn't work once you actually need blocking waiters, + not just spinners. + +## Deadlock avoidance for latches (lock ordering) +- Because latches have no deadlock detection, the only way to prevent deadlock once a thread + might hold more than one at a time is a **canonical global acquisition order** — every thread, + every code path, always acquires latches in the same relative order. +- If two code paths acquire the same two latches in *opposite* order, that's a deadlock waiting + to happen the instant they interleave — doesn't matter how rare, it will eventually fire under + load. ==This is exactly the kind of bug that's invisible in single-threaded testing and shows + up as a production hang under concurrency.== +- Corollary: any operation that legitimately needs to hold two latches of the *same kind* + simultaneously (e.g. two different page latches) must pick a deterministic order between them + (e.g. by numeric id), never an order that depends on caller/argument order. + +## Sharding as a concurrency technique +- Splitting one global structure (e.g. a `page_id → frame_id` map) into `N` independently-locked + shards means lookups for unrelated keys never serialize against each other — contention drops + roughly by a factor of `N` for uniformly-distributed keys. +- Shard count is usually a small fixed power of two rather than derived from core count at + runtime — simpler, no CPU-detection dependency, and easy to bump later if profiling shows + contention. +- Hash function only needs to be as strong as the key distribution requires — sequential integer + keys (like auto-incrementing page ids) already spread evenly across shards with a plain + bitmask (`key & (N-1)`); a fancier hash (multiplicative, Fibonacci) is solving a problem that + doesn't exist for that access pattern. +- Cost: any operation that needs to move an entry *between* shards (e.g. changing what a key + maps to when the shard boundary matters) has to acquire multiple shards, in ascending order, + and re-validate state after acquiring the second one — someone else could have changed things + while only the first shard lock was held. + +See also: [[Buffer-Pool-Manager]] for how kernSQL actually applies all of this. diff --git a/src/buffer/buffer_pool_manager.cpp b/src/buffer/buffer_pool_manager.cpp new file mode 100644 index 0000000..bec89ad --- /dev/null +++ b/src/buffer/buffer_pool_manager.cpp @@ -0,0 +1,686 @@ +#include "buffer_pool_manager.hpp" + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "buffer/frame.hpp" +#include "buffer/freelist.hpp" +#include "buffer/pool_stats.hpp" +#include "buffer/replacer.hpp" +#include "common/logger.hpp" +#include "common/page_header.hpp" +#include "common/status.hpp" +#include "common/types.hpp" + +using namespace kernsql; + +BufferPoolManager::BufferPoolManager(DiskManager& disk_manager, std::size_t capacity) + : frames_(std::make_unique(capacity)), + capacity_(capacity), + free_list_(capacity), + replacer_(capacity), + disk_manager_(disk_manager) { + assert(capacity > 0); +} + +BufferPoolManager::~BufferPoolManager() { + if (shutdown_) return; + LOG_INFO("Shutdown() was never called; flushing from ~BufferPoolManager()"); + Status st = Shutdown(); + if (!st.ok()) { + // %.*s, not %s: message() is a std::string_view — a pointer/length pair, not a + // null-terminated char*. Passing it to a printf-style variadic is undefined even though + // the view happens to be backed by a std::string today. + const auto msg = st.message(); + LOG_INFO("shutdown flush failed in ~BufferPoolManager, aborting: %.*s", + static_cast(msg.size()), msg.data()); + std::abort(); + } +} + +Status BufferPoolManager::FlushPage(page_id_t page_id) { + frame_id_t frame_id; + { + auto shard = page_table_.AcquireShard(page_id); + auto found = shard.Find(page_id); + if (!found) return Status::OK(); // nothing cached so no Flush required + frame_id = *found; + + auto& frame = FrameAt(frame_id); + std::lock_guard lock(frame.mtx_); + if (frame.state_ != FrameState::Resident) return Status::OK(); + frame.pin_count_ += 1; + } + + Status st = FlushFrame(frame_id, page_id); + UnpinPage(frame_id); + return st; +} + +Status BufferPoolManager::FlushAllPages() { + Status st = Status::OK(); + const auto n = static_cast(capacity_); + + for (frame_id_t frame_id = 0; frame_id < n; ++frame_id) { + auto& frame = FrameAt(frame_id); + + page_id_t page_id; + { + std::lock_guard lock(frame.mtx_); + if (frame.state_ != FrameState::Resident) continue; + page_id = frame.page_id_; + frame.pin_count_ += 1; + } + + auto s = FlushFrame(frame_id, page_id); + UnpinPage(frame_id); + if (st.ok() && !s.ok()) st = s; // cache the first error + } + return st; +} + +Status BufferPoolManager::FlushFrame(frame_id_t frame_id, page_id_t page_id) { + auto& frame = FrameAt(frame_id); + auto st = Status::OK(); + if (!frame.IsDirty()) return st; + + uint64_t D; + { + auto rd_latch = frame.AcquireRead(); + D = frame.SampleDirtyEpoch(); + st = disk_manager_.WritePage(page_id, frame.data_); + } + if (!st.ok()) return st; + frame.MarkFlushed(D); + + return st; +} + +Status BufferPoolManager::Shutdown() { + if (shutdown_) return Status::OK(); + shutdown_ = true; // the attempt has been made + + LOG_INFO("BufferPoolManager::Shutdown() starting"); + + // Verify that each frame is unpinned and not on FrameState::Loading + const auto n = static_cast(capacity_); + for (frame_id_t frame_id = 0; frame_id < n; frame_id++) { + auto& frame = FrameAt(frame_id); + { + std::lock_guard lock(frame.mtx_); + if (frame.pin_count_ != 0 || frame.state_ == FrameState::Loading) { + LOG_INFO("frame %d is not quiesced: pin_count = %d, state = %s", frame_id, + frame.pin_count_, FrameStateName(frame.state_)); + } + assert(frame.pin_count_ == 0 && frame.state_ != FrameState::Loading); + } + } + + Status st = FlushAllPages(); + Status sync = disk_manager_.Sync(); // unconditional + if (st.ok()) st = sync; + return st; +} + +PoolStats BufferPoolManager::GetStats() const { + PoolStats stats{}; + stats.capacity = capacity_; + + const auto n = static_cast(capacity_); + for (frame_id_t frame_id = 0; frame_id < n; ++frame_id) { + const auto& frame = FrameAt(frame_id); + + // One frame's mutex at a time, released before the next is taken. Never two at once, + // and no shard lock anywhere near this — a census that held locks across frames would + // be a new lock-ordering edge for a diagnostic, which is a bad trade. + std::lock_guard lock(frame.mtx_); + switch (frame.state_) { + case FrameState::Free: + ++stats.free_frames; + break; + case FrameState::Loading: + ++stats.loading_frames; + break; + case FrameState::Resident: + ++stats.resident_frames; + break; + case FrameState::Failed: + ++stats.failed_frames; + break; + } + if (frame.pin_count_ > 0) ++stats.pinned_frames; + } + + // AFTER the loop, deliberately: DD-002 forbids calling into the free list or the replacer + // while holding a frame's metadata mutex. Both take their own leaf lock, so reading them + // here adds no ordering constraint. + stats.free_list_size = free_list_.Size(); + stats.evictable = replacer_.EvictableCount(); + + return stats; +} + +void BufferPoolManager::UnpinPage(frame_id_t frame_id) { + auto& frame = FrameAt(frame_id); + + std::lock_guard lock(frame.mtx_); + + if (frame.state_ == FrameState::Failed) frame.cv_.notify_all(); + assert(frame.pin_count_ > 0); + + const bool evictable = ((frame.pin_count_ == 1) && (frame.state_ == FrameState::Resident)); + frame.pin_count_ -= 1; + if (frame.pin_count_ < 0) { + LOG_INFO("frame %d pin count is dropped to %d", frame_id, frame.pin_count_); + } + + // Under the metadata mutex, and it is the ONE exception to DD-002's "never call into the + // replacer while holding a frame's metadata mutex" (see "Lock ordering", which now names it). + // + // Publishing this outside the mutex splits the decision from the act, and DeletePage fits in + // the gap: it takes the mutex, sees the pin_count == 0 we just produced, vacates the frame, + // calls SetEvictable(false) -- which removes nothing, because our `true` has not landed yet -- + // and pushes the frame to the free list. Our `true` then arrives at a frame that is now free + // stock, leaving it on the free list and in the candidate set at once. That is exactly the + // double membership DeletePage's own comment says must never exist, and only ReclaimFrame's + // re-validation of `state == Resident` keeps it from becoming two threads on one frame. + // + // The 0->1 direction in FetchFrame needs no such treatment and keeps the deferred form. It is + // not symmetric: the only thing that ADDS a frame to the candidate set is an unpin, and in + // that window the caller holds the pin, so no unpin can happen and no straggler can undo the + // SetEvictable(false). + // + // Safe as an exception because Replacer is a strict leaf -- every method takes only its own + // mutex and never reaches back into a frame, a shard, or the free list. The edge this adds is + // metadata -> replacer, and nothing anywhere goes the other way. + if (evictable) replacer_.SetEvictable(frame_id, true); +} + +Result BufferPoolManager::FetchFrame(page_id_t page_id) { + // A loop, not straight-line code: the miss path releases the shard lock to acquire a frame, + // and another thread can publish this same page in that window. The only correct response is + // to hand our frame back and start over from the lookup. + for (;;) { + // -1 rather than left indeterminate: every path below assigns it before use, and if one + // ever stops doing so, FrameAt's bounds assert fires immediately instead of silently + // indexing an unrelated frame. + frame_id_t frame_id{-1}; + + // Whether *we* are the pin that took this frame from 0 to 1. Only that thread owes the + // replacer a SetEvictable(false); a frame already pinned by someone else left the + // candidate set when they pinned it. + bool from_zero{false}; + + // Declared HERE, outside the block below, and this placement is the whole trick. The + // lookup must hold the shard lock and the metadata mutex simultaneously — dropping the + // shard lock before pinning would let a reclaimer take the frame between Find() and the + // increment. But the Loading wait must NOT hold the shard lock (DD-002: never block on a + // condvar while holding one), and ShardGuard exposes no early unlock. Declaring `meta` + // first means the ShardGuard dies at the end of the block while this lock lives on. + // + // unique_lock rather than lock_guard because cv_.wait() requires one. + std::unique_lock meta; + { + auto shard = page_table_.AcquireShard(page_id); + auto found = shard.Find(page_id); + if (found) { + frame_id = *found; + auto& frame = FrameAt(frame_id); + meta = std::unique_lock(frame.mtx_); // shard -> metadata, the canonical order + + // Pin BEFORE inspecting state_ — the pin invariant, not an optimization. The + // pin makes AbandonLoad (which waits for pin_count == 1, its own) unable to + // dispose of this frame, so its identity is fixed across the wait below and + // nothing has to be re-validated on wake. + from_zero = (frame.pin_count_ == 0); + frame.pin_count_ += 1; + } + } // shard released here; `meta` survives if we hit + + // owns_lock() is the hit/miss discriminator: we take the metadata mutex only on a hit. + if (meta.owns_lock()) { + auto& frame = FrameAt(frame_id); + + // Predicate form handles both a spurious wakeup and the already-published case. + frame.cv_.wait(meta, [&] { return frame.state_ != FrameState::Loading; }); + + // Free is unreachable: a frame is unmapped in the same critical section that turns it + // Free, so the shard lookup above could not have returned it — and our pin has kept + // it that way ever since. + assert(frame.state_ == FrameState::Resident || frame.state_ == FrameState::Failed); + + if (frame.state_ == FrameState::Failed) { + // The loader's read failed and it owns disposal; we only drop the pin we took. + // unlock() first is mandatory, not tidiness: UnpinPage takes this same mutex and + // std::mutex is not recursive. UnpinPage is also what notifies the loader that a + // waiter has left, which is how AbandonLoad's pin_count == 1 wait terminates. + meta.unlock(); + UnpinPage(frame_id); + return std::unexpected( + Status::IOError(std::format("unable to fetch frame {}", frame_id))); + } + + // Release before touching the replacer — DD-002 forbids calling into it while holding + // a frame's metadata mutex. This admits a brief window where the frame is still marked + // evictable despite being pinned; that is harmless because reclaim re-validates + // pin_count == 0 under the metadata mutex and simply declines. + meta.unlock(); + replacer_.RecordAccess(frame_id); + if (from_zero) replacer_.SetEvictable(frame_id, false); + return frame_id; + } + + // --- Cache miss. We reach here holding NO lock, which is required rather than incidental: + // AcquireFrame may reclaim, and re-validating a victim needs the *victim's* shard lock, + // about which our own shard lock says nothing. + + auto found_frame = AcquireFrame(); + if (!found_frame.has_value()) { + // kBufferPoolFull, propagated. Nothing was acquired, so nothing is owed. + return std::unexpected(found_frame.error()); + } + // Free, unmapped, unpinned, and out of both the free list and the candidate set — private + // to this thread until we publish it below. + frame_id = found_frame.value(); + + { + auto shard = page_table_.AcquireShard(page_id); + + // Look up AGAIN. We had no shard lock across AcquireFrame, so another thread may have + // published this page in the meantime. Loading it twice would give two frames the same + // identity. + auto found = shard.Find(page_id); + if (found.has_value()) { + // Lost the race. Our frame goes back to the FREE LIST, not the replacer: it holds + // no page, and a Free frame is stock rather than an eviction candidate. Pushing + // under the shard lock is safe — the free-list lock is a leaf that nothing is ever + // held across, so no cycle is possible. + free_list_.Push(frame_id); + continue; + } + + // Publish the mapping BEFORE the read. This is what makes a concurrent fetcher of this + // page take the hit path and sleep on Loading instead of starting a second, redundant + // read of the same page into a second frame. + shard.Insert(page_id, frame_id); + auto& frame = FrameAt(frame_id); + + // A scoped lock, deliberately NOT the function-scope `meta`: that one outlives this + // block, so assigning to it would leave the metadata mutex held across the read below + // — and the re-lock after the read would then deadlock against ourselves. + std::lock_guard lock(frame.mtx_); + frame.page_id_ = page_id; + frame.state_ = FrameState::Loading; + frame.pin_count_ = 1; // a store, not an increment: the frame was Free and ours alone + frame.ResetEpochs(); // stale epochs from the previous occupant would read as dirty + } // both released + + // No lock and no content latch. Safe because the frame is Loading: no guard for it can + // exist (guards only come from this function returning), and every other fetcher of this + // page is asleep on the condvar rather than touching data_. + auto& frame = FrameAt(frame_id); + auto st = disk_manager_.ReadPage(page_id, frame.data_); + if (!st.ok()) { + // The loader owns disposal. AbandonLoad erases the mapping, publishes Failed, waits + // for the waiters' pins to drain, and returns the frame to the free list. Do not unpin + // or push here as well — that would be a double release. + AbandonLoad(frame_id, page_id); + return std::unexpected(st); + } + + // Validate what came back BEFORE publishing it. Effectively free: the 4 KB read already + // happened, so this is two comparisons against bytes already in cache. Deliberately not + // done on the hit path — a resident page was validated when it missed. + // + // FREE catches a fetch of a deallocated page. That is a caller bug (nothing should hold + // a page id after deleting it), but without the check the page is silently re-cached, + // and the next AllocatePage to recycle that id finds a stale mapping — two frames + // claiming one page. It narrows rather than closes the window: a fetch that lands + // between DeletePage's mapping erase and its DeallocatePage still reads the old header + // and passes. The full guarantee has to come from the layer above never asking for a + // page it deleted. + // + // A page_id mismatch catches a different class: a misdirected write, an off-by-one in + // page arithmetic, a torn extend, a file copied at the wrong offset. The offset says + // which page we asked for and the header says which page the bytes think they are — + // only two independently derived answers can disagree, which is the whole reason the + // id is stored despite being implied by the offset. + const auto header = PageHeader::ReadFrom(std::span(frame.data_).first()); + if (header.page_type == PageType::FREE || header.page_id != page_id) { + Status bad = + (header.page_type == PageType::FREE) + ? Status::InvalidArgument(std::format("page {} is not allocated", page_id)) + : Status::Corruption(std::format("page {} header claims to be page {}", page_id, + header.page_id)); + // Same disposal path as a read error: the frame is Loading, mapped and pinned, and + // may have waiters asleep on it. Waiters get the generic Failed-path error rather + // than this one — a pre-existing property of that path, not new here. + AbandonLoad(frame_id, page_id); + return std::unexpected(bad); + } + + { + // Publish the contents. notify_all under the mutex so no waiter can check the + // predicate and sleep between our store and the notification. + std::lock_guard lock(frame.mtx_); + frame.state_ = FrameState::Resident; + frame.cv_.notify_all(); + } + + // No SetEvictable(false) on this path: the frame came from the free list or a reclaim, so + // it was never in the candidate set to begin with. + replacer_.RecordAccess(frame_id); + return frame_id; // pin_count == 1, and it is the caller's + } +} + +Result BufferPoolManager::AcquireFrame() { + auto frame_id = free_list_.TryPop(); + if (frame_id.has_value()) return frame_id.value(); + return ReclaimFrame(); +} + +Result BufferPoolManager::ReclaimFrame() { + for (;;) { + auto victim = replacer_.Evict(); + if (!victim.has_value()) { + return std::unexpected(Status::BufferPoolFull("buffer full")); + } + + auto& frame = FrameAt(victim.value()); + + // Peek the victim's identity. A shard is keyed by page_id, which lives under the + // metadata mutex — and the canonical order forbids taking a shard lock while holding + // one. So read it here, release, and lock in order below. This value is nothing but a + // hint about WHICH shard to take; every predicate tested here is tested again under + // both locks, page_id_ included. + page_id_t peeked{INVALID_PAGE}; + { + std::lock_guard lock(frame.mtx_); + if (frame.state_ != FrameState::Resident || frame.pin_count_ != 0) + continue; // Delete page raced this thread + peeked = frame.page_id_; + } + + // Scoped, because ShardGuard has no early unlock: the dirty path below must reach its + // flush with the shard lock already gone, and the end of this block is the only thing + // that releases it. Falling out of the block therefore means exactly one thing — the + // victim was dirty and we pinned it. Every other outcome returns or continues from + // inside. + { + auto shard = page_table_.AcquireShard(peeked); + std::lock_guard lock(frame.mtx_); + + // Re-validate: the metadata mutex was released across the peek, so the frame may + // have been re-pinned, deleted, or repurposed into a different page since. A + // declined victim is a normal outcome, not an error — it stays out of the + // candidate set until its next unpin re-adds it. + if (frame.state_ != FrameState::Resident || frame.pin_count_ != 0 || + frame.page_id_ != peeked) + continue; + + // Implied rather than checked: mappings change only under the shard lock we now + // hold, and every path that erases peeked's mapping leaves Resident in that same + // critical section. A mismatch means an invariant broke elsewhere. + assert(shard.Find(peeked) == victim); + + // The clean-check is exact, not conservative. Nothing can newly dirty the frame — + // that needs a guard, which needs the mapping we hold the shard lock for — and + // nothing already dirtied is missed, since a guard's release bumps dirty_epoch_ + // strictly before it decrements pin_count_. + if (!frame.IsDirty()) { + // The entire reclaim, with no I/O in it: one critical section, so no other + // thread can observe a frame mid-reclaim and there is nothing to cancel. + shard.Erase(peeked); + frame.page_id_ = INVALID_PAGE; + frame.state_ = FrameState::Free; + frame.ResetEpochs(); // stale epochs would read as dirty for the next occupant + return victim.value(); + } + + // Dirty. Pin across the writeback: the pin, not any lock, is what fixes the + // frame's identity while we hold no lock at all. The mapping stays live, so a + // concurrent fetcher of this page takes the hit path and pins — and we see + // pin_count_ > 1 at re-validation below and decline. + frame.pin_count_ += 1; + } // both released + + Status st = FlushFrame(victim.value(), peeked); + if (!st.ok()) { + // Deliberately NOT another trip round the loop. UnpinPage hands this frame back to + // the candidate set on its 1->0 transition, so the next Evict() can return it + // straight back to us — flush, fail, repeat. A persistently unwritable page would + // turn reclaim into a livelock rather than an error, so drop the pin and let the + // caller's fetch fail with the I/O error it actually hit. + UnpinPage(victim.value()); + return std::unexpected(st); + } + + { + auto shard = page_table_.AcquireShard(peeked); + std::lock_guard lock(frame.mtx_); + + // Same checks, with two changes: pin_count_ == 1 is now the condition, that one + // being ours, and IsDirty() is asked again because a writer can have dirtied the + // page while the write was in flight. + if (frame.state_ == FrameState::Resident && frame.page_id_ == peeked && + frame.pin_count_ == 1 && !frame.IsDirty()) { + assert(shard.Find(peeked) == victim); + shard.Erase(peeked); + frame.page_id_ = INVALID_PAGE; + frame.state_ = FrameState::Free; + frame.ResetEpochs(); + + // Our own pin dies as part of the same stamp that unmaps the frame, NOT + // through UnpinPage. This is the narrow exception to "UnpinPage is the only + // place a pin is decremented" — that rule is about releasing a pin while the + // frame keeps its identity. Here the frame is becoming free-list stock, and + // UnpinPage would publish it to the replacer as an eviction candidate on the + // 1->0 transition, so TryPop and Evict could hand the same frame to two + // threads. AbandonLoad drops its pin the same way and for the same reason. + frame.pin_count_ = 0; + return victim.value(); + } + } // both released + + // Lost it during the writeback — someone pinned it, or dirtied it again. Wasted work, + // nothing corrupted (DD-002, "Known behaviour"). UnpinPage only after both locks are + // gone: it may call into the replacer, and DD-002 keeps the replacer lock off any path + // that already holds a shard lock or a metadata mutex. + UnpinPage(victim.value()); + } +} + +void BufferPoolManager::AbandonLoad(frame_id_t frame_id, page_id_t page_id) { + { + auto shard = page_table_.AcquireShard(page_id); + shard.Erase(page_id); + } + + auto& frame = FrameAt(frame_id); + { + std::unique_lock lock(frame.mtx_); + frame.state_ = FrameState::Failed; + frame.cv_.notify_all(); + + assert(frame.page_id_ >= 1); + frame.cv_.wait(lock, [&] { return frame.pin_count_ == 1; }); + + frame.page_id_ = INVALID_PAGE; + frame.state_ = FrameState::Free; + frame.pin_count_ = 0; + frame.ResetEpochs(); + } + + free_list_.Push(frame_id); +} + +Result BufferPoolManager::FetchPageRead(page_id_t page_id) { + auto frame_found = FetchFrame(page_id); + if (!frame_found.has_value()) return std::unexpected(frame_found.error()); + + auto& frame = FrameAt(frame_found.value()); + + return ReadPageGuard(this, &frame, frame_found.value(), page_id, frame.AcquireRead()); +} + +Result BufferPoolManager::FetchPageWrite(page_id_t page_id) { + auto frame_found = FetchFrame(page_id); + if (!frame_found.has_value()) return std::unexpected(frame_found.error()); + + auto& frame = FrameAt(frame_found.value()); + + return WritePageGuard(this, &frame, frame_found.value(), page_id, frame.AcquireWrite()); +} + +Result BufferPoolManager::NewPage() { + auto frame_found = AcquireFrame(); + if (!frame_found.has_value()) { + return std::unexpected(frame_found.error()); + } + + auto page_allocated = disk_manager_.AllocatePage(); + if (!page_allocated.has_value()) { + free_list_.Push(frame_found.value()); // cleanup the acquired frame + return std::unexpected(page_allocated.error()); + } + + page_id_t page_id = page_allocated.value(); + + auto& frame = FrameAt(frame_found.value()); + { + // The frame is unreachable through the page table and the replacer, so no other + // FETCHER can find it -- but GetStats walks every frame BY INDEX and reads state_ and + // pin_count_ under this mutex, so "private to this thread" does not cover it. Without + // the lock these three stores race a concurrent census. FetchFrame's miss path takes + // the same lock for the same stores. + std::lock_guard lock(frame.mtx_); + frame.page_id_ = page_id; + frame.state_ = FrameState::Resident; + frame.pin_count_ = 1; + } + + // data_ needs no lock: nothing can reach this frame's bytes until the mapping is published + // below, and a guard only ever comes from a successful page-table lookup. + frame.data_.fill(std::byte{0}); + + // Stamp the page's own identity. Zeroing left page_id at 0 — which is META_PAGE_ID — and a + // page that lies about which page it is defeats the entire point of storing the id. + // DiskManager stamps this on every header it writes; NewPage is the one path that produces + // page contents without ever reading them, so it owes the same. page_type is ALLOCATED, the + // "handed out but not yet given a purpose" state (InnoDB spells it FIL_PAGE_TYPE_ALLOCATED), + // matching what AllocatePage already stamped on disk. Everything past the header stays the + // caller's business (DD-003) — this is identity, not semantics. + PageHeader header; + header.page_type = PageType::ALLOCATED; + header.page_id = page_id; + header.WriteTo(std::span(frame.data_).first()); + frame.ResetEpochs(); + + { + auto shard = page_table_.AcquireShard(page_id); + shard.Insert(page_id, frame_found.value()); + } + + replacer_.RecordAccess(frame_found.value()); + + return WritePageGuard(this, &frame, frame_found.value(), page_id, frame.AcquireWrite()); +} + +Status BufferPoolManager::DeletePage(page_id_t page_id) { + // Checked BEFORE anything destructive. DeallocatePage rejects these two ids, and reaching + // that rejection at the end of this function would mean we had already vacated the frame — + // evicting the catalog root from the pool and returning an error, for a call that could + // never have succeeded. Failing here leaves nothing to undo. + if (page_id == META_PAGE_ID || page_id == CATALOG_ROOT_PAGE_ID) { + return Status::InvalidArgument( + std::format("page {} is reserved and cannot be deleted", page_id)); + } + + // Held for the WHOLE operation, deallocation included, rather than released after the erase. + // That is the fix for the stale-mapping race (DD-002, "Known behaviour"): between erasing the + // mapping and stamping the header FREE, a fetcher that missed could publish a fresh Loading + // mapping for this page, read a header still marked ALLOCATED, pass the miss-path validation + // at FetchFrame's read check, and leave the pool caching a page that is on the disk freelist. + // The next AllocatePage then hands that id out again and the new owner's fetch is a cache hit + // on the dead page's bytes. Holding the shard is the whole guarantee: no new mapping for this + // page can be published until the header says FREE, and then the validation rejects it. + // + // This is the second named exception in DD-002's "Lock ordering": it blocks on I/O while + // holding a shard lock. Affordable because the I/O is bounded and this path is not hot — + // DeallocatePage is three syscalls with no fsync, and it already serializes every deleter and + // allocator in the process on DiskManager's global meta_latch_, so the shard lock adds only + // the other pages in this one shard, for a span the call already spends. New ordering edge, + // shard -> DiskManager::meta_latch_, closes no cycle: DiskManager knows nothing about the + // page table and can never reach back for a shard. + auto shard = page_table_.AcquireShard(page_id); + + // -1 means "not cached" — the page may legitimately have no frame, in which case there is + // nothing to vacate and only the disk side needs doing. + frame_id_t frame_id{-1}; + { + auto found = shard.Find(page_id); + + if (found.has_value()) { + frame_id = *found; + auto& frame = FrameAt(frame_id); + std::lock_guard lock(frame.mtx_); // shard -> metadata, the canonical order + + // Reject rather than wait. Waiting would deadlock outright when the caller is + // deleting a page it still holds a guard on, and nothing at this layer detects + // that. It also fires spuriously while a reclaimer holds a transient pin across a + // dirty victim's writeback (DD-002, "Known behaviour") — that case is retryable and + // this one is not, and a single status cannot tell the caller which it hit. + // + // The pin check subsumes a state check: Loading and Failed frames both carry at + // least the loader's own pin, so neither can reach the vacate below. + if (frame.pin_count_ != 0) { + return Status::InvalidArgument( + std::format("page {} is pinned ({} holders) and cannot be deleted", page_id, + frame.pin_count_)); + } + assert(frame.state_ == FrameState::Resident); + + // Dirty contents are discarded WITHOUT a flush, deliberately. Flushing would write + // bytes into a page we are about to stamp FREE, and race that stamp while doing it. + // This is the one place in the component where a dirty frame owes no writeback. + frame.page_id_ = INVALID_PAGE; + frame.state_ = FrameState::Free; + frame.ResetEpochs(); + shard.Erase(page_id); + } + } // frame metadata mutex released; the shard lock is still held, deliberately + + if (frame_id != -1) { + // Mandatory, and the only place in the component that needs it. A Resident frame with + // pin_count == 0 is in the replacer's candidate set by definition, so pushing it to the + // free list while it is still a candidate would leave it in both structures — and + // TryPop and Evict could then hand the same frame to two threads. Every other path that + // frees a frame gets one that already left the set: Evict removed ReclaimFrame's, and + // AbandonLoad's was never in it. + replacer_.SetEvictable(frame_id, false); + } + + // Vacated first, which is what satisfies DeallocatePage's not-resident contract + // (disk_manager.hpp). It stamps the on-disk header FREE directly, and a later flush of a + // still-resident frame would write the old header back over that stamp — leaving a page + // that is on the disk freelist and reachable as live data at the same time. Once the frame + // is Free, FlushAllPages skips it, so no flush can ever reach this page again. + // + // Still under the shard lock. See the acquisition above for why that matters. + Status st = disk_manager_.DeallocatePage(page_id); + + // Returned regardless of the deallocation result. The frame is already Free, unmapped and + // out of the candidate set, so it is usable stock either way; withholding it because the + // disk call failed would shrink the pool permanently for a reason that has nothing to do + // with the frame. + if (frame_id != -1) free_list_.Push(frame_id); + + return st; +} diff --git a/src/buffer/buffer_pool_manager.hpp b/src/buffer/buffer_pool_manager.hpp new file mode 100644 index 0000000..cdce665 --- /dev/null +++ b/src/buffer/buffer_pool_manager.hpp @@ -0,0 +1,212 @@ +#pragma once + +#include +#include +#include + +#include "buffer/frame.hpp" +#include "buffer/freelist.hpp" +#include "buffer/page_guard.hpp" +#include "buffer/page_table.hpp" +#include "buffer/pool_stats.hpp" +#include "buffer/replacer.hpp" +#include "common/status.hpp" +#include "common/types.hpp" +#include "storage/disk_manager.hpp" + +namespace kernsql { + +// The caching layer over DiskManager (DD-002). Hands out RAII page guards, never raw frames. +// +// The guards live in page_guard.hpp with this class forward-declared, and call the private +// UnpinPage on release; they are friends of this class. UnpinPage is deliberately not +// public — two owners of one pin is a double-decrement waiting to happen. +// +// There are no background threads. A miss pops free_list_, and when that is empty it reclaims +// a frame inline: replacer_.Evict() picks a victim, then one critical section under that +// victim's page-table shard lock and metadata mutex re-validates it (Resident, unpinned, +// clean, still mapped here) and turns it Free. A dirty victim is pinned, flushed with no lock +// held, and then re-validated. Because the reclaim itself performs no I/O, no other thread can +// observe a frame mid-reclaim and nothing needs to cancel it. +class BufferPoolManager { + public: + // Allocates the frame array. free_list_ stocks itself with every frame_id in its own + // constructor, so a fully-stocked free list is an invariant of that type rather than + // something this constructor has to remember to establish. Definitions still to be written. + BufferPoolManager(DiskManager& disk_manager, std::size_t capacity); + + // A backstop, NOT the durability mechanism — that is Shutdown(), below. + // + // If Shutdown() already ran, this does nothing. If it did not, the caller has a bug: a + // destructor cannot return a Status or throw, so it is structurally the wrong place to decide + // what a failed flush means. It logs, flushes best-effort, and aborts rather than silently + // discarding dirty pages. + // + // Requires quiescence, which it verifies rather than arranges (see Shutdown()). Nothing to + // join or signal — the pool owns no threads. + ~BufferPoolManager(); + + BufferPoolManager(const BufferPoolManager&) = delete; + BufferPoolManager& operator=(const BufferPoolManager&) = delete; + BufferPoolManager(BufferPoolManager&&) = delete; + BufferPoolManager& operator=(BufferPoolManager&&) = delete; + + // --- The frozen public API (DD-002, "Page guards") --- + + // Pin `page_id` and return it latched in the named mode. A hit pins before it waits, so the + // frame's identity is provably unchanged across the wait; a miss acquires a frame, publishes + // the mapping as Loading, and reads with no lock held. Errors: kIOError from the read, + // kBufferPoolFull when every frame is pinned. + // + // Two entry points rather than one mode flag: the mode is what picks the guard type, so a + // runtime parameter would force a return type that can hold either. + [[nodiscard]] Result FetchPageRead(page_id_t page_id); + [[nodiscard]] Result FetchPageWrite(page_id_t page_id); + + // Allocates a page on disk and returns it zeroed and write-latched. No Loading phase — there + // is nothing on disk to read — and no re-lookup race, since a page_id nobody has seen yet + // cannot already be mapped. + [[nodiscard]] Result NewPage(); + + // Writes the page back if dirty. Pins for the duration, so it is safe against a concurrent + // reclaim. Not an error if the page is absent or already clean. + Status FlushPage(page_id_t page_id); + + // A sequential loop over every frame. Best-effort and NOT atomic across frames: a page + // dirtied after its own frame has been visited is not flushed, so this is a checkpoint + // primitive only in the presence of external quiescence. Returns the first error, having + // still attempted the rest. + Status FlushAllPages(); + + // Drops the page from the pool and deallocates it on disk. Dirty contents are discarded + // without a flush — the page is going away. Rejects with kInvalidArgument if the frame is + // pinned rather than waiting for the pin to clear, which makes deleting a page you still + // hold a guard on an error instead of a deadlock. See "Known behaviour" in DD-002: this can + // also fail spuriously against a reclaimer's transient pin. + Status DeletePage(page_id_t page_id); + + // The durable shutdown operation (DD-002, "Shutdown and destruction"). Idempotent — a second + // call is a no-op, not an error. + // + // Verifies quiescence (every frame unpinned and not Loading), runs FlushAllPages(), then + // DiskManager::Sync(). Both steps matter: FlushAllPages only moves page bytes into the OS + // page cache via pwrite, which is not durability. + // + // Returns Status rather than living in the destructor precisely so a failed flush is the + // caller's decision — exit non-zero, refuse to mark the database cleanly closed — instead of + // a log line nobody reads. + // + // Quiescence is a PRECONDITION this verifies, not one it arranges. Destroying or shutting + // down a pool another thread is still using is undefined regardless of what happens here: a + // live guard holds a BufferPoolManager* that dangles the moment the pool goes away, and its + // later Drop() unpins through freed memory. The owner establishes quiescence by joining every + // worker thread — guards are stack-scoped, so an unwound stack has released every pin, and + // join() supplies the synchronizes-with edge (DD-003). A non-zero pin here is therefore proof + // that a guard outlived the pool; assert on it rather than working around it. + [[nodiscard]] Status Shutdown(); + + // Pool census. See PoolStats above for its two caveats — O(capacity), and a sample rather + // than a snapshot. + [[nodiscard]] PoolStats GetStats() const; + + private: + // The guards construct themselves through this class's private constructors and call + // UnpinPage below on release, so the friendship goes both ways. + friend class ReadPageGuard; + friend class WritePageGuard; + + // Steps 3-5 of the frozen guard release sequence (see page_guard.hpp). Under the frame's + // metadata mutex: notify all waiters if the frame is Failed, note whether this is a 1->0 + // transition on a Resident frame, decrement. Release the mutex, and only then call + // replacer_.SetEvictable(frame_id, true) if it transitioned — never while still holding the + // metadata mutex, per DD-002's lock ordering. + // + // Both halves of that condition are load-bearing. Testing the post-decrement value instead of + // the transition marks a frame evictable on a spurious unpin at pin_count == 0; testing the + // pin without the state admits a Free frame — one already on the free list — into the + // candidate set, so TryPop and Evict can hand the same frame to two threads. + // + // The Failed notify is why this must be the ONLY place a pin is ever decremented, including + // on the failed-load path where a waiter drops the pin it took before sleeping. A second + // decrement site is a site that has to remember the notify, and forgetting it parks the + // loader on the condvar forever. + // + // Underflow is a caller bug, not a condition to absorb: assert in debug, and log at a level + // that survives NDEBUG. In release the count goes negative and the frame is stuck for good — + // never 1->0 again, so never re-evictable, and DeletePage rejects it forever. That is a + // capacity leak rather than corruption, which is the only direction safe to be wrong in. + // + // Private on purpose. Two owners of one pin, a guard and a caller, is a double-decrement + // waiting to happen, so nothing above this layer ever unpins by hand. + void UnpinPage(frame_id_t frame_id); + + // The whole of FetchPage — hit path, miss path, Loading wait, failed-load handling — with + // the content latch deliberately left un-acquired. Returns a frame that is pinned, Resident, + // and already through RecordAccess/SetEvictable(false); the caller acquires the latch in its + // own mode and wraps the result in the matching guard. + // + // The split is where it is because DD-002's ordering demands it: the latch is taken last, + // strictly after both the metadata mutex and the replacer call are done with. That leaves + // nothing mode-specific in the protocol itself, so FetchPageRead and FetchPageWrite differ + // only in their final two lines rather than duplicating the miss path. + // + // The caller owns the returned pin from the moment this succeeds: every path out of it, + // including the latch acquisition throwing, must unpin or the frame is stranded. + [[nodiscard]] Result FetchFrame(page_id_t page_id); + + // Miss step 1: free_list_.TryPop(), else ReclaimFrame(). Returns a frame in state Free that + // is private to this thread — unmapped, unpinned, and out of both the free list and the + // replacer's candidate set. + [[nodiscard]] Result AcquireFrame(); + + // The inline reclaim loop of DD-002, "Reclaiming a frame". Entered holding no other lock, + // because re-validating a victim needs the *victim's* shard lock, which the caller's own + // shard lock says nothing about. Loops over victims until one re-validates; kBufferPoolFull + // when the replacer has no candidate left. + [[nodiscard]] Result ReclaimFrame(); + + // The failed-load path. Named for what it is — the loader's obligation, not a general + // cleanup any thread may run. Erases the mapping, publishes Failed, then waits for + // pin_count to fall to 1 (its own) before resetting to Free and pushing to the free list, + // NOT to the replacer: a Free frame is not a candidate, it is stock. + // + // Exactly one thread per Loading episode may call this, and it must be the thread that + // published Loading. Waiters observing Failed drop their pins and return the error; they + // dispose of nothing. + void AbandonLoad(frame_id_t frame_id, page_id_t page_id); + + // The five-step flush sequence (DD-002). Caller must already hold a pin — this takes the + // shared content latch and issues I/O, and an unpinned frame can be repurposed underneath + // both. Every flush path in this class routes through here so the epoch sampling and the + // CAS-upward on flushed_epoch_ exist in exactly one place. + Status FlushFrame(frame_id_t frame_id, page_id_t page_id); + + [[nodiscard]] Frame& FrameAt(frame_id_t frame_id) { + assert(frame_id >= 0 && static_cast(frame_id) < capacity_); + return frames_[static_cast(frame_id)]; + } + + [[nodiscard]] const Frame& FrameAt(frame_id_t frame_id) const { + assert(frame_id >= 0 && static_cast(frame_id) < capacity_); + return frames_[static_cast(frame_id)]; + } + + // One contiguous allocation made once at construction, per DD-002: Frame owns mutexes + // and a condition_variable, so it is neither copyable nor movable and cannot live in a + // container that might reallocate. + std::unique_ptr frames_; + std::size_t capacity_; + + FreeList free_list_; + PageTable page_table_; + Replacer replacer_; + DiskManager& disk_manager_; + + // Makes Shutdown() idempotent and tells the destructor whether it is the backstop path. + // Deliberately a plain bool, not an atomic: Shutdown() runs only under the quiescence it + // verifies, so there is no second thread to race with. An atomic here would advertise a + // thread-safety this operation does not have and cannot have. + bool shutdown_{false}; +}; + +} // namespace kernsql diff --git a/src/buffer/frame.hpp b/src/buffer/frame.hpp new file mode 100644 index 0000000..2a63e8b --- /dev/null +++ b/src/buffer/frame.hpp @@ -0,0 +1,199 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "common/types.hpp" + +namespace kernsql { + +// A frame's lifecycle (DD-002, "Frame lifecycle"), guarded by Frame::mtx_. One enum rather +// than a set of booleans, so the illegal combinations are unrepresentable rather than merely +// unreachable, and every transition has exactly one owner. +// +// There is deliberately no Reclaiming state. Reclaim performs no I/O, so it completes inside a +// single critical section under the victim's shard lock and metadata mutex — no other thread +// can observe a frame mid-reclaim, so there is nothing to cancel. +enum class FrameState : uint8_t { + // Holds no page: unmapped, unpinned, sitting on the free list. + Free, + // A disk read is in flight. Already mapped, so a concurrent fetcher of the same page finds + // it and waits on cv_ rather than racing to load it independently; data_ is meaningless + // until the loader publishes Resident. + Loading, + // Holds a valid page. In the Replacer's candidate set iff pin_count_ == 0. + Resident, + // The load errored. Already unmapped by the loader, which owns disposal — waiters observe + // this, drop their pins and return the error without cleaning up themselves. + Failed, +}; + +// The one place the state names live. std::formatter below delegates here rather than carrying +// its own switch, and the printf-style LOG_* macros need it because they cannot see a +// std::formatter at all — passing a FrameState for %s makes vsnprintf dereference the enum's +// value as a pointer. Returns a string literal, so it allocates nothing and is safe on an +// error path. +[[nodiscard]] constexpr const char* FrameStateName(FrameState state) { + switch (state) { + case FrameState::Free: + return "Free"; + case FrameState::Loading: + return "Loading"; + case FrameState::Resident: + return "Resident"; + case FrameState::Failed: + return "Failed"; + } + return "Unknown"; +} + +// Frame is the in-memory container for one cached page (see DD-002). It owns two +// independent synchronization primitives, deliberately kept separate: `latch_` guards +// the page bytes themselves and can be held for a while (e.g. across a scan); `mtx_` +// guards only the small bookkeeping fields below it and is held for nanoseconds. Mixing +// the two into one lock would let a slow content access block a fast metadata check — +// see the Concurrency-Control note for why that produces convoys. +// +// Deliberately NOT stored here: `frame_id` (purely positional — the frame's own index +// in the pool array), `page_lsn` (already part of `data_`'s on-disk page header; a WAL +// concern, not a buffer-pool one), and `usage_count` (belongs to the Replacer's +// bookkeeping, not the frame's identity). +// +// Cache-line aligned. `data_` sits at offset 0 and is exactly PAGE_SIZE — a whole number of +// 64-byte lines — so this one annotation buys both separations that matter: the metadata below +// starts on a line of its own instead of sharing one with the tail of the page bytes, and +// `sizeof(Frame)` rounds up to a multiple of 64, so frame N's metadata cannot share a line with +// frame N+1's page bytes either. Without it a thread writing page bytes and a thread probing an +// unrelated frame's pin count bounce the same line between cores for no reason. Postgres gets +// this separation structurally by keeping BufferDescriptors and BufferBlocks in two arrays; this +// is the cheap version of the same idea, costing 16 bytes of tail padding per frame +// (sizeof goes 4272 -> 4288, or 0.4%). +// +// The pool's `new Frame[n]` honours it because C++17 routes over-aligned types through +// operator new[](size_t, align_val_t) — this silently did nothing before C++17. +struct alignas(64) Frame { + // The PAGE_SIZE bytes currently resident in this frame. Guarded by `latch_`. + std::array data_; + + // The alignas is redundant today — `data_` alone already lands this at a 64-byte offset — + // but it is what actually pins the page-bytes/metadata split, so it is stated rather than + // left as a consequence of the member order above. + alignas(64) std::shared_mutex latch_; + + // --- Dirtiness: two monotonic counters, guarded by NEITHER lock (DD-002, "Dirtiness + // as an epoch pair"). The frame is dirty iff dirty_epoch_ > flushed_epoch_. + // + // This is deliberately not a `bool is_dirty_` under mtx_. A flusher must hold the + // shared content latch while it writes (or it captures a torn page), and if it then + // dropped the latch before clearing the bool, a writer could dirty the page in the gap + // and have its flag cleared a moment later — a lost update on disk. Keeping the bool + // correct therefore required holding latch_ and mtx_ simultaneously, in the *reverse* + // of this codebase's canonical lock order. With epochs there is nothing to clear, so + // the two locks are never held together at all. + // + // Unlike Replacer::usage_count_, these are NOT heuristic counters — a missed dirty bump + // loses data — so they use release/acquire rather than relaxed ordering. The cost is + // nil on x86 and it keeps the reasoning local instead of leaning on the fact that mtx_ + // happens to order most of these accesses anyway. + std::atomic dirty_epoch_{0}; + std::atomic flushed_epoch_{0}; + + // --- Everything below is guarded by mtx_ --- + // + // mutable so that an observer which only *reads* frame state — BufferPoolManager::GetStats + // — can still be const. Locking a mutex is not a logical mutation of the object, which is + // exactly the case `mutable` exists for; FreeList::mtx_ is mutable for the same reason. + mutable std::mutex mtx_; + FrameState state_{FrameState::Free}; + page_id_t page_id_{INVALID_PAGE}; // INVALID_PAGE iff state_ == Free. + int32_t pin_count_{0}; // a claim on this frame's identity — DD-002, "The pin + // invariant". Evictable only at 0. + + // Paired with mtx_. Two waits use it, and both hold a pin while they sleep: a miss waiter + // blocks here while state_ == Loading, and on the failed-load path the loader blocks here + // until pin_count_ falls to 1 (its own) so it can dispose of the frame itself. + std::condition_variable cv_; + + Frame() = default; + + // Implied by owning a mutex/condition_variable/shared_mutex already, but stated + // explicitly: DD-002 requires Frame to be non-copyable and non-movable, since it's + // embedded inline in one fixed contiguous pool allocation made once at construction. + Frame(const Frame&) = delete; + Frame& operator=(const Frame&) = delete; + Frame(Frame&&) = delete; + Frame& operator=(Frame&&) = delete; + + // The concrete lock types are an implementation detail that will change once `latch_` is + // replaced by a hybrid spin-then-block latch (see DD-002). Bind with `auto` where you can; + // where you cannot — a page guard has to name the type to store one as a member — spell it + // as Frame::ReadLatch / Frame::WriteLatch so the swap stays a one-line change here. + // + // Whatever replaces them must keep the same three operations the guards rely on: move + // construction, `unlock()`, and a destructor that releases only if still held. + using ReadLatch = std::shared_lock; + using WriteLatch = std::unique_lock; + + [[nodiscard]] ReadLatch AcquireRead() { return ReadLatch(latch_); } + [[nodiscard]] WriteLatch AcquireWrite() { return WriteLatch(latch_); } + + // --- Dirty-epoch protocol (DD-002). None of these take a lock. --- + + // Called by WritePageGuard's release, BEFORE it drops the content latch, so a flusher + // holding the shared latch always observes an epoch that already accounts for every + // write it is about to capture. + void MarkDirty() { dirty_epoch_.fetch_add(1, std::memory_order_release); } + + [[nodiscard]] bool IsDirty() const { + return dirty_epoch_.load(std::memory_order_acquire) > + flushed_epoch_.load(std::memory_order_acquire); + } + + // Step 2 of the flush sequence: sample the epoch while holding the shared content latch, + // hand the result to MarkFlushed() once the write completes. + [[nodiscard]] uint64_t SampleDirtyEpoch() const { + return dirty_epoch_.load(std::memory_order_acquire); + } + + // Step 5: raise flushed_epoch_ to `observed`. Only ever moves upward, so concurrent + // flushers cannot walk it backwards. A writer that dirtied the page after the latch was + // released has already bumped dirty_epoch_ past `observed`, leaving the frame dirty — + // which is the only direction that is safe to be wrong in. + void MarkFlushed(uint64_t observed) { + uint64_t current = flushed_epoch_.load(std::memory_order_relaxed); + while (current < observed && + !flushed_epoch_.compare_exchange_weak(current, observed, std::memory_order_release, + std::memory_order_relaxed)) { + } + } + + // Called when a frame is repurposed (miss path stamping it, a reclaimer turning it Free, + // DeletePage vacating it) — always with mtx_ held and no other thread holding a pin, so a + // plain store is enough. + void ResetEpochs() { + dirty_epoch_.store(0, std::memory_order_relaxed); + flushed_epoch_.store(0, std::memory_order_relaxed); + } +}; + +} // namespace kernsql + +// Specialize std::formatter for FrameState +template <> +struct std::formatter { + // Parses format specifiers (e.g., {:x}). We just accept standard empty {} bounds. + constexpr auto parse(format_parse_context& ctx) { return ctx.begin(); } + + // Formats the FrameState enum value into the output context. Delegates to FrameStateName so + // the names exist in exactly one place — the LOG_* macros need the same mapping and cannot + // reach a std::formatter. + auto format(const kernsql::FrameState& state, format_context& ctx) const { + return std::format_to(ctx.out(), "{}", kernsql::FrameStateName(state)); + } +}; diff --git a/src/buffer/freelist.hpp b/src/buffer/freelist.hpp new file mode 100644 index 0000000..4bf610d --- /dev/null +++ b/src/buffer/freelist.hpp @@ -0,0 +1,60 @@ +#pragma once + +#include +#include +#include +#include + +#include "common/types.hpp" + +namespace kernsql { + +// FreeList tracks frame_ids in state FrameState::Free: never used yet, vacated by DeletePage, +// or just turned Free by a reclaimer — distinct from the Replacer, which tracks frames that +// hold a page but are unpinned (see DD-002, "Free list and replacer"). +// +// It is a plain LIFO stack behind a plain mutex, and deliberately nothing more. An empty free +// list is a normal outcome rather than something to block on: a miss that finds it empty +// reclaims a frame inline through the Replacer instead. Nothing ever waits on this structure, +// so it has no producer/consumer sides to coordinate — hence no condition variable, no +// pool-exhausted signal, no watermarks and no shutdown handshake. +class FreeList { + public: + // Fully populates the list with every index 0..capacity-1 up front, so a freshly + // constructed FreeList is already fully stocked as an invariant of the type itself. + // lifo_'s size never exceeds capacity for the pool's lifetime (frame_ids only ever + // cycle between "free" and "in use," never duplicated or added to), so this + // reserve() is the only allocation FreeList will ever need to do. + explicit FreeList(std::size_t capacity) { + lifo_.reserve(capacity); + for (std::size_t i = 0; i < capacity; i++) { + lifo_.push_back(static_cast(i)); + } + } + + void Push(frame_id_t frame_id) { + std::lock_guard lock(mtx_); + lifo_.push_back(frame_id); + } + + // nullopt => empty right now. Not an error: it is the miss path's cue to reclaim a frame + // itself, which is the only other source of free frames. + [[nodiscard]] std::optional TryPop() { + std::lock_guard lock(mtx_); + if (lifo_.empty()) return std::nullopt; + frame_id_t frame_id = lifo_.back(); + lifo_.pop_back(); + return frame_id; + } + + [[nodiscard]] std::size_t Size() const { + std::lock_guard lock(mtx_); + return lifo_.size(); + } + + private: + mutable std::mutex mtx_; + std::vector lifo_; +}; + +} // namespace kernsql diff --git a/src/buffer/page_guard.cpp b/src/buffer/page_guard.cpp new file mode 100644 index 0000000..0caf11c --- /dev/null +++ b/src/buffer/page_guard.cpp @@ -0,0 +1,56 @@ +#include "page_guard.hpp" + +#include + +#include "buffer_pool_manager.hpp" + +using namespace kernsql; + +ReadPageGuard& ReadPageGuard::operator=(ReadPageGuard&& other) noexcept { + if (this == &other) return *this; + + Drop(); + + bpm_ = std::exchange(other.bpm_, nullptr); + frame_ = std::exchange(other.frame_, nullptr); + frame_id_ = other.frame_id_; + page_id_ = other.page_id_; + latch_ = std::move(other.latch_); + + return *this; +} + +void ReadPageGuard::Drop() { + if (bpm_ == nullptr) return; + + latch_.unlock(); + bpm_->UnpinPage(frame_id_); + + bpm_ = nullptr; + frame_ = nullptr; +} + +WritePageGuard& WritePageGuard::operator=(WritePageGuard&& other) noexcept { + if (this == &other) return *this; + + Drop(); + + bpm_ = std::exchange(other.bpm_, nullptr); + frame_ = std::exchange(other.frame_, nullptr); + frame_id_ = other.frame_id_; + page_id_ = other.page_id_; + latch_ = std::move(other.latch_); + + return *this; +} + +void WritePageGuard::Drop() { + if (bpm_ == nullptr) return; + + frame_->MarkDirty(); + latch_.unlock(); + bpm_->UnpinPage(frame_id_); + + bpm_ = nullptr; + frame_ = nullptr; +} diff --git a/src/buffer/page_guard.hpp b/src/buffer/page_guard.hpp new file mode 100644 index 0000000..e307e92 --- /dev/null +++ b/src/buffer/page_guard.hpp @@ -0,0 +1,229 @@ +#pragma once + +#include +#include +#include + +#include "buffer/frame.hpp" +#include "common/page_header.hpp" +#include "common/types.hpp" + +namespace kernsql { + +// BufferPoolManager returns guards and the guards call back into it on release, so the +// dependency is genuinely circular. It is broken the standard way: forward-declare here, and +// define the release path out of line in page_guard.cpp, which includes the manager's header. +class BufferPoolManager; + +// A ReadPageGuard/WritePageGuard owns a pin and a content latch together, and releases both +// when it dies (DD-002, "Page guards"). The pool never hands out a raw Frame*: a single missed +// unpin on an early-return or exception path does not fail loudly, it permanently pins one +// frame and silently shrinks the pool for the process's lifetime, with a symptom that shows up +// arbitrarily far from the leak. +// +// Guards are latched from birth, in the mode the caller named at fetch time. There is no +// pin-only guard and no read->write upgrade — a shared->unique upgrade has to drop the shared +// side, and anything the caller established while holding the read latch is void across that +// gap. Code that discovers it needs write access must drop the guard, re-fetch for write, and +// re-validate. +// +// RELEASE SEQUENCE (frozen in DD-002 — the order is load-bearing, do not rearrange): +// +// 1. WritePageGuard only: bump dirty_epoch. +// 2. Release the content latch. +// 3. Metadata mutex; decrement pin_count; note a 1->0 transition. +// 4. Release the metadata mutex. +// 5. If it transitioned, SetEvictable(frame_id, true). +// +// Steps 3-5 live in BufferPoolManager::UnpinPage, which is private and which these two classes +// are friends of. Step 1 precedes step 2 so a flusher holding the shared latch always observes +// an epoch that already accounts for every write it is about to capture; step 1 precedes step 3 +// so a reclaimer that sees pin_count == 0 under the metadata mutex is guaranteed to see this +// guard's dirty bump too, which is what makes its clean-check exact rather than merely +// conservative. Between steps 2 and 3 the frame cannot be reclaimed: the pin is still held, and +// it is the pin, not the latch, that protects a frame's identity. + +class ReadPageGuard { + public: + // Guards are only ever produced by a fetch, so there is no public constructor and no + // default constructor — a guard that holds nothing is not a state any caller should be + // able to create. + ReadPageGuard() = delete; + + // Non-copyable: a copy would decrement the same pin twice. + ReadPageGuard(const ReadPageGuard&) = delete; + ReadPageGuard& operator=(const ReadPageGuard&) = delete; + + // Movable, since fetches return by value. The moved-from guard is left inert. + ReadPageGuard(ReadPageGuard&& other) noexcept + : bpm_(std::exchange(other.bpm_, nullptr)), + frame_(std::exchange(other.frame_, nullptr)), + frame_id_(other.frame_id_), + page_id_(other.page_id_), + latch_(std::move(other.latch_)) {} + + ReadPageGuard& operator=(ReadPageGuard&& other) noexcept; + + ~ReadPageGuard() { Drop(); } + + // Releases the pin and the latch, in the order documented above. Explicit because latch + // crabbing has to release the parent before the child's scope ends, not at scope exit. + // Idempotent, and implicitly called by the destructor — calling it twice, or letting a + // dropped guard die, is well-defined and does nothing the second time. + void Drop(); + + // The page this guard holds. Stored by value at construction, deliberately never read back + // from Frame::page_id_: that field is guarded by the frame's metadata mutex, so reading it + // through an accessor that takes no lock is a data race (and a tsan report). It cannot + // change while the guard lives anyway — a pinned frame is never repurposed. + [[nodiscard]] page_id_t PageId() const { return page_id_; } + + // The page BODY — everything after the 32-byte header. Deliberately not the whole page: + // see WritePageGuard::MutableBody for why the header is unreachable as raw bytes. + // Const-only here; it is the type system, not a runtime mode check, that stops a reader + // writing. + [[nodiscard]] std::span Body() const { + return std::span(frame_->data_) + .subspan(); + } + + // The header, BY VALUE. A copy, so there is no reference to write through. + // + // Header().page_id and PageId() answer different questions and should always agree: + // PageId() is what the buffer pool believes this frame holds, Header().page_id is what + // the bytes claim. Disagreement means the page was clobbered after it was validated. + [[nodiscard]] PageHeader Header() const { + return PageHeader::ReadFrom( + std::span(frame_->data_).first()); + } + + private: + friend class BufferPoolManager; + + ReadPageGuard(BufferPoolManager* bpm, Frame* frame, frame_id_t frame_id, page_id_t page_id, + Frame::ReadLatch latch) + : bpm_(bpm), + frame_(frame), + frame_id_(frame_id), + page_id_(page_id), + latch_(std::move(latch)) {} + + // Null once dropped or moved from, which is exactly what makes Drop() idempotent and a + // moved-from guard's destructor inert. + BufferPoolManager* bpm_; + Frame* frame_; + frame_id_t frame_id_; + page_id_t page_id_; + Frame::ReadLatch latch_; +}; + +class WritePageGuard { + public: + WritePageGuard() = delete; + + WritePageGuard(const WritePageGuard&) = delete; + WritePageGuard& operator=(const WritePageGuard&) = delete; + + WritePageGuard(WritePageGuard&& other) noexcept + : bpm_(std::exchange(other.bpm_, nullptr)), + frame_(std::exchange(other.frame_, nullptr)), + frame_id_(other.frame_id_), + page_id_(other.page_id_), + latch_(std::move(other.latch_)) {} + + WritePageGuard& operator=(WritePageGuard&& other) noexcept; + + ~WritePageGuard() { Drop(); } + + // As ReadPageGuard::Drop, plus step 1: this bumps the frame's dirty_epoch before releasing + // the latch. The bump is UNCONDITIONAL — holding a write guard is itself the declaration of + // intent, and no attempt is made to track whether a write actually landed. That + // over-approximates dirtiness, costing at most a redundant flush for a write guard that + // wrote nothing. The precise alternative is an explicit MarkDirty() the caller must + // remember, which reintroduces the exact class of bug guards exist to eliminate, with data + // loss rather than a wasted write as its failure mode. + void Drop(); + + [[nodiscard]] page_id_t PageId() const { return page_id_; } + + [[nodiscard]] std::span Body() const { + return std::span(frame_->data_) + .subspan(); + } + + // The only mutable view a caller ever gets, and it starts AFTER the header. + // + // This is what makes the miss-path validation in FetchFrame trustworthy rather than + // hopeful. If a caller could name all PAGE_SIZE bytes, then `memcpy(page, buf, PAGE_SIZE)` + // — the most natural thing in the world to write in a heap or B+tree layer — would + // overwrite page_id and page_type, and the page would fail validation on its next miss, + // long after the code that broke it ran. Handing out only the body means that line cannot + // be written. Same reasoning as guards existing at all: delete the class of bug rather + // than document it. + [[nodiscard]] std::span MutableBody() { + return std::span(frame_->data_).subspan(); + } + + [[nodiscard]] PageHeader Header() const { + return PageHeader::ReadFrom( + std::span(frame_->data_).first()); + } + + // The header fields a layer above owns, exposed one at a time rather than as a whole + // PageHeader. A SetHeader(const PageHeader&) would reopen exactly the hole this class + // closes — a caller could write a page_id that disagrees with the frame's. + // + // Deliberately absent: page_id and format_version belong to DiskManager and NewPage, + // checksum to whoever implements it, page_lsn to a WAL that does not exist. None of them + // are a caller's to set, so none of them have a setter. + void SetPageType(PageType type) { + PageHeader header = Header(); + header.page_type = type; + WriteHeader(header); + } + void SetFlags(uint8_t flags) { + PageHeader header = Header(); + header.flags = flags; + WriteHeader(header); + } + void SetNextPageId(page_id_t next) { + PageHeader header = Header(); + header.next_page_id = next; + WriteHeader(header); + } + void SetPrevPageId(page_id_t prev) { + PageHeader header = Header(); + header.prev_page_id = prev; + WriteHeader(header); + } + + private: + friend class BufferPoolManager; + + // Private, so the only way to write the header is through the typed setters above — which + // is what keeps the fields a caller does not own untouchable. + // + // Deliberately not a template taking a lambda. Inside a member template, `.first()` on + // a CTAD-deduced span is treated as a dependent template name by GCC and needs a + // `template` disambiguator; clang accepts it without. A portability trap is a bad price + // for saving two lines per setter. + void WriteHeader(const PageHeader& header) { + header.WriteTo(std::span(frame_->data_).first()); + } + + WritePageGuard(BufferPoolManager* bpm, Frame* frame, frame_id_t frame_id, page_id_t page_id, + Frame::WriteLatch latch) + : bpm_(bpm), + frame_(frame), + frame_id_(frame_id), + page_id_(page_id), + latch_(std::move(latch)) {} + + BufferPoolManager* bpm_; + Frame* frame_; + frame_id_t frame_id_; + page_id_t page_id_; + Frame::WriteLatch latch_; +}; + +} // namespace kernsql diff --git a/src/buffer/page_table.hpp b/src/buffer/page_table.hpp new file mode 100644 index 0000000..d0880e1 --- /dev/null +++ b/src/buffer/page_table.hpp @@ -0,0 +1,86 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include + +#include "common/logger.hpp" +#include "common/types.hpp" + +namespace kernsql { +class PageTable { + public: + static constexpr std::size_t kNumShards = 16; // fixed power of two (DD-002) + + // RAII handle: holds one shard's lock + gives access to that shard's map for as + // long as it's alive. Non-copyable (holds a unique_lock), movable via guaranteed + // copy elision on return (constructed in place at the call site, C++17). + class ShardGuard { + public: + [[nodiscard]] std::optional Find(page_id_t page_id) const { + auto it = map_.find(page_id); + if (it != map_.end()) return it->second; + return std::nullopt; + } + + // A duplicate key means two frames believe they hold the same page: writes land in + // one, reads come from the other, and whichever flushes last wins. That is silent + // data corruption, not a recoverable condition — so this survives NDEBUG rather than + // compiling out into an emplace that quietly does nothing and leaves the stale + // mapping in place. Same reasoning as ~BufferPoolManager aborting rather than + // discarding dirty pages: a destructor, and this, are both the wrong place to decide + // that corruption is survivable. + void Insert(page_id_t page_id, frame_id_t frame_id) { + assert(ShardIndex(page_id) == shard_index_); + auto [it, inserted] = map_.emplace(page_id, frame_id); + if (!inserted) { + LOG_INFO("page %d is already mapped to frame %d; refusing to remap it to %d", + page_id, it->second, frame_id); + std::abort(); + } + } + void Erase(page_id_t page_id) { + assert(ShardIndex(page_id) == shard_index_); + map_.erase(page_id); + } + + private: + friend class PageTable; + ShardGuard(std::mutex& mtx, std::unordered_map& map, + std::size_t shard_index) + : lock_(mtx), map_(map), shard_index_(shard_index) {} + + std::unique_lock lock_; + std::unordered_map& map_; + std::size_t shard_index_; // for the assert in Find/Insert/Erase — never changes after + // construction. + }; + + // The common case: acquire the shard page_id hashes to. + [[nodiscard]] ShardGuard AcquireShard(page_id_t page_id) { + std::size_t shard_idx = ShardIndex(page_id); + return AcquireShardByIndex(shard_idx); + } + + // For the cross-shard case (DD-002 lock ordering: acquire both shards ascending, + // re-validate) — acquiring by raw index is what makes "ascending order" expressible + // at all, since the two page_ids involved hash to different shards by definition. + [[nodiscard]] ShardGuard AcquireShardByIndex(std::size_t shard_index) { + return ShardGuard(mutexes_[shard_index], maps_[shard_index], shard_index); + } + + // Exposed so BufferPoolManager can compute which two indices it needs, and in what + // order, before acquiring either. + [[nodiscard]] static std::size_t ShardIndex(page_id_t page_id) { + return static_cast(page_id) & (kNumShards - 1); + } + + private: + std::array mutexes_; + std::array, kNumShards> maps_; +}; +} // namespace kernsql diff --git a/src/buffer/pool_stats.hpp b/src/buffer/pool_stats.hpp new file mode 100644 index 0000000..d46e14e --- /dev/null +++ b/src/buffer/pool_stats.hpp @@ -0,0 +1,33 @@ +#pragma once + +#include + +namespace kernsql { + +// A census of the buffer pool, for diagnostics and tests. Two properties to respect: +// +// 1. Producing it is O(capacity) and takes every frame's metadata mutex in turn. Fine for a +// 16-frame test pool, real work for a 262144-frame one. A diagnostic, never a hot path — +// the same way pg_buffercache walks Postgres's BufferDescriptors. +// 2. It is a SAMPLE, not a snapshot. By the time the walk reaches frame 5000, frame 0 has +// moved on, so no cross-frame invariant holds unless the pool is quiescent. A caller that +// wants to assert on these numbers must establish quiescence first, exactly as Shutdown() +// requires. +// +// At quiescence the invariants worth asserting are: +// free_frames == free_list_size a Free frame not on the list is unreachable +// forever, which is the leak class that has bitten +// this component twice +// loading_frames == failed_frames == 0 both states carry a pin by construction +// pinned_frames == 0 +struct PoolStats { + size_t capacity{0}; + size_t free_frames{0}; // state == Free + size_t loading_frames{0}; // nonzero at quiescence is itself a bug + size_t resident_frames{0}; + size_t failed_frames{0}; // ditto + size_t pinned_frames{0}; + size_t free_list_size{0}; + size_t evictable{0}; // replacer's candidate count +}; +} // namespace kernsql diff --git a/src/buffer/replacer.hpp b/src/buffer/replacer.hpp new file mode 100644 index 0000000..0410f20 --- /dev/null +++ b/src/buffer/replacer.hpp @@ -0,0 +1,118 @@ +#pragma once + +#include +#include +#include +#include +#include +#include + +#include "common/types.hpp" + +namespace kernsql { + +// Clock-sweep eviction-candidate set: frames that currently hold a page but are unpinned. +// +// Threading note (DD-002, "Reclaiming a frame"): every method here is called from any thread. +// RecordAccess and SetEvictable fire on every access and every pin-count transition; Evict() +// is called by whichever thread missed and found the FreeList empty, and reclaims a frame +// inline. There is no background thread. +// +// Two concurrent Evict() callers are therefore expected, and are safe because Evict removes +// its victim from the candidate set before returning — so they cannot be handed the same +// frame. The caller then re-validates the victim under its page-table shard lock and metadata +// mutex, and a caller that declines the frame it was given is a normal outcome, not an error: +// the frame simply stays out of the candidate set until its next unpin re-adds it. +class Replacer { + public: + explicit Replacer(std::size_t capacity) : usage_count_(capacity), evictable_(capacity) {} + + // Bumps this frame's usage_count, capped at kMaxUsageCount. Called on every access + // (hit or miss), independent of the frame's current evictable state. + void RecordAccess(frame_id_t frame_id) { + assert(static_cast(frame_id) < usage_count_.size()); + auto& count = usage_count_[static_cast(frame_id)]; + uint8_t old = count.load(std::memory_order_relaxed); + while (old < kMaxUsageCount && + !count.compare_exchange_weak(old, old + 1, std::memory_order_relaxed, + std::memory_order_relaxed)) { + } + } + + // Adds/removes frame_id from the eviction-candidate set. Idempotent. + void SetEvictable(frame_id_t frame_id, bool evictable) { + const auto index = static_cast(frame_id); + assert(index < evictable_.size()); + std::lock_guard guard(mtx_); + + if (evictable_[index] == evictable) return; + + evictable_[index] = evictable; + if (evictable) + ++evictable_count_; + else + --evictable_count_; + } + + // Clock sweep: returns the first evictable frame found with usage_count == 0, + // decrementing every evictable frame's usage_count as the hand passes it. + // nullopt only when nothing is currently evictable; otherwise a victim is guaranteed + // (bounded sweep + fallback, per DD-002 "Sweep semantics"). + [[nodiscard]] std::optional Evict() { + std::lock_guard guard(mtx_); + if (evictable_count_ == 0) return std::nullopt; + + const std::size_t n = usage_count_.size(); + + for (std::size_t scanned = 0; scanned < (kMaxUsageCount + 1) * n; ++scanned) { + if (!evictable_[hand_]) { + hand_ = (hand_ + 1) % n; + continue; + } + + if (usage_count_[hand_].load(std::memory_order_relaxed) == 0) return ClaimVictim(); + + usage_count_[hand_].fetch_sub(1, std::memory_order_relaxed); + hand_ = (hand_ + 1) % n; + } + + // Bound exhausted — only reachable if concurrent RecordAccess keeps re-bumping + // candidates as fast as the hand decrements them. Take the first evictable frame + // regardless of count; membership can't change while mtx_ is held and + // evictable_count_ > 0, so this finds one within n steps. + while (!evictable_[hand_]) hand_ = (hand_ + 1) % n; + return ClaimVictim(); + } + + // How many frames are currently eviction candidates. A diagnostic, not a decision input: + // the answer is stale the moment the lock is released, so nothing may branch on it. Evict() + // re-reads evictable_count_ under the same lock it evicts from, which is what makes *its* + // use of the value sound. + [[nodiscard]] std::size_t EvictableCount() const { + std::lock_guard guard(mtx_); + return evictable_count_; + } + + private: + // Victim post-conditions (DD-002): leaves the candidate set and its usage_count resets + // to 0 before Evict returns. Requires mtx_ held with hand_ on an evictable frame. + frame_id_t ClaimVictim() { + const std::size_t victim = hand_; + + evictable_[victim] = false; + --evictable_count_; + usage_count_[victim].store(0, std::memory_order_relaxed); + + hand_ = (hand_ + 1) % usage_count_.size(); + return static_cast(victim); + } + + static constexpr uint8_t kMaxUsageCount = 3; // resolved in DD-002 + + mutable std::mutex mtx_; + std::vector usage_count_; // indexed by frame_id + std::vector evictable_; // 0/1 flags — deliberately not vector + std::size_t hand_ = 0; // single global hand (resolved in DD-002) + std::size_t evictable_count_ = 0; +}; +} // namespace kernsql diff --git a/src/common/logger.cpp b/src/common/logger.cpp index dc8ddfe..7af64c7 100644 --- a/src/common/logger.cpp +++ b/src/common/logger.cpp @@ -1,5 +1,7 @@ #include "logger.hpp" +#include + #include #include #include @@ -10,6 +12,18 @@ namespace { std::mutex g_log_mutex; +// The kernel TID, not std::thread::id: this is the number gdb reports as LWP, that `perf` and +// `top -H` show, and that names the directory under /proc//task — so a log line can be +// matched against a debugger session. std::thread::id has no printf conversion and hashes to +// something that matches nothing outside this process. +// +// Cached per thread: gettid() is a real syscall, and a thread's id never changes, so this costs +// one syscall per thread for the life of the process rather than one per log line. +int cached_tid() { + static thread_local const int tid = static_cast(gettid()); + return tid; +} + const char* level_tag(LogLevel level) { switch (level) { case LogLevel::DEBUG: @@ -44,8 +58,8 @@ void log_impl(LogLevel level, const char* file, int line, const char* fmt, ...) // 3. Assemble the full line, then do one write under the mutex. char linebuf[1280]; - std::snprintf(linebuf, sizeof(linebuf), "%s.%03d [%-5s] %s:%d %s\n", ts, - static_cast(ms.count()), level_tag(level), file, line, msg); + std::snprintf(linebuf, sizeof(linebuf), "%s.%03d [%-5s] [tid %d] %s:%d %s\n", ts, + static_cast(ms.count()), level_tag(level), cached_tid(), file, line, msg); std::lock_guard guard(g_log_mutex); std::fputs(linebuf, stdout); diff --git a/src/common/page_header.hpp b/src/common/page_header.hpp index bace51c..45e60b0 100644 --- a/src/common/page_header.hpp +++ b/src/common/page_header.hpp @@ -10,16 +10,58 @@ namespace kernsql { /* - * struct PageHeader forms the first 24 bytes of a page, it is zero-padded to ensure byte alignment + * struct PageHeader forms the first 32 bytes of every page. + * + * Field order is chosen so every member lands on its natural alignment with no implicit + * padding: page_lsn sits at offset 16 and the whole struct is a multiple of 8. The two + * reserved members are explicit rather than left to the compiler so that ReadFrom/WriteTo, + * which memcpy the struct wholesale, move a fully-defined 32 bytes. + * + * NOT PORTABLE ACROSS ARCHITECTURES. ReadFrom/WriteTo are a raw memcpy, so the file is + * host-endian and host-ABI. A database file written on one machine is only readable on + * another with the same byte order and layout rules. That is a deliberate non-goal — a + * byte-at-a-time serializer would fix it and buys nothing for a single-node engine. */ struct PageHeader { PageType page_type{PageType::FREE}; - uint8_t reserved0[3]{}; // explicit padding — always zero + uint8_t flags{0}; // reserved — always zero + uint16_t format_version{PAGE_FORMAT_VERSION}; + + // This page's own id, redundant with the offset it was read from — and redundant on + // purpose. The offset says which page we *asked* for; this says which page the bytes + // think they are. Derive identity from the offset alone and a disagreement becomes + // undetectable by construction, because you have assumed the answer. Catches misdirected + // writes, off-by-one page arithmetic, torn extends, and backups copied at a wrong offset. + // InnoDB stores the same thing as FIL_PAGE_OFFSET; Postgres folds the block number into + // its checksum to get the same effect. + // + // It does NOT catch a page freed and handed back out under the same id — only page_type + // == FREE catches that. + // + // Stamped by DiskManager::write_page_header for every header it writes, and by + // BufferPoolManager::NewPage for the one page that is created without ever being read. + page_id_t page_id{INVALID_PAGE}; + + // Doubles as the disk freelist link (DD-001) and, later, the B+tree leaf sibling pointer. + // Postgres keeps sibling pointers in per-page-type special space at the end of the page + // rather than in the shared header, since only some page types have siblings. This is a + // deliberate deviation: the freelist is already built on it, and 8 bytes for next+prev is + // 0.2% of a page. page_id_t next_page_id{INVALID_PAGE}; page_id_t prev_page_id{INVALID_PAGE}; - uint32_t reserved1{0}; // explicit padding before 8-aligned lsn + + // Reserved for a write-ahead log. Nothing writes it today and there is no WAL planned for + // this engine, but the field is here because growing a page header later shifts every byte + // of every page — an on-disk format break — whereas eight unused bytes cost 0.2%. lsn_t page_lsn{0}; + // Reserved. A checksum is what turns this header from "catches my bugs" into "catches the + // disk's", which is the class Postgres's PageIsVerified and InnoDB's page checksum are + // actually defending against. Space claimed now so implementing it later is not a format + // break. + uint32_t checksum{0}; + uint32_t reserved{0}; + static PageHeader ReadFrom(std::span page_header) { PageHeader h; std::memcpy(&h, page_header.data(), sizeof(PageHeader)); @@ -30,7 +72,9 @@ struct PageHeader { } }; -static_assert(sizeof(PageHeader) == PAGE_HEADER_SIZE, "Page Header must be exactly 24 bytes"); +static_assert(sizeof(PageHeader) == PAGE_HEADER_SIZE, "Page Header must be exactly 32 bytes"); +static_assert(alignof(PageHeader) == 8, "page_lsn must land 8-aligned, so the struct must too"); +static_assert(offsetof(PageHeader, page_lsn) == 16, "no implicit padding before page_lsn"); static_assert(std::is_standard_layout_v, "Page Header must be standard layout"); static_assert(std::is_trivially_copyable_v, "PageHeader must be trivially copyable"); diff --git a/src/common/types.hpp b/src/common/types.hpp index 6436f22..b090843 100644 --- a/src/common/types.hpp +++ b/src/common/types.hpp @@ -15,7 +15,17 @@ inline constexpr page_id_t INVALID_PAGE = -1; inline constexpr page_id_t META_PAGE_ID = 0; // reserved superblock page; never allocatable inline constexpr page_id_t CATALOG_ROOT_PAGE_ID = 1; // reserved page for catalog root inline constexpr std::size_t PAGE_SIZE = 4096; -inline constexpr std::size_t PAGE_HEADER_SIZE = 24; +inline constexpr std::size_t PAGE_HEADER_SIZE = 32; + +// Everything after the header. This — not PAGE_SIZE — is what a page guard hands out, so a +// caller cannot name the header bytes as raw memory and cannot clobber them. +inline constexpr std::size_t PAGE_BODY_SIZE = PAGE_SIZE - PAGE_HEADER_SIZE; + +// Bumped whenever the on-disk layout of PageHeader changes in a way an older build would +// misread. Stamped into every header DiskManager writes; nothing reads it back yet, which is +// the point — it exists so that a future format change has a discriminator to branch on +// instead of guessing. +inline constexpr uint16_t PAGE_FORMAT_VERSION = 1; struct RID { page_id_t page_id{INVALID_PAGE}; @@ -25,6 +35,27 @@ struct RID { bool isValid() const { return page_id != INVALID_PAGE; } }; -enum class PageType : uint8_t { META, INDEX_INTERNAL, INDEX_LEAF, HEAP, ALLOCATED, FREE, CATALOG }; +// INVALID is pinned at 0 so that an all-zero page cannot pass for a real one. Zeros +// are what the filesystem hands back for a sparse or truncate-extended file, so with +// META at 0 an empty file's page 0 validated as a genuine meta page — and Open() then +// read a freelist head of 0 out of the zeroed next_page_id, putting the reserved meta +// page itself at the head of the freelist. +// +// This is a cheap mitigation, not a format check: a garbage file whose first byte +// happens to equal a valid PageType still passes. A magic + version field in the meta +// page is the actual answer and does not exist yet. +// +// These values are persisted in every page header, so renumbering them is an on-disk +// format break. Append new types at the end; never reorder or reuse a value. +enum class PageType : uint8_t { + INVALID = 0, + META, + INDEX_INTERNAL, + INDEX_LEAF, + HEAP, + ALLOCATED, + FREE, + CATALOG +}; } // namespace kernsql diff --git a/src/shell/main.cpp b/src/shell/main.cpp index a8d6045..da6524e 100644 --- a/src/shell/main.cpp +++ b/src/shell/main.cpp @@ -1,17 +1,218 @@ +// A hand-driving REPL for the storage engine. Not SQL — it exposes one command per +// BufferPoolManager operation so the layer can be exercised and misused on purpose before +// anything is built on top of it. +// +// Two rules from DD-003 shape the structure here, and they are the reason every command +// acquires and drops its guard inside a single function: +// +// - a page guard never outlives the function that acquired it, and +// - a pin never spans a client round-trip. +// +// This REPL *is* the client. A guard held across the prompt would pin a frame for as long as +// the user takes to type, which is exactly the pool exhaustion those rules exist to prevent. + +#include +#include #include +#include #include +#include +#include +#include +#include "buffer/buffer_pool_manager.hpp" +#include "buffer/page_guard.hpp" +#include "buffer/pool_stats.hpp" #include "common/status.hpp" +#include "common/types.hpp" #include "storage/disk_manager.hpp" -int main() { - using namespace kernsql; +using namespace kernsql; + +namespace { + +constexpr std::size_t kPoolFrames = 16; + +// The guard hands out the body only — the header is not reachable as raw bytes, so there is +// no offset arithmetic to get wrong here. The buffer pool has no opinion about page contents +// (DD-003), so this shell just treats the body as a NUL-terminated blob. +constexpr std::size_t kBodySize = PAGE_BODY_SIZE; + +std::string_view Trim(std::string_view s) { + while (!s.empty() && (s.front() == ' ' || s.front() == '\t')) s.remove_prefix(1); + while (!s.empty() && (s.back() == ' ' || s.back() == '\t' || s.back() == '\r')) + s.remove_suffix(1); + return s; +} + +// Splits the leading whitespace-delimited token off `rest`, advancing it past what was taken. +std::string_view NextToken(std::string_view& rest) { + rest = Trim(rest); + const auto end = rest.find_first_of(" \t"); + std::string_view token = rest.substr(0, end); + rest = (end == std::string_view::npos) ? std::string_view{} : Trim(rest.substr(end)); + return token; +} + +bool ParsePageId(std::string_view token, page_id_t& out) { + if (token.empty()) return false; + const auto* first = token.data(); + const auto* last = token.data() + token.size(); + auto [ptr, ec] = std::from_chars(first, last, out); + return ec == std::errc{} && ptr == last; +} + +void PrintStatus(std::string_view what, const Status& st) { + if (st.ok()) + std::println("{}: ok", what); + else + std::println("{}: {}", what, st.message()); +} + +void CmdNew(BufferPoolManager& bpm) { + auto guard = bpm.NewPage(); + if (!guard) { + std::println("new: {}", guard.error().message()); + return; + } + // Report the id and drop the guard immediately. Holding it would be the round-trip pin. + std::println("new: allocated page {}", guard->PageId()); +} + +void CmdWrite(BufferPoolManager& bpm, page_id_t page_id, std::string_view text) { + if (text.size() + 1 > kBodySize) { + std::println("write: text too long ({} bytes, max {})", text.size(), kBodySize - 1); + return; + } + + auto guard = bpm.FetchPageWrite(page_id); + if (!guard) { + std::println("write: {}", guard.error().message()); + return; + } + + auto body = guard->MutableBody(); + std::fill(body.begin(), body.end(), std::byte{0}); // no tail of a previous, longer write + std::memcpy(body.data(), text.data(), text.size()); + + std::println("write: page {} <- {} bytes", page_id, text.size()); + // Guard drops here: dirty_epoch bumped, latch released, pin dropped. Nothing is on disk + // yet — that needs `flush` or `quit`. +} + +void CmdRead(BufferPoolManager& bpm, page_id_t page_id) { + auto guard = bpm.FetchPageRead(page_id); + if (!guard) { + std::println("read: {}", guard.error().message()); + return; + } + + auto body = guard->Body(); + std::size_t len = 0; + while (len < body.size() && body[len] != std::byte{0}) ++len; + + std::println("read: page {} -> \"{}\"", page_id, + std::string_view(reinterpret_cast(body.data()), len)); +} + +void CmdHelp() { + std::println("commands:"); + std::println(" new allocate a page and return its id"); + std::println(" write overwrite the page body"); + std::println(" read print the page body"); + std::println(" delete free the page"); + std::println(" flush write one page through to the file"); + std::println(" flushall write every resident page through"); + std::println(" stat page count on disk"); + std::println(" help this list"); + std::println(" quit shutdown (flush + fsync) and exit"); +} + +// Returns false when the REPL should stop. +bool Dispatch(BufferPoolManager& bpm, DiskManager& dm, std::string_view line) { + std::string_view rest = line; + const std::string_view cmd = NextToken(rest); + if (cmd.empty()) return true; + + auto needs_page_id = [&](page_id_t& id) { + if (ParsePageId(NextToken(rest), id)) return true; + std::println("{}: expected a page id", cmd); + return false; + }; + + if (cmd == "quit" || cmd == "exit") return false; + if (cmd == "help") { + CmdHelp(); + } else if (cmd == "new") { + CmdNew(bpm); + } else if (cmd == "stat") { + const PoolStats st = bpm.GetStats(); + std::println("stat: {} pages on disk", dm.PageCount()); + std::println(" frames {}: {} free / {} resident / {} loading / {} failed", st.capacity, + st.free_frames, st.resident_frames, st.loading_frames, st.failed_frames); + std::println(" {} pinned, {} evictable, free list holds {}", st.pinned_frames, + st.evictable, st.free_list_size); + } else if (cmd == "flushall") { + PrintStatus("flushall", bpm.FlushAllPages()); + } else if (cmd == "write") { + page_id_t id{}; + if (needs_page_id(id)) CmdWrite(bpm, id, rest); + } else if (cmd == "read") { + page_id_t id{}; + if (needs_page_id(id)) CmdRead(bpm, id); + } else if (cmd == "delete") { + page_id_t id{}; + if (needs_page_id(id)) PrintStatus("delete", bpm.DeletePage(id)); + } else if (cmd == "flush") { + page_id_t id{}; + if (needs_page_id(id)) PrintStatus("flush", bpm.FlushPage(id)); + } else { + std::println("unknown command '{}' — try `help`", cmd); + } + return true; +} + +} // namespace + +int main(int argc, char** argv) { + const std::filesystem::path db_path = + (argc > 1) ? std::filesystem::path(argv[1]) + : std::filesystem::temp_directory_path() / "kernsql.db"; + + auto dm = DiskManager::Open(db_path); + if (!dm) { + std::println(stderr, "cannot open {}: {}", db_path.string(), dm.error().message()); + return 1; + } + + BufferPoolManager bpm(**dm, kPoolFrames); + + std::println("kernSQL — {} ({} pages)", db_path.string(), (*dm)->PageCount()); + std::println("`help` for commands, `quit` to shut down cleanly."); - std::println("Hello kernSQL"); - Status s = Status::OK(); - std::println("Testing: {}", s.message()); + std::string line; + while (true) { + std::print("kernsql> "); + std::cout.flush(); + if (!std::getline(std::cin, line)) { + // std::println("") rather than std::println(): the zero-argument overload is + // P3142, a C++26 addition. libc++ 21 has it, libc++ 18 (what CI builds with) does + // not, and this is the only place we would need it. + std::println(""); // EOF (ctrl-D) — treat as a clean quit + break; + } + if (!Dispatch(bpm, **dm, line)) break; + } - auto DB_PATH = std::filesystem::temp_directory_path() / "kernsql.db"; - auto dm = DiskManager::Open(DB_PATH); - if (dm) return 0; + // Shutdown() is the durable operation, not the destructor (DD-002). Quiescence is trivially + // satisfied here — the REPL is single-threaded and every guard was function-scoped, so no + // pin outlives the loop. Skipping this would trip the destructor's backstop, which logs the + // mistake and aborts rather than silently dropping dirty pages. + Status st = bpm.Shutdown(); + if (!st.ok()) { + std::println(stderr, "shutdown failed: {}", st.message()); + return 1; // the caller decides what a failed flush means — here, a non-zero exit + } + std::println("shutdown ok"); + return 0; } diff --git a/src/storage/disk_manager.cpp b/src/storage/disk_manager.cpp index 54db387..f8dd84d 100644 --- a/src/storage/disk_manager.cpp +++ b/src/storage/disk_manager.cpp @@ -6,12 +6,15 @@ #include #include +#include #include #include #include #include #include +#include #include +#include #include "common/logger.hpp" #include "common/page_header.hpp" @@ -49,7 +52,16 @@ Result> DiskManager::Open(const std::filesystem::pa auto dm = std::unique_ptr(new DiskManager(fd, path, page_count)); if (page_count == 0) { - // brand new file: reserve page 0 as the meta/superblock page + // Brand-new file: reserve page 0 as the meta/superblock page. + // + // Every early return in this branch leaves a partially initialized file on + // disk — one page long, or two pages with no freelist head persisted. The + // next Open() sees page_count == 1 and reports Corruption rather than + // silently repairing it. That is a deliberate v1 decision, not an oversight: + // unwinding correctly would mean either unlinking a file the caller may not + // have wanted us to delete, or a bootstrap-recovery path, and both are the + // job of the WAL that does not exist yet. Crash-during-create is + // unrecoverable-by-design until then; the file is safe to delete by hand. Status st = dm->write_empty_page(META_PAGE_ID); if (!st.ok()) { return std::unexpected(st); @@ -152,7 +164,7 @@ Status DiskManager::validate_page_access(page_id_t page_id) { } if (!valid_page(page_id)) { return Status::InvalidArgument( - std::format("invalid page id {}, page id < {}", page_id, this->page_count_)); + std::format("invalid page id {}, page id < {}", page_id, this->page_count_.load())); } return Status::OK(); } @@ -162,13 +174,10 @@ Status DiskManager::ReadPage(page_id_t page_id, std::span return s; } - ssize_t status = - pread(this->fd_, out.data(), PAGE_SIZE, static_cast(page_id * PAGE_SIZE)); - if (status != static_cast(PAGE_SIZE)) { - return Status::IOError("failed to read page"); - } - - return Status::OK(); + // No latch: pread is atomic per call with respect to the file offset, and the + // bounds check above reads an atomic page_count_. Two callers touching the same + // page is the buffer pool's problem, not ours. + return full_read(PageOffset(page_id), out); } Status DiskManager::WritePage(page_id_t page_id, std::span in) { @@ -176,16 +185,17 @@ Status DiskManager::WritePage(page_id_t page_id, std::spanfd_, in.data(), PAGE_SIZE, static_cast(page_id * PAGE_SIZE)); - if (status != static_cast(PAGE_SIZE)) { - return Status::IOError("failed to write to page"); - } - - return Status::OK(); + // Latch-free for the same reason as ReadPage. + return full_write(PageOffset(page_id), in); } Result DiskManager::AllocatePage() { + // Held for the whole operation, not just the freelist_head_ store: the read of + // the head, the walk to its successor, the page-0 persist and the local update + // have to be one indivisible step, or two allocators both read the same head and + // both hand it out. + std::scoped_lock lock(this->meta_latch_); + PageHeader allocated_header; allocated_header.page_type = PageType::ALLOCATED; allocated_header.next_page_id = INVALID_PAGE; @@ -223,22 +233,40 @@ Result DiskManager::AllocatePage() { return free_page; } else { // Allocate NEW Page by extending the file - Status st = write_empty_page(page_count_); + page_id_t new_page = this->page_count_; + + Status st = write_empty_page(new_page); if (!st.ok()) { return std::unexpected(st); } // Write Header - st = write_page_header(page_count_, allocated_header); + st = write_page_header(new_page, allocated_header); if (!st.ok()) { - LOG_DEBUG("failed to stamp allocated header for new page %d", page_count_); + LOG_DEBUG("failed to stamp allocated header for new page %d", new_page); return std::unexpected(st); } - return this->page_count_++; + + // Publish last. Until this store lands the page fails valid_page(), so a + // concurrent ReadPage cannot observe a page whose header has not been + // stamped yet — and an error on either write above leaves page_count_ + // untouched, so the half-written slot is simply retried by the next + // allocation rather than becoming visible. + this->page_count_ = new_page + 1; + return new_page; } } Status DiskManager::DeallocatePage(page_id_t page_id) { + // Taken before the already-FREE check below, not after it. That check is a read + // of on-disk state that the rest of this function then acts on, so it is the + // start of the read-modify-write, not a precondition outside it: two concurrent + // deallocations of the same page would otherwise both observe a non-FREE header, + // both proceed, and thread the page onto the freelist twice. The result is a + // cycle in the chain, which does not fail here — it fails much later as an + // allocation loop that returns the same page forever. + std::scoped_lock lock(this->meta_latch_); + if (page_id == CATALOG_ROOT_PAGE_ID) { LOG_DEBUG("Can't deallocate page %d: reserved catalog root page", page_id); return Status::InvalidArgument(std::format( @@ -284,10 +312,8 @@ Result DiskManager::read_page_header(page_id_t page_id) { return std::unexpected(Status::InvalidArgument("invalid page")); } std::array buf; - ssize_t status = - pread(this->fd_, buf.data(), PAGE_HEADER_SIZE, static_cast(page_id * PAGE_SIZE)); - if (status != static_cast(PAGE_HEADER_SIZE)) { - return std::unexpected(Status::IOError("unable to read page header")); + if (Status s = full_read(PageOffset(page_id), buf); !s.ok()) { + return std::unexpected(s); } return PageHeader::ReadFrom(buf); @@ -295,28 +321,36 @@ Result DiskManager::read_page_header(page_id_t page_id) { Status DiskManager::write_page_header(page_id_t page_id, const PageHeader& header) { if (!valid_write_target(page_id)) { - return Status::InvalidArgument( - std::format("invalid page id {} for write, page id <= {}", page_id, this->page_count_)); + return Status::InvalidArgument(std::format("invalid page id {} for write, page id <= {}", + page_id, this->page_count_.load())); } + // Identity is stamped HERE, not by callers. Five sites write headers (both reserved pages + // in Open, AllocatePage, DeallocatePage, persist_freelist_head) and every one of them owes + // the same two fields. A page whose header does not carry its own id is indistinguishable + // from a page that landed at the wrong offset, so a single forgetful caller silently + // disables the check for that page forever. One place that knows beats five that remember. + PageHeader stamped = header; + stamped.page_id = page_id; + stamped.format_version = PAGE_FORMAT_VERSION; + std::array buf; - header.WriteTo(buf); - if (pwrite(this->fd_, buf.data(), PAGE_HEADER_SIZE, static_cast(page_id * PAGE_SIZE)) != - static_cast(PAGE_HEADER_SIZE)) { - return Status::Internal(std::format("unable to write page header for page {}", page_id)); + stamped.WriteTo(buf); + if (Status s = full_write(PageOffset(page_id), buf); !s.ok()) { + LOG_DEBUG("failed to write page header for page %d", page_id); + return s; } return Status::OK(); } Status DiskManager::write_empty_page(page_id_t page_id) { if (!valid_write_target(page_id)) { - return Status::InvalidArgument( - std::format("invalid page id {} for write, page id <= {}", page_id, this->page_count_)); + return Status::InvalidArgument(std::format("invalid page id {} for write, page id <= {}", + page_id, this->page_count_.load())); } std::array empty_buf{}; - if (pwrite(this->fd_, empty_buf.data(), PAGE_SIZE, static_cast(page_id * PAGE_SIZE)) != - static_cast(PAGE_SIZE)) { + if (Status s = full_write(PageOffset(page_id), empty_buf); !s.ok()) { LOG_DEBUG("failed to write empty page %d", page_id); - return Status::IOError(std::format("unable to write empty page {}", page_id)); + return s; } return Status::OK(); } @@ -330,5 +364,52 @@ Status DiskManager::persist_freelist_head(page_id_t new_head) { } page_id_t DiskManager::PageCount() const { - return page_count_; + // No latch needed: page_count_ is atomic precisely so this and the bounds checks + // on the ReadPage/WritePage path stay off meta_latch_. + return page_count_.load(); +} + +Status DiskManager::full_write(off_t off, std::span buf) { + std::size_t done = 0; + while (done < buf.size()) { + ssize_t n = + pwrite(this->fd_, buf.data() + done, buf.size() - done, off + static_cast(done)); + if (n < 0) { + // Capture errno before anything else can clobber it — std::format and + // Status construction are both allowed to make library calls. + int err = errno; + if (err == EINTR) continue; + return Status::IOError( + std::format("pwrite at offset {} failed after {} of {} bytes: {}", off, done, + buf.size(), std::system_category().message(err))); + } + done += static_cast(n); + } + return Status::OK(); +} + +Status DiskManager::full_read(off_t off, std::span buf) { + std::size_t done = 0; + while (done < buf.size()) { + ssize_t n = + pread(this->fd_, buf.data() + done, buf.size() - done, off + static_cast(done)); + if (n < 0) { + int err = errno; + if (err == EINTR) continue; + return Status::IOError(std::format("pread at offset {} failed after {} of {} bytes: {}", + off, done, buf.size(), + std::system_category().message(err))); + } + if (n == 0) { + // EOF with bytes still outstanding. Unlike a short read this is terminal: + // the file is shorter than the caller's bounds check believed, so looping + // would never make progress. Corruption rather than IOError — nothing + // went wrong at the syscall level, the file is simply not the size the + // page count says it is. + return Status::Corruption(std::format( + "unexpected EOF at offset {}: wanted {} bytes, got {}", off, buf.size(), done)); + } + done += static_cast(n); + } + return Status::OK(); } diff --git a/src/storage/disk_manager.hpp b/src/storage/disk_manager.hpp index 8b6e6e3..33189c6 100644 --- a/src/storage/disk_manager.hpp +++ b/src/storage/disk_manager.hpp @@ -1,8 +1,12 @@ #pragma once +#include + +#include #include #include #include +#include #include #include "common/page_header.hpp" @@ -36,11 +40,22 @@ namespace kernsql { // PageType::ALLOCATED at the moment it's returned — it will never be mistaken // for FREE or for a real content type before the caller writes its own header. // +// Thread safety, precisely: +// - AllocatePage/DeallocatePage are safe to call concurrently with each other and +// with any other method here. Each holds meta_latch_ across its entire +// read-modify-persist sequence, not just the freelist_head_ assignment: the +// freelist walk and the page-0 head persist have to be atomic against a +// concurrent allocation or deallocation. Two deallocations of the same page that +// interleave would both pass the already-FREE check and thread the page onto the +// list twice, producing a cycle — which surfaces much later as an allocation loop +// that hands out the same page forever. +// - ReadPage/WritePage/PageCount are deliberately latch-free, and stay that way. +// pread/pwrite are atomic per call with respect to the file offset, and +// page_count_ is atomic, so the bounds check races with nothing. Excluding two +// callers from the *same* page is not this layer's problem — that is exactly what +// the buffer pool's per-frame content latch is for (see DD-002). +// // Explicitly NOT guaranteed: -// - Thread safety. No method here is safe to call concurrently with another call -// on the same DiskManager, including two calls touching different pages, since -// freelist_head_/page_count_ are mutated without synchronization. Concurrency -// control (per-page latching) belongs one layer up, in the buffer pool. // - Leak freedom. If a caller lets a page_id from AllocatePage go out of scope // without ever calling DeallocatePage, that page is gone for good — same // contract as malloc/free. DiskManager has no way to detect or reclaim it. @@ -94,6 +109,15 @@ class DiskManager { // catalog root can never be freed or reused, even though it's readable/writable // through ReadPage/WritePage). Does not clear the page's body — only its header // is overwritten to mark it FREE. + // + // CONTRACT: `page_id` must not be resident in the buffer pool when this is + // called. This method stamps the page's on-disk header FREE directly, so any + // cached copy of that page still held in a frame is stale the instant this + // returns — and a later flush of that frame would write the old header back over + // the FREE stamp, resurrecting a page that is on the freelist and simultaneously + // reachable as live data. Callers above this layer therefore never call this + // directly; they go through BufferPoolManager::DeletePage, which vacates the + // frame first and then calls here (see DD-002, "NewPage / DeletePage"). Status DeallocatePage(page_id_t page_id); // fsyncs the underlying file descriptor. This is the only durability @@ -131,12 +155,56 @@ class DiskManager { Status write_empty_page(page_id_t page_id); + // Requires meta_latch_ held. Status persist_freelist_head(page_id_t new_head); + // Byte offset of `page_id` in the file. Exists so the conversion is written + // exactly once: every raw I/O in this class is a (PageOffset(id), span) pair, and + // an offset computed ad hoc at each call site is a place to get the arithmetic + // subtly wrong. The cast precedes the multiply on principle — the product is + // already 64-bit here because PAGE_SIZE is std::size_t, but that is a property of + // a constant in another header, not something this expression should depend on. + // Callers are responsible for rejecting negative page_ids first (valid_page / + // valid_write_target both do); a negative id here yields a negative offset. + [[nodiscard]] + static constexpr off_t PageOffset(page_id_t page_id) { + return static_cast(page_id) * static_cast(PAGE_SIZE); + } + + // Every raw pread/pwrite in this class routes through these two. A short transfer + // is legal for both syscalls and is not an error, and EINTR is a retry rather than + // a failure — handling that at five separate call sites is five chances to get it + // wrong. They also carry real errno text out, so an ENOSPC or EBADF is visible in + // the Status instead of a generic "failed to write page". + Status full_write(off_t off, std::span buf); + + // As full_write, with one asymmetry: a zero-byte return from pread is EOF, not a + // retry condition. It means the file is shorter than the caller's bounds check + // believed, which is corruption; treating it as a short read and looping would + // spin forever on a truncated file. + Status full_read(off_t off, std::span buf); + int fd_; std::filesystem::path path_; + + // Serializes the read-modify-persist sequences in AllocatePage/DeallocatePage. + // See the thread-safety note in the class comment for why the scope is the whole + // operation and not just the freelist_head_ store. + std::mutex meta_latch_; + + // Guarded by meta_latch_. page_id_t freelist_head_{INVALID_PAGE}; - page_id_t page_count_{0}; + + // Written only under meta_latch_, so the read-modify-write when AllocatePage + // extends the file is serialized. Atomic rather than a plain int because it is + // *read* without the latch, by valid_page/valid_write_target on the latch-free + // ReadPage/WritePage path. Default (seq_cst) ordering is deliberate: the store + // that publishes a new page must not be visible before the header write that + // stamped it, or a concurrent reader passes the bounds check and reads a page + // whose header has not been written yet. The load is a plain mov on x86 and the + // store happens once per file extension, next to a syscall — the ordering is + // free at this frequency. + std::atomic page_count_{0}; }; } // namespace kernsql diff --git a/test/buffer/buffer_pool_manager_test.cpp b/test/buffer/buffer_pool_manager_test.cpp new file mode 100644 index 0000000..876aa20 --- /dev/null +++ b/test/buffer/buffer_pool_manager_test.cpp @@ -0,0 +1,1143 @@ +#include "buffer/buffer_pool_manager.hpp" + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "buffer/page_guard.hpp" +#include "buffer/pool_stats.hpp" +#include "common/page_header.hpp" +#include "common/status.hpp" +#include "common/types.hpp" +#include "storage/disk_manager.hpp" + +namespace kernsql { + +class BufferPoolManagerTest : public ::testing::Test { + protected: + // Four frames, not sixteen. Reclaim, pool exhaustion and dirty eviction all have to be + // reachable in a handful of operations, or every test spends its first twenty lines + // manufacturing memory pressure and every failure message requires counting to sixteen. + static constexpr std::size_t kFrames = 4; + + void SetUp() override { + path_ = std::filesystem::temp_directory_path() / + (std::string("kernsql_bpm_test_") + + testing::UnitTest::GetInstance()->current_test_info()->name()); + std::filesystem::remove(path_); // in case a prior crashed run left it behind + + auto dm = DiskManager::Open(path_); + ASSERT_TRUE(dm.has_value()) << dm.error().message(); + dm_ = std::move(dm.value()); + bpm_ = std::make_unique(*dm_, kFrames); + } + + void TearDown() override { + // Shutdown() explicitly rather than letting ~BufferPoolManager's backstop run: the + // backstop aborts on a failed flush, which would take down the whole test binary + // instead of failing one case. Note it still asserts quiescence internally, so a test + // that leaks a pin aborts here — AssertQuiesced() below is what turns that into a + // readable failure at the point it happened. + if (bpm_) { + EXPECT_TRUE(bpm_->Shutdown().ok()); + } + bpm_.reset(); // must die before dm_; it holds a DiskManager& + dm_.reset(); + std::filesystem::remove(path_); + } + + // Call at the END of every test. Most frame-accounting bugs are invisible in the result of + // the operation that caused them and only surface much later as an unexplained + // BufferPoolFull; this turns them into a failure at the point of the mistake. + // + // Only meaningful at quiescence — PoolStats is a sample, not a snapshot (see pool_stats.hpp). + // In a single-threaded test that is automatic; in a concurrent one, call it after joining. + void AssertQuiesced(std::string_view where = "") { + const PoolStats s = bpm_->GetStats(); + SCOPED_TRACE(std::string("AssertQuiesced: ") + std::string(where)); + + // The one that catches leaks: a frame in state Free that is not on the free list is + // unreachable for the rest of the process's life. + EXPECT_EQ(s.free_frames, s.free_list_size) << "a Free frame is missing from the free list"; + EXPECT_EQ(s.pinned_frames, 0u) << "a guard outlived its scope"; + EXPECT_EQ(s.loading_frames, 0u) << "Loading carries a pin by construction"; + EXPECT_EQ(s.failed_frames, 0u) << "Failed carries a pin by construction"; + EXPECT_EQ(s.free_frames + s.resident_frames, kFrames) << "a frame is in no known state"; + EXPECT_LE(s.evictable, s.resident_frames) + << "a non-resident frame is an eviction candidate"; + } + + // Allocates a page through DiskManager and writes `body` into it, bypassing the pool. + // + // It reads the page back before writing so the header AllocatePage stamped survives. That + // is not incidental tidiness: FetchFrame validates page_type and the header's page_id on + // every miss, so a seeded page whose header was zeroed is rejected as unallocated and the + // test fails for a reason that has nothing to do with what it meant to check. + page_id_t SeedPage(std::string_view body) { + auto id = dm_->AllocatePage(); + EXPECT_TRUE(id.has_value()) << (id.has_value() ? "" : id.error().message()); + if (!id.has_value()) return INVALID_PAGE; + + std::array page{}; + EXPECT_TRUE(dm_->ReadPage(*id, page).ok()); + EXPECT_LE(body.size(), PAGE_BODY_SIZE); + std::memcpy(page.data() + PAGE_HEADER_SIZE, body.data(), body.size()); + EXPECT_TRUE(dm_->WritePage(*id, page).ok()); + return *id; + } + + // What is actually on disk, read straight through DiskManager. Deliberately bypasses the + // pool: a test that reads back through the pool proves the cache is self-consistent, not + // that anything was ever written. + std::string ReadBodyFromDisk(page_id_t page_id) { + std::array page{}; + EXPECT_TRUE(dm_->ReadPage(page_id, page).ok()); + const char* body = reinterpret_cast(page.data() + PAGE_HEADER_SIZE); + return std::string(body, ::strnlen(body, PAGE_BODY_SIZE)); + } + + // The header as it is ON DISK, bypassing the pool -- the counterpart to ReadBodyFromDisk, + // and the only way to tell "the pool refused" from "the pool refused after already telling + // DiskManager to free the page". + PageHeader ReadHeaderFromDisk(page_id_t page_id) { + std::array page{}; + EXPECT_TRUE(dm_->ReadPage(page_id, page).ok()); + return PageHeader::ReadFrom( + std::span(page).first()); + } + + // Success only if every byte is zero, naming the first offset that is not. A bare EXPECT + // over 4064 bytes either says nothing useful or says it 4064 times. + static ::testing::AssertionResult AllZero(std::span bytes) { + const auto it = std::ranges::find_if(bytes, [](std::byte b) { return b != std::byte{0}; }); + if (it == bytes.end()) return ::testing::AssertionSuccess(); + return ::testing::AssertionFailure() + << "byte " << (it - bytes.begin()) << " of " << bytes.size() << " is " + << static_cast(*it) << ", expected all zeroes"; + } + + static void WriteBody(WritePageGuard& guard, std::string_view text) { + auto body = guard.MutableBody(); + std::fill(body.begin(), body.end(), std::byte{0}); + std::memcpy(body.data(), text.data(), text.size()); + } + + // std::byte is a distinct type on purpose: no implicit conversion to char or to an + // integer, so "raw memory" can never be silently treated as text. That also means nothing + // in the standard library will compare a span -- or a std::byte* -- to a + // string, and gtest's failure for that is a hundred lines of candidate overloads hiding one + // real error line. Convert explicitly, here, once. + // + // Unlike ReadBody below this does NOT stop at a NUL, so it is what you want for comparing + // an exact-length prefix. + static std::string_view AsChars(std::span bytes) { + return {reinterpret_cast(bytes.data()), bytes.size()}; + } + + static std::string ReadBody(std::span body) { + const char* p = reinterpret_cast(body.data()); + return std::string(p, ::strnlen(p, PAGE_BODY_SIZE)); + } + + // Pins every frame in the pool by fetching kFrames distinct pages and keeping the guards + // alive. Returns them so the caller controls when the pressure is released. + std::vector FillPool() { + std::vector guards; + for (std::size_t i = 0; i < kFrames; ++i) { + const page_id_t id = SeedPage("filler"); + auto g = bpm_->FetchPageRead(id); + EXPECT_TRUE(g.has_value()) << (g.has_value() ? "" : g.error().message()); + if (g.has_value()) guards.push_back(std::move(g.value())); + } + return guards; + } + + std::filesystem::path path_; + std::unique_ptr dm_; + std::unique_ptr bpm_; // declared after dm_, so destroyed before it +}; + +// --- Three starters, each with a known-failing history, so you can confirm the harness is +// --- actually exercising what it looks like it is. + +TEST_F(BufferPoolManagerTest, FreshPoolIsFullyStocked) { + const PoolStats s = bpm_->GetStats(); + EXPECT_EQ(s.capacity, kFrames); + EXPECT_EQ(s.free_frames, kFrames); + EXPECT_EQ(s.free_list_size, kFrames); + EXPECT_EQ(s.resident_frames, 0u); + EXPECT_EQ(s.evictable, 0u); + AssertQuiesced("fresh pool"); +} + +TEST_F(BufferPoolManagerTest, WrittenPageReachesDiskOnFlush) { + const page_id_t id = SeedPage(""); + + { + auto guard = bpm_->FetchPageWrite(id); + ASSERT_TRUE(guard.has_value()) << guard.error().message(); + WriteBody(guard.value(), "durable"); + } // guard drops: dirty_epoch bumped, latch released, pin dropped + + EXPECT_EQ(ReadBodyFromDisk(id), "") << "nothing should reach disk before a flush"; + ASSERT_TRUE(bpm_->FlushPage(id).ok()); + EXPECT_EQ(ReadBodyFromDisk(id), "durable"); + + AssertQuiesced("after flush"); +} + +TEST_F(BufferPoolManagerTest, FetchAfterDeleteIsRejected) { + const page_id_t id = SeedPage("henlo"); + + { + auto guard = bpm_->FetchPageRead(id); + ASSERT_TRUE(guard.has_value()) << guard.error().message(); + EXPECT_EQ(ReadBody(guard->Body()), "henlo"); + } + + ASSERT_TRUE(bpm_->DeletePage(id).ok()); + + // The bytes are still on disk — DeallocatePage only stamps the header FREE — so without + // the miss-path check this silently returns "henlo" and re-caches a deallocated page. + auto again = bpm_->FetchPageRead(id); + EXPECT_FALSE(again.has_value()) << "a deleted page must not be fetchable"; + + AssertQuiesced("after rejected fetch"); +} + +// --- Tier 1: deterministic, single-threaded. Most bugs get caught here. --- + +// Seed a page with known bytes through DiskManager, fetch it, compare. The base case +// everything else assumes. +TEST_F(BufferPoolManagerTest, FetchedBytesMatchWhatIsOnDisk) { + const std::string test_data = "verify bytes read from buffer pool"; + const page_id_t page_id = SeedPage(test_data); + + { + auto page = bpm_->FetchPageRead(page_id); + ASSERT_TRUE(page.has_value()) << page.error().message(); + EXPECT_EQ(AsChars(page->Body().subspan(0, test_data.size())), test_data); + } // scoped so the guard is released before the pool census below + + AssertQuiesced("after fetch"); +} + +// Pin all kFrames frames with live guards, then fetch a page that is not resident. Must fail +// with kBufferPoolFull -- and AssertQuiesced afterwards, because the interesting bug is not +// the error, it is a frame lost on the way to producing it. +TEST_F(BufferPoolManagerTest, ExhaustedPoolReturnsBufferPoolFull) { + auto pages = FillPool(); + auto new_page = bpm_->NewPage(); + ASSERT_FALSE(new_page.has_value()); + ASSERT_EQ(new_page.error().code(), ErrorCode::kBufferPoolFull); + + // Released BEFORE the census, not after: these guards are the test's own memory pressure, + // not a leak, and AssertQuiesced counts pins. Dropping them here is what makes the count + // below mean "the failed NewPage left a frame behind". + pages.clear(); + AssertQuiesced("after NewPage failed on a full pool"); +} + +// Same setup, then drop one guard. The next fetch must succeed -- proving ReclaimFrame found +// the now-unpinned frame through the replacer rather than the free list. +TEST_F(BufferPoolManagerTest, ReleasingAGuardMakesAFrameReclaimable) { + auto pages = FillPool(); + pages.pop_back(); + + // The precondition, asserted rather than assumed: nothing free, one candidate. Without it + // the test still passes if FillPool quietly stopped filling, and would then be proving + // nothing about the replacer. + const PoolStats before = bpm_->GetStats(); + ASSERT_EQ(before.free_list_size, 0u) << "a free frame would let NewPage skip ReclaimFrame"; + ASSERT_EQ(before.pinned_frames, kFrames - 1); + ASSERT_EQ(before.evictable, 1u); + + auto new_page = bpm_->NewPage(); + ASSERT_TRUE(new_page.has_value()) << new_page.error().message(); + + pages.clear(); + new_page->Drop(); + AssertQuiesced("after reclaiming the released frame"); +} + +// Write a page, drop the guard, then force it out by fetching enough other pages. Its bytes +// must be on disk afterwards, read straight through DiskManager. This is the only test that +// exercises the dirty path of ReclaimFrame, which is the one that spans I/O. +TEST_F(BufferPoolManagerTest, DirtyVictimIsWrittenBackWhenEvicted) { + const page_id_t page_id = SeedPage(""); + { + auto write_page = bpm_->FetchPageWrite(page_id); + ASSERT_TRUE(write_page.has_value()) << write_page.error().message(); + WriteBody(write_page.value(), "some data"); + } // guard drops: dirty, unpinned, and now the pool's only eviction candidate + + EXPECT_EQ(ReadBodyFromDisk(page_id), "") << "dropping a guard must not write to disk"; + + // Pigeonhole, not policy. The victim holds one frame and kFrames-1 are free, so fetching + // kFrames other pages and keeping every guard alive leaves the last fetch with no free + // frame and exactly one candidate: it must evict page_id, whatever the sweep would have + // preferred. A loop of "enough" fetches instead bets on the replacer's current usage-count + // policy, and starts flaking the day that policy is tuned. + std::vector pressure; + for (std::size_t i = 0; i < kFrames; ++i) { + const page_id_t other = SeedPage("some other data"); + ASSERT_NE(other, INVALID_PAGE); + auto g = bpm_->FetchPageRead(other); + ASSERT_TRUE(g.has_value()) << g.error().message(); + pressure.push_back(std::move(g.value())); + } + + EXPECT_EQ(ReadBodyFromDisk(page_id), "some data") + << "the dirty victim was evicted without a write-back"; + + pressure.clear(); + AssertQuiesced("after the dirty victim was evicted"); +} + +// The body must be all zeroes and the header must carry this page's own id with page_type +// ALLOCATED. The header half matters: NewPage is the one path that creates page contents +// without reading them, so a missing stamp is only discovered on a later miss. +TEST_F(BufferPoolManagerTest, NewPageIsZeroedAndItsHeaderIsStamped) { + { + auto np = bpm_->NewPage(); + ASSERT_TRUE(np.has_value()) << np.error().message(); + + // EXPECT, not ASSERT: these are independent fields, and the useful output is which of + // them is wrong rather than just that the first one was. + const PageHeader header = np->Header(); + EXPECT_EQ(header.page_id, np->PageId()) + << "the bytes disagree with what the pool believes this frame holds"; + EXPECT_EQ(header.page_type, PageType::ALLOCATED); + EXPECT_EQ(header.format_version, PAGE_FORMAT_VERSION) + << "NewPage stamps this by default-construction, not through write_page_header"; + + // INVALID_PAGE is -1 while the zeroed value is 0, which is META_PAGE_ID: an unstamped + // header does not read as empty, it reads as a link to the superblock -- and + // next_page_id doubles as the disk freelist link. This is the pair that separates + // "stamped" from "merely zeroed". + EXPECT_EQ(header.next_page_id, INVALID_PAGE); + EXPECT_EQ(header.prev_page_id, INVALID_PAGE); + + EXPECT_TRUE(AllZero(np->Body())); + } + + AssertQuiesced("after NewPage"); +} + +// The zero-fill only has anything to do when the frame NewPage acquires is a reclaimed one +// still holding another page's bytes. On a fresh pool the frame comes off the free list with +// memory that was already zero at construction, so the test above stays green even with the +// fill deleted -- this is the one that fails. +TEST_F(BufferPoolManagerTest, NewPageZeroesARecycledFrame) { + // Dirty every frame with recognisable bytes and release them all: the free list is empty + // and every frame is a candidate, so NewPage has no choice but to go through ReclaimFrame. + { + std::vector guards; + for (std::size_t i = 0; i < kFrames; ++i) { + const page_id_t id = SeedPage(""); + ASSERT_NE(id, INVALID_PAGE); + auto g = bpm_->FetchPageWrite(id); + ASSERT_TRUE(g.has_value()) << g.error().message(); + WriteBody(g.value(), "previous tenant"); + guards.push_back(std::move(g.value())); + } + } + + const PoolStats before = bpm_->GetStats(); + ASSERT_EQ(before.free_list_size, 0u) << "the frame NewPage takes must be a reclaimed one"; + ASSERT_EQ(before.evictable, kFrames); + + { + auto np = bpm_->NewPage(); + ASSERT_TRUE(np.has_value()) << np.error().message(); + EXPECT_TRUE(AllZero(np->Body())) << "the previous tenant's bytes survived the reclaim"; + } + + AssertQuiesced("after NewPage on a recycled frame"); +} + +// NewPage with every frame pinned. It acquires a frame BEFORE allocating a page id, so the +// failure path has nothing to unwind -- but assert the page count on disk did not grow, and +// AssertQuiesced. +TEST_F(BufferPoolManagerTest, NewPageOnFullPoolLeaksNothing) { + auto pages = FillPool(); + const page_id_t pages_before = dm_->PageCount(); + + auto np = bpm_->NewPage(); + ASSERT_FALSE(np.has_value()) << "a fully pinned pool cannot hand out a new page"; + EXPECT_EQ(np.error().code(), ErrorCode::kBufferPoolFull); + + // PageCount only moves on AllocatePage's extend path, which makes it a leak detector here + // only because nothing in this test has ever deallocated -- the disk freelist is empty, so + // growth is the only way a leaked allocation can show. Add a DeletePage to the setup and a + // leaked page would be consumed off the freelist instead, invisible to this check, and + // DiskManager exposes no freelist head to assert against. + EXPECT_EQ(dm_->PageCount(), pages_before) << "the failed NewPage allocated a page id anyway"; + + pages.clear(); + AssertQuiesced("after NewPage failed on a full pool"); +} + +// DeletePage while a guard is alive must be refused, and refusal must change nothing: the +// page still fetchable, the frame still resident. +TEST_F(BufferPoolManagerTest, DeletePageOnAPinnedPageIsRejected) { + page_id_t page_id{INVALID_PAGE}; + { + auto guard = bpm_->NewPage(); + ASSERT_TRUE(guard.has_value()) << guard.error().message(); + page_id = guard->PageId(); + + // Deleting a page this same thread holds a guard on is exactly the case DeletePage + // refuses to wait for -- waiting on our own pin would deadlock outright. + const Status st = bpm_->DeletePage(page_id); + ASSERT_EQ(st.code(), ErrorCode::kInvalidArgument) << st.message(); + + // "Changed nothing", asserted rather than hoped. The fetch at the end cannot see this: + // the frame is still mapped, so that fetch is a cache HIT and never touches disk -- a + // DeletePage that vacated the frame anyway would merely turn it into a miss that + // re-reads and still succeeds. + const PoolStats s = bpm_->GetStats(); + EXPECT_EQ(s.resident_frames, 1u) << "the refused delete vacated the frame anyway"; + EXPECT_EQ(s.pinned_frames, 1u); + EXPECT_EQ(s.free_frames, kFrames - 1); + + // The other half the fetch cannot see: a rejection that ran DeallocatePage anyway + // leaves the page on the disk freelist while it is still cached and readable -- free + // stock and live data at once. Only a read straight through DiskManager catches it. + EXPECT_EQ(ReadHeaderFromDisk(page_id).page_type, PageType::ALLOCATED) + << "the refused delete reached the disk allocator"; + } + + { + auto pg = bpm_->FetchPageRead(page_id); + ASSERT_TRUE(pg.has_value()) << pg.error().message(); + } + + AssertQuiesced("after the refused delete"); +} + +// After deleting, free_frames goes up by one and evictable goes DOWN by one. The second half +// is the assertion that matters -- DeletePage is the only place that pulls a frame out of the +// candidate set by hand, and forgetting it puts the frame in the free list and the replacer +// at once, so two threads can be handed the same frame. +TEST_F(BufferPoolManagerTest, DeletePageReturnsTheFrameAndLeavesTheCandidateSet) { + page_id_t page_id{INVALID_PAGE}; + { + auto np = bpm_->NewPage(); + ASSERT_TRUE(np.has_value()) << np.error().message(); + page_id = np->PageId(); + } // guard drops: one frame resident, unpinned, and in the candidate set + + // Absolute values rather than deltas. These counters are unsigned, so a precondition of + // evictable == 0 would make `before.evictable - 1` wrap to SIZE_MAX and report a bug that + // is not the one that fired. + const PoolStats before = bpm_->GetStats(); + ASSERT_EQ(before.free_frames, kFrames - 1); + ASSERT_EQ(before.resident_frames, 1u); + ASSERT_EQ(before.evictable, 1u) << "an unpinned resident frame must be a candidate"; + + // The page is dirty here -- NewPage's write guard bumped the epoch on release -- so this + // also covers the one path in the component where a dirty frame owes no writeback. + const Status st = bpm_->DeletePage(page_id); + ASSERT_TRUE(st.ok()) << st.message(); + + const PoolStats after = bpm_->GetStats(); + EXPECT_EQ(after.free_frames, kFrames); + EXPECT_EQ(after.resident_frames, 0u); + EXPECT_EQ(after.evictable, 0u) << "the freed frame is still an eviction candidate"; + + // The half neither counter above can see: free_frames counts state == Free, which + // DeletePage sets in a different place from free_list_.Push. Delete the Push and both + // EXPECTs above still pass while the frame is unreachable forever -- AssertQuiesced's + // free_frames == free_list_size is the only thing that compares the two. + AssertQuiesced("after deleting an unpinned page"); +} + +// Dirty several pages, call FlushAllPages once, verify every one of them on disk. +TEST_F(BufferPoolManagerTest, FlushAllPagesWritesEveryDirtyPage) { + std::vector pages; + std::string dirty_payload{"dirty the page"}; + for (size_t i = 0; i < kFrames; i++) { + auto np = bpm_->NewPage(); + ASSERT_TRUE(np.has_value()); + pages.push_back(np.value().PageId()); + WriteBody(np.value(), dirty_payload); + } + + auto st = bpm_->FlushAllPages(); + ASSERT_EQ(st.code(), ErrorCode::kOk); + + // verify the content + for (auto pg_id : pages) { + std::string disk_read_value = ReadBodyFromDisk(pg_id); + EXPECT_EQ(disk_read_value, dirty_payload); + } + + AssertQuiesced("after flush pages and read"); +} + +// Two Shutdown() calls in a row: the second is a no-op returning OK, not a second flush and +// not an error. +TEST_F(BufferPoolManagerTest, ShutdownIsIdempotent) { + auto st1 = bpm_->Shutdown(); + EXPECT_EQ(st1.code(), ErrorCode::kOk); + auto st2 = bpm_->Shutdown(); + EXPECT_EQ(st2.code(), ErrorCode::kOk); + + AssertQuiesced("after shutdown"); +} + +// Write through the pool, Shutdown, destroy both manager and DiskManager, reopen the same +// path, read back. The end-to-end durability claim -- and the only test that would catch +// Shutdown flushing but never fsyncing. +TEST_F(BufferPoolManagerTest, DataSurvivesShutdownAndReopen) { + std::vector pages; + std::string payload{"something"}; + for (size_t i = 0; i < 2 * kFrames; i++) { + auto pg = bpm_->NewPage(); + ASSERT_TRUE(pg.has_value()); + pages.push_back(pg.value().PageId()); + WriteBody(pg.value(), payload); + } + auto st = bpm_->Shutdown(); + EXPECT_EQ(st.code(), ErrorCode::kOk); + + bpm_.reset(); + dm_.reset(); + + auto tmp_dm = DiskManager::Open(path_).value(); + dm_ = std::move(tmp_dm); + bpm_ = std::make_unique(*dm_, kFrames); + + for (page_id_t page_id : pages) { + auto pg = bpm_->FetchPageRead(page_id); + ASSERT_TRUE(pg.has_value()); + EXPECT_EQ(AsChars(pg.value().Body().subspan(0, payload.size())), payload); + } +} + +// Fetching a page id past the end of the file must fail cleanly. The real assertion is +// AssertQuiesced: the frame acquired for the load has to come back. +TEST_F(BufferPoolManagerTest, FetchPastEndOfFileLeaksNothing) { + for (size_t i = 0; i < kFrames; i++) { + auto write_page = bpm_->NewPage(); + EXPECT_TRUE(write_page.has_value()); + } + + auto pg = bpm_->FetchPageRead(kFrames * 3); + ASSERT_FALSE(pg.has_value()); + ASSERT_EQ(pg.error().code(), ErrorCode::kInvalidArgument); +} + +// --- Tier 2: the failed-load path. Nothing below has ever run. --- + +// Trigger a read error without a mock: truncate the file underneath the pool with +// std::filesystem::resize_file, then fetch a page that used to exist. AbandonLoad runs. +// Assert the error surfaces and the frame is back -- that is the pin_count == 0 half of +// AbandonLoad's disposal, which was wrong once already. +TEST_F(BufferPoolManagerTest, FailedLoadReturnsErrorAndReclaimsTheFrame) { + page_id_t page_id{INVALID_PAGE}; + { + auto write_page = bpm_->NewPage(); + ASSERT_TRUE(write_page.has_value()); + page_id = write_page.value().PageId(); + WriteBody(write_page.value(), "some data that should get lost"); + } + FillPool(); + + std::filesystem::resize_file( + path_, static_cast(page_id) * PAGE_SIZE); // truncate from this page onwards + + auto read_page = bpm_->FetchPageRead(page_id); + ASSERT_FALSE(read_page.has_value()); + ASSERT_EQ(read_page.error().code(), ErrorCode::kCorruption); + + AssertQuiesced("after failed load"); +} + +// The same, with several threads fetching the SAME page so some are asleep on Loading when +// the loader publishes Failed. Exercises the waiter drain, UnpinPage's notify on Failed, and +// AbandonLoad's pin_count == 1 wait. Every thread must get an error, and the pool must be +// whole after joining. If the notify is ever dropped this test hangs rather than fails, so +// give it a timeout. +TEST_F(BufferPoolManagerTest, FailedLoadWakesEveryWaiter) { + // Fixed, not hardware_concurrency(): that is allowed to return 0, which would spawn no + // threads and pass vacuously, and it would otherwise make the laptop and CI run different + // tests. Rounds, because a single round is one sample of one interleaving. + constexpr std::size_t kThreads = 8; + constexpr std::size_t kRounds = 32; + + // Victims are seeded straight through DiskManager, so they exist on disk and were never + // resident: every fetch below is a guaranteed miss that reaches the file. Creating them + // with NewPage instead would need each one evicted first, and evicting a dirty page + // rewrites it -- re-extending the very file this test is about to truncate. + std::vector victims; + for (std::size_t i = 0; i < kRounds; ++i) { + const page_id_t id = SeedPage(""); + ASSERT_NE(id, INVALID_PAGE); + victims.push_back(id); + } + + // From the first victim onwards. Everything below it -- the meta page and the catalog root + // -- survives, so TearDown's Shutdown still has a well-formed file underneath it. + std::filesystem::resize_file(path_, static_cast(victims.front()) * PAGE_SIZE); + + std::size_t loads_observed = 0; + std::size_t waits_observed = 0; + + for (const page_id_t victim : victims) { + // One slot per thread, sized before any thread starts. Each thread writes only its own + // index, so there is nothing to lock; aggregating happens on the main thread after the + // join. A shared counter here would need a mutex and would tell you less. + std::vector results(kThreads, Status::OK()); + + // Every thread blocks until the last one arrives, so they storm FetchPageRead together + // rather than trickling in after the loader has already failed and cleaned up. This is + // what makes a thread asleep on Loading likely at all. + std::latch gate(static_cast(kThreads)); + + std::vector threads; + threads.reserve(kThreads); + for (std::size_t i = 0; i < kThreads; ++i) { + threads.emplace_back([&, i] { + gate.arrive_and_wait(); + auto page = bpm_->FetchPageRead(victim); + + results[i] = page.has_value() ? Status::OK() : page.error(); + }); + } + for (auto& t : threads) t.join(); + + for (std::size_t i = 0; i < kThreads; ++i) { + const Status& st = results[i]; + + // The invariant that always holds: a page whose bytes are gone is fetchable by + // nobody, whatever order the threads ran in. + EXPECT_FALSE(st.ok()) << "thread " << i << " got a guard for page " << victim + << ", which cannot be read"; + + // Three codes are legal, and which one a thread gets is decided by the interleaving + // -- asserting any single one of them would be asserting on a race: + // kCorruption the loader; its pread hit EOF on the truncated file + // kIOError a waiter; it slept on Loading and woke to find Failed + // kBufferPoolFull it lost the race for a frame -- the loader's frame is pinned + // and the rest are momentarily held by other retrying threads, + // so the replacer has no candidate to offer + // Anything else is the bug this test is looking for. + EXPECT_TRUE(st.code() == ErrorCode::kCorruption || st.code() == ErrorCode::kIOError || + st.code() == ErrorCode::kBufferPoolFull) + << "thread " << i << " on page " << victim << ": " << st.message(); + + if (st.code() == ErrorCode::kCorruption) ++loads_observed; + if (st.code() == ErrorCode::kIOError) ++waits_observed; + } + + // Per round rather than once at the end. Every thread has joined, so the pool really is + // quiescent and PoolStats is meaningful -- and a frame lost in round 3 is a failure in + // round 3, not an unexplained kBufferPoolFull in round 30. + AssertQuiesced("after a failed concurrent load"); + } + + // Guaranteed: the free list is full at the start of every round, so somebody always wins a + // frame, publishes the mapping and reaches the read. + EXPECT_GT(loads_observed, 0u) << "no thread ever got as far as the disk read"; + + // NOT asserted, deliberately. The failing read is one pread that returns in microseconds + // and there is no fault-injection hook to widen that window, so no amount of hammering can + // GUARANTEE a thread is asleep on Loading when Failed is published. Asserting on it would + // be asserting on a race -- the definition of a flaky test. Recorded instead, so a run that + // exercised the drain is distinguishable from one that did not. + RecordProperty("waiter_wakeups_observed", static_cast(waits_observed)); + if (waits_observed == 0) { + GTEST_LOG_(WARNING) << "no thread was ever asleep on Loading: the waiter drain, " + "UnpinPage's notify on Failed and AbandonLoad's pin_count == 1 " + "wait were NOT exercised in this run"; + } +} + +// --- Tier 3: concurrency. Run under TSan; these prove nothing on their own. --- +// +// A stress test that passes a thousand times says nothing about the thousand-and-first +// interleaving. What makes them worth running is TSan, which checks happens-before rather +// than outcomes and so flags a race even in a run whose timing happened to work out. +// Assert AssertQuiesced() after joining, not just "did not crash". + +// N threads fetching one page. Everyone who gets the page must see identical bytes, and the page +// may occupy only one frame. +// +// Not "all 32 must succeed". A miss calls AcquireFrame() BEFORE re-checking the shard, so in the +// opening stampede up to kFrames threads can each be holding a frame while they race to publish +// the mapping; the next one along finds the free list empty and every frame taken, and is refused +// with kBufferPoolFull. That is documented behaviour (buffer_pool_manager.hpp, "Errors"), not a +// failure, and it fires roughly 2 runs in 15 under asan. What must never happen is a thread +// getting the page and reading the wrong bytes, or the page landing in two frames. +TEST_F(BufferPoolManagerTest, ConcurrentFetchesOfOnePageShareOneFrame) { + std::string data{"all must see this"}; + page_id_t page_id = SeedPage(data); + + size_t kThreads{32}; + std::vector threads; + threads.reserve(kThreads); + + // Status and bytes kept separately. Folding a failure into the body string ("something else") + // loses which status it was, and kBufferPoolFull is the only legal one here -- there is no + // deleter, the page exists, and its header is valid, so kInvalidArgument, kIOError or + // kCorruption would each be a real bug wearing the same disguise. + struct Outcome { + Status status = Status::OK(); + std::string body{}; + }; + + std::latch gate(static_cast(kThreads)); + std::vector outcomes(kThreads); + + for (size_t i = 0; i < kThreads; i++) { + threads.emplace_back([&, i] { + gate.arrive_and_wait(); + auto read_page = bpm_->FetchPageRead(page_id); + if (read_page.has_value()) + outcomes[i].body = AsChars(read_page.value().Body().subspan(0, data.size())); + else + outcomes[i].status = read_page.error(); + }); + } + + for (auto& t : threads) t.join(); + + size_t granted = 0; + for (size_t i = 0; i < kThreads; i++) { + SCOPED_TRACE(std::format("thread {}", i)); + if (!outcomes[i].status.ok()) { + EXPECT_EQ(outcomes[i].status.code(), ErrorCode::kBufferPoolFull) + << "the only legal refusal here is a full pool, got: " + << outcomes[i].status.message(); + continue; + } + granted++; + EXPECT_EQ(data, outcomes[i].body); + } + + // The thread that published the mapping owns the frame and cannot be refused, so a run in + // which nobody got the page means something other than pool pressure went wrong. + EXPECT_GT(granted, 0u) << "every fetch was refused; this run proved nothing"; + RecordProperty("fetches_granted", static_cast(granted)); + + ASSERT_EQ(bpm_->GetStats().resident_frames, 1); // every thread pins the page to a single frame + AssertQuiesced("after releasing all guards"); +} + +// N threads fetching randomly from a working set larger than the pool, so the replacer is +// under constant pressure. Assert every read returns that page's own seeded bytes -- a frame +// handed to two threads shows up as one thread reading another page's content. +TEST_F(BufferPoolManagerTest, ConcurrentFetchesUnderPressureNeverCrossPages) { + // The working set is six times the pool, so almost every fetch is a miss and the replacer + // is evicting continuously -- which is the state this test needs. A working set that fits + // would make every fetch a cache hit and prove nothing about reclaim. + constexpr std::size_t kWorkingSet = 24; + constexpr std::size_t kThreads = 8; + constexpr std::size_t kIterations = 500; + constexpr std::uint32_t kSeed = 0x5EED; + + // Each page carries its own index in its body, so a thread that is handed the wrong frame + // reads a string that names the page it actually got. The failure message can then print + // both sides instead of "some bytes differed". + std::vector page_ids; + std::vector expected; + for (std::size_t i = 0; i < kWorkingSet; ++i) { + expected.push_back(std::format("data for page {}", i)); + const page_id_t id = SeedPage(expected.back()); + ASSERT_NE(id, INVALID_PAGE); + page_ids.push_back(id); + } + + // One record per thread, written only by that thread and read only after the join, so + // nothing here needs a lock. Counts plus the FIRST bad observation: a crossed read under + // this much pressure would repeat thousands of times, and one well-described instance is + // worth more than a flood. + struct Outcome { + std::size_t ok{0}; + std::size_t pool_full{0}; + std::size_t crossings{0}; + page_id_t crossed_page{INVALID_PAGE}; + std::string want; + std::string got; + Status unexpected = Status::OK(); + }; + std::vector outcomes(kThreads); + + std::latch gate(static_cast(kThreads)); + std::vector threads; + threads.reserve(kThreads); + + for (std::size_t i = 0; i < kThreads; ++i) { + threads.emplace_back([&, i] { + // Per-thread generator. std::mt19937 is not thread-safe -- drawing from one shared + // engine is every thread mutating the same internal state, which is both a data + // race TSan will report and a quiet correlation of the streams. Seeded from a fixed + // base plus the thread index rather than random_device, so a failure reproduces. + std::mt19937 gen(kSeed + static_cast(i)); + std::uniform_int_distribution pick(0, kWorkingSet - 1); + + Outcome& out = outcomes[i]; + gate.arrive_and_wait(); + + for (std::size_t n = 0; n < kIterations; ++n) { + const std::size_t idx = pick(gen); + auto page = bpm_->FetchPageRead(page_ids[idx]); + + if (!page.has_value()) { + // Legal and expected: kThreads guards contend for kFrames frames, so a + // thread that finds the free list empty with every frame pinned is told the + // pool is full. Any OTHER error is a real failure -- recorded, not asserted, + // because a gtest fatal assertion off the main thread would only return + // from this lambda. + if (page.error().code() == ErrorCode::kBufferPoolFull) { + ++out.pool_full; + } else if (out.unexpected.ok()) { + out.unexpected = page.error(); + } + continue; + } + ++out.ok; + + // The whole point. A frame handed to two threads at once shows up here as one + // of them reading a different page's bytes under this page's id. + std::string body = ReadBody(page->Body()); + if (body != expected[idx]) { + if (out.crossings == 0) { + out.crossed_page = page_ids[idx]; + out.want = expected[idx]; + out.got = std::move(body); + } + ++out.crossings; + } + } // guard drops here, before the next iteration -- one pin per thread at a time + }); + } + for (auto& t : threads) t.join(); + + std::size_t total_ok = 0, total_full = 0, total_crossed = 0; + for (std::size_t i = 0; i < kThreads; ++i) { + const Outcome& out = outcomes[i]; + total_ok += out.ok; + total_full += out.pool_full; + total_crossed += out.crossings; + + EXPECT_TRUE(out.unexpected.ok()) + << "thread " << i + << " got an error that is not kBufferPoolFull: " << out.unexpected.message(); + EXPECT_EQ(out.crossings, 0u) + << "thread " << i << " read page " << out.crossed_page << " and got another page's " + << "bytes -- want \"" << out.want << "\", got \"" << out.got << "\""; + } + + // Not a property of the pool, a guard against the test quietly doing nothing: if every + // fetch were rejected, the per-thread loop above would pass without having read a byte. + EXPECT_GT(total_ok, 0u) << "every fetch failed; this run proved nothing"; + EXPECT_EQ(total_crossed, 0u); + + RecordProperty("fetches_ok", static_cast(total_ok)); + RecordProperty("fetches_pool_full", static_cast(total_full)); + + AssertQuiesced("after concurrent pressure"); +} + +// N threads calling NewPage. Every returned page id must be distinct. This is the test that +// catches a duplicate page-table mapping, which today aborts inside ShardGuard::Insert. +TEST_F(BufferPoolManagerTest, ConcurrentNewPageReturnsDistinctPages) { + constexpr std::size_t kThreads = 32; + + // One slot per thread: the page id on success, the error otherwise. Deliberately not a + // shared set behind a mutex -- that lock would serialise the threads at precisely the + // moment they are supposed to collide, and merging after the join costs nothing. + struct Outcome { + page_id_t page_id{INVALID_PAGE}; + Status status = Status::OK(); + }; + std::vector outcomes(kThreads); + + const page_id_t pages_before = dm_->PageCount(); + std::latch gate(static_cast(kThreads)); + + // A reader for the frame metadata NewPage publishes, and a deliberate part of the test + // rather than scaffolding. Without it this race is INVISIBLE: a frame taken off the free + // list is unreachable through both the page table and the replacer, so no allocating + // thread ever touches another's frame and TSan only ever sees one side of the access. + // GetStats is a genuine concurrent caller -- any metrics endpoint does exactly this -- and + // it walks every frame by index under each frame's metadata mutex, which is the reader + // NewPage's stores have to be paired against. + std::atomic sampling{true}; + std::thread sampler([&] { + while (sampling.load(std::memory_order_relaxed)) (void)bpm_->GetStats(); + }); + + std::vector threads; + threads.reserve(kThreads); + for (std::size_t i = 0; i < kThreads; ++i) { + threads.emplace_back([&, i] { + gate.arrive_and_wait(); + auto page = bpm_->NewPage(); + if (page.has_value()) { + outcomes[i].page_id = page->PageId(); + } else { + outcomes[i].status = page.error(); + } + }); // the guard dies here, before the join, so no pin outlives the census below + } + for (auto& t : threads) t.join(); + sampling.store(false, std::memory_order_relaxed); + sampler.join(); + + std::set distinct; + std::size_t granted = 0; + for (std::size_t i = 0; i < kThreads; ++i) { + const Outcome& out = outcomes[i]; + if (out.page_id == INVALID_PAGE) { + // kThreads threads contend for kFrames frames and each holds its guard for the + // length of the call, so most of these are refused. That is the pool working, not + // failing -- but any OTHER code is a real bug. + EXPECT_EQ(out.status.code(), ErrorCode::kBufferPoolFull) + << "thread " << i << ": " << out.status.message(); + continue; + } + ++granted; + distinct.insert(out.page_id); + } + + // Distinctness is the property that holds under EVERY interleaving. The count is not: + // asserting granted == kThreads would be asserting on the scheduler, and it fails the + // moment the threads genuinely overlap. A duplicate would also abort inside + // ShardGuard::Insert before reaching here -- this is the softer variant of that backstop. + EXPECT_EQ(distinct.size(), granted) << "two threads were handed the same page id"; + + // Guards against a run where every call was refused, which would make the check above + // vacuously true. At least one thread always wins: the free list is full at the start. + EXPECT_GT(granted, 0u) << "every NewPage was refused; this run proved nothing"; + + // The allocator and the pool must agree. A refused NewPage must consume no page id -- it + // acquires its frame BEFORE calling AllocatePage, so there is nothing to unwind -- and a + // granted one must consume exactly one. + EXPECT_EQ(static_cast(dm_->PageCount() - pages_before), granted) + << "the disk allocator and the pool disagree about how many pages were handed out"; + + RecordProperty("new_pages_granted", static_cast(granted)); + AssertQuiesced("after concurrent NewPage"); +} + +// One thread deleting while others fetch the same page. +// +// Every operation must either succeed or fail with a status from the legal set below, the pool's +// frame accounting must survive the race, and -- the headline -- a fetch that succeeds must return +// THIS round's bytes. Not an earlier round's, not another page's. +// +// That last assertion only became true with the stale-mapping fix (DD-002, "Known behaviour"). +// Before it, DeletePage erased the page-table mapping, dropped both locks, and only then called +// DeallocatePage. A fetcher that missed inside that window read a header still stamped ALLOCATED, +// passed the miss-path validation, and republished a Resident mapping for a page about to land on +// the disk freelist; the next round's SeedPage recycled that id, and the fetch was a cache hit on +// the stale frame -- round N reading round N-1's bytes, at 1-5% of successful fetches and ~6% of +// rounds. DeletePage now holds the shard lock across the deallocation, so no new mapping for the +// page can be published until its header says FREE, at which point the miss-path check rejects it. +// fetch_stale and stale_after_delete are the regression detectors: both must be zero, and both +// were reliably non-zero before the fix. +// +// Expect spurious DeletePage rejections -- that is documented behaviour, not a failure. +TEST_F(BufferPoolManagerTest, ConcurrentDeleteAndFetchStayConsistent) { + struct FetcherOutcome { + Status status = Status::OK(); + std::string res{}; + }; + + struct DeleterOutcome { + Status status = Status::OK(); + size_t rejections{0}; + }; + + // Rounds are cheap now that the threads outlive them (see the barrier below), so the count + // is set by how many chances a fetcher gets to land inside DeletePage's window -- between + // the shard lock dropping and DeallocatePage stamping the header FREE -- not by what a + // spawn-per-round loop could afford. Turn it down if a sanitizer build gets tedious; that + // is the only reason to. + constexpr size_t kRounds{500}; + constexpr size_t kDeleterMaxRetries{50}; + constexpr size_t kDeleters{1}, kFetchers{8}; + constexpr size_t kThreads{kDeleters + kFetchers}; + + struct Result { + DeleterOutcome del_res; + std::array fetch_res; + }; + + std::vector results(kRounds); + + // The round's page, written ONLY by the barrier's completion function and read by every + // thread after the barrier releases them. No atomic and no lock: the completion runs on one + // thread with all others blocked, and the phase transition is the happens-before edge. + page_id_t page_id = INVALID_PAGE; + size_t next_round = 0; + + // How often the hazard in the header comment actually fired: the delete landed, yet the page + // was still fetchable afterwards. Counted, not asserted. + size_t stale_after_delete = 0; + + // Whether the round's delete landed, published for the completion function below. The + // completion runs on whichever thread arrives LAST, so reading the deleter's results slot + // from there is a cross-thread read. The barrier's arrive-to-completion edge makes that well + // defined, but TSan cannot see through libc++'s backoff spin and reports it as a race ("As if + // synchronized via sleep"). An explicit release/acquire flag costs nothing and keeps the + // sanitizer output honest. + std::atomic delete_landed{false}; + + // Runs once per phase, at the one moment in the test that is provably quiescent: every + // thread has finished the previous round and none has been released into the next. That + // makes it the right home for the per-round postconditions as well as the seeding. + // + // Must be noexcept -- std::barrier requires it, and an escaping exception is a terminate. + auto begin_round = [&]() noexcept { + // The previous round's checks, before page_id is overwritten. Skipped once anything has + // failed: a frame lost in round 7 would otherwise fail identically in all 493 rounds after + // it, and the flood buries the one report that named the cause. + if (next_round > 0 && !::testing::Test::HasFailure()) { + // Every guard is destroyed before its thread arrives here, so a non-zero pin count at + // this point is a leak and not a straggler. + AssertQuiesced(std::format("after round {}", next_round - 1)); + + if (delete_landed.load(std::memory_order_acquire)) { + // The stale-mapping probe. A deleted page SHOULD be unfetchable -- that is what + // FetchAfterDeleteIsRejected asserts single-threaded, and a leaked mapping is the + // bug class that let AbandonLoad's shard.Erase go missing without a single test + // noticing. Under this race it can legitimately succeed, so it is a counter rather + // than an expectation. + auto probe = bpm_->FetchPageRead(page_id); + stale_after_delete += probe.has_value(); + } + } + + page_id = SeedPage(std::format("round {} page", next_round)); + ++next_round; + }; + + // Reusable where std::latch is single-use, which is the whole reason the threads had to be + // respawned every round before. One barrier, kRounds phases: thread creation leaves the hot + // loop and stops dominating the window this test is trying to hit. + std::barrier sync(static_cast(kThreads), begin_round); + + std::vector threads; + threads.reserve(kThreads); + + // Threads 0..kDeleters-1 delete, the rest fetch. + for (size_t i = 0; i < kThreads; i++) { + threads.emplace_back([&, i] { + for (size_t round = 0; round < kRounds; round++) { + // Releases only once begin_round() has seeded this round's page, so it is both + // the start gun and the previous round's join. + sync.arrive_and_wait(); + + if (i < kDeleters) { + auto& out = results[round].del_res; + for (size_t attempt = 0; attempt < kDeleterMaxRetries; attempt++) { + out.status = bpm_->DeletePage(page_id); + if (out.status.ok()) break; + out.rejections++; + // A rejection means someone holds a transient pin. Re-taking the shard + // lock immediately just fights them for it; yield so the pin can drop. + std::this_thread::yield(); + } + delete_landed.store(out.status.ok(), std::memory_order_release); + } else { + auto& out = results[round].fetch_res[i - kDeleters]; + auto read_page = bpm_->FetchPageRead(page_id); + if (read_page.has_value()) + out.res = ReadBody(read_page->Body()); // NUL-terminated: the seeded + // prefix, not 4064 raw bytes + else + out.status = read_page.error(); + } + } + }); + } + + for (auto& t : threads) t.join(); + + size_t fetch_ok = 0, fetch_stale = 0, fetch_failed = 0, delete_ok = 0, rejections = 0; + + for (size_t round = 0; round < kRounds; round++) { + const std::string expected = std::format("round {} page", round); + SCOPED_TRACE(std::format("round {}", round)); + + // kInvalidArgument means two different things depending on who returned it -- "pinned" + // from a delete, "not allocated" from a fetch -- which is why the two roles are audited + // separately and never compared. + const Status& del = results[round].del_res.status; + EXPECT_TRUE(del.ok() || del.code() == ErrorCode::kInvalidArgument) + << "the only legal delete failure is the pinned rejection, got: " << del.message(); + delete_ok += del.ok(); + rejections += results[round].del_res.rejections; + + for (const FetcherOutcome& f : results[round].fetch_res) { + if (!f.status.ok()) { + fetch_failed++; + // kInvalidArgument: the loader read a header already stamped FREE. + // kIOError: a waiter on a frame whose loader then abandoned it. + // kBufferPoolFull: frame contention. + // Anything else -- kCorruption above all, which means the header's page_id + // disagreed with the id that was asked for -- is a real bug. + EXPECT_TRUE(f.status.code() == ErrorCode::kInvalidArgument || + f.status.code() == ErrorCode::kIOError || + f.status.code() == ErrorCode::kBufferPoolFull) + << "fetch failed with a status outside the legal set: " << f.status.message(); + continue; + } + + fetch_ok++; + + // The headline. Anything but this round's string means the pool served bytes for a + // page id that no longer means what the fetcher asked for -- a stale mapping for a + // deallocated page, or a crossed frame. + EXPECT_EQ(f.res, expected) << "a successful fetch returned the wrong page's bytes"; + fetch_stale += (f.res != expected); + } + } + + // Vacuity guards. A run in which every fetch was refused, or in which no delete ever landed, + // is green and worthless: the window this test exists to open was never open. + EXPECT_GT(fetch_ok, 0u) << "every fetch was refused; this run proved nothing"; + EXPECT_GT(delete_ok, 0u) << "no delete ever landed; the post-delete window never opened"; + + // Both were reliably non-zero before the stale-mapping fix; see the header comment. They are + // redundant with the per-fetch EXPECT_EQ above and the probe, and kept because a count is what + // tells you a regression is rare-and-real rather than a one-off. + EXPECT_EQ(fetch_stale, 0u) << "a fetch was served an earlier round's bytes"; + EXPECT_EQ(stale_after_delete, 0u) + << "a page was still fetchable through the pool after its delete landed"; + + RecordProperty("fetch_ok", static_cast(fetch_ok)); + RecordProperty("fetch_stale", static_cast(fetch_stale)); + RecordProperty("fetch_failed", static_cast(fetch_failed)); + RecordProperty("delete_ok", static_cast(delete_ok)); + RecordProperty("delete_rejections", static_cast(rejections)); + RecordProperty("stale_after_delete", static_cast(stale_after_delete)); + + AssertQuiesced("after concurrent delete and fetch"); +} + +} // namespace kernsql diff --git a/test/buffer/replacer_test.cpp b/test/buffer/replacer_test.cpp new file mode 100644 index 0000000..c4d396e --- /dev/null +++ b/test/buffer/replacer_test.cpp @@ -0,0 +1,282 @@ +#include "buffer/replacer.hpp" + +#include + +#include +#include +#include +#include +#include +#include + +#include "common/types.hpp" + +namespace kernsql { + +// No fixture needed — Replacer has no external state (no files, no fds), so plain +// TEST() throughout. Every test constructs its own Replacer at the capacity the +// scenario needs; small capacities (2-4) keep the clock traces small enough to +// verify by hand in the comments below. +// +// A note on method: the Replacer exposes no getters, so none of these tests peek +// at usage counts directly. Each policy test is instead built so that the +// *sequence of victims* differs between a correct and a buggy implementation — +// the observable behavior is the eviction order, same way the DiskManager tests +// observe on-disk bytes rather than private fields. + +// --------------------------------------------------------------------------- +// Evict() — empty / no candidates +// --------------------------------------------------------------------------- + +TEST(ReplacerTest, EvictOnFreshReplacerReturnsNullopt) { + // Construct Replacer(8) and call Evict() immediately -> expect nullopt. + // Proves both the evictable_count_ == 0 fast path and that frames start + // life as non-candidates (nothing is evictable until SetEvictable says so). + + auto replacer = Replacer(8); + ASSERT_EQ(std::nullopt, replacer.Evict()); +} + +TEST(ReplacerTest, AccessesAloneDoNotCreateCandidates) { + // RecordAccess a handful of frames several times each, but never call + // SetEvictable. Evict() -> expect nullopt. Proves usage history and + // candidate membership are independent axes — a hot page that's pinned + // must never be evicted no matter how its counter looks. + + auto replacer = Replacer(4); + std::size_t recordCount = 5; + for (std::size_t i = 0; i < recordCount; i++) { + replacer.RecordAccess(0); + replacer.RecordAccess(1); + replacer.RecordAccess(3); + } + ASSERT_EQ(std::nullopt, replacer.Evict()); +} + +// --------------------------------------------------------------------------- +// Evict() — victim post-conditions +// --------------------------------------------------------------------------- + +TEST(ReplacerTest, EvictsTheOnlyCandidateThenGoesEmpty) { + // Replacer(4); SetEvictable(2, true); Evict() -> expect frame 2. + // Then Evict() again -> expect nullopt. The second call is the important + // half: it proves Evict removed its victim from the candidate set itself + // (the frozen post-condition), rather than leaving that to the caller. + + auto replacer = Replacer(4); + replacer.SetEvictable(2, true); + + ASSERT_EQ(2, replacer.Evict()); + ASSERT_EQ(std::nullopt, replacer.Evict()); +} + +TEST(ReplacerTest, EvictedFrameCanBeReaddedAndEvictedAgain) { + // Same start as above, but after the first eviction call + // SetEvictable(2, true) again — this is exactly what the BPM does when + // the frame's new page gets unpinned. Evict() -> expect frame 2 again. + // Proves eviction doesn't permanently retire a frame id. + + auto replacer = Replacer(4); + replacer.SetEvictable(2, true); + ASSERT_EQ(2, replacer.Evict()); + + replacer.SetEvictable(2, true); + ASSERT_EQ(2, replacer.Evict()); +} + +// --------------------------------------------------------------------------- +// Evict() — clock policy +// --------------------------------------------------------------------------- + +TEST(ReplacerTest, ColdCandidatesEvictInClockOrder) { + // Replacer(4); mark all four frames evictable, no accesses anywhere. + // Four Evict() calls -> expect victims 0, 1, 2, 3 in that order, then a + // fifth call -> nullopt. Pins down the hand's starting position and + // direction, which every trace-based test below depends on. + + auto replacer = Replacer(4); + for (frame_id_t i = 0; i < 4; i++) { + replacer.SetEvictable(i, true); + } + + for (frame_id_t i = 0; i < 4; i++) { + ASSERT_EQ(i, replacer.Evict()); + } + + ASSERT_EQ(std::nullopt, replacer.Evict()); +} + +TEST(ReplacerTest, AccessedFrameGetsSecondChance) { + // Replacer(3); all three evictable; RecordAccess(1) once. + // Expect eviction order: 0, 2, 1. + // Trace: Evict#1 finds 0 at count 0 -> victim, hand moves to 1. + // Evict#2: frame 1 has count 1 -> decremented to 0, hand passes on, + // frame 2 at count 0 -> victim. Evict#3: 1 is now at 0 -> victim. + // This is the second-chance property in its smallest observable form. + auto replacer = Replacer(3); + for (frame_id_t i = 0; i < 3; i++) { + replacer.SetEvictable(i, true); + } + replacer.RecordAccess(1); + + ASSERT_EQ(0, replacer.Evict()); + ASSERT_EQ(2, replacer.Evict()); + ASSERT_EQ(1, replacer.Evict()); +} + +TEST(ReplacerTest, UsageCountCapsAtThree) { + // Replacer(2); RecordAccess(0) fifty times, RecordAccess(1) exactly + // three times (== kMaxUsageCount); mark both evictable. + // Evict() -> expect frame 0. + // Why this discriminates: with the cap working, both frames sit at 3 and + // the sweep walks them down together — 0 reaches zero first purely by + // hand order (trace: 3/3 -> 2/2 -> 1/1 -> 0/0, hand lands on 0). If the + // cap leaked, frame 0 would sit at 50 and outlast frame 1, making 1 the + // first victim instead. One assertion, and it fails precisely when the + // CAS loop's cap check is broken. + + auto replacer = Replacer(2); + for (int i = 0; i < 50; i++) replacer.RecordAccess(0); + for (int i = 0; i < 3; i++) replacer.RecordAccess(1); + + replacer.SetEvictable(0, true); + replacer.SetEvictable(1, true); + ASSERT_EQ(0, replacer.Evict()); +} + +TEST(ReplacerTest, PinnedHotFrameKeepsItsProtection) { + // Proves the sweep passes over non-evictable frames WITHOUT touching + // their usage history — DD-002: "skipped without decrementing; its + // history is preserved." + // + // Replacer(2). + // Setup round: RecordAccess(0) three times, but leave 0 non-evictable + // (it's "pinned"). SetEvictable(1, true); RecordAccess(1) once. + // Evict() -> expect 1. During this sweep the hand passes frame 0 twice; + // a buggy implementation decrements it both times (3 -> 1), a correct + // one leaves it at 3. + // Second round: SetEvictable(0, true) ("unpinned" now), + // SetEvictable(1, true), RecordAccess(1) once. + // Evict() -> expect 1 again. Correct trace: 0 enters at 3, gets walked + // 3->2->... while 1 (at 1) reaches zero first. Buggy trace: 0 entered at + // 1, reaches zero first, and 0 is wrongly evicted — the pinned-hot page + // lost the protection it earned before being pinned. + // Optional third Evict() -> expect 0, confirming it was still there. + + auto replacer = Replacer(2); + for (int i = 0; i < 3; i++) replacer.RecordAccess(0); + replacer.SetEvictable(0, false); + + replacer.RecordAccess(1); + replacer.SetEvictable(1, true); + + ASSERT_EQ(1, replacer.Evict()); + + replacer.SetEvictable(0, true); + + replacer.SetEvictable(1, true); + replacer.RecordAccess(1); + + ASSERT_EQ(1, replacer.Evict()); + ASSERT_EQ(0, replacer.Evict()); +} + +// --------------------------------------------------------------------------- +// SetEvictable() — idempotency / bookkeeping +// --------------------------------------------------------------------------- +// Heads-up for both tests below: the failure mode of broken evictable_count_ +// bookkeeping is not a wrong value but a HANG — Evict's fallback loop spins +// looking for a candidate the count claims exists. A test that never finishes +// IS the failure signal here; if you want it bounded, run ctest with +// --timeout N. + +TEST(ReplacerTest, RedundantMarkEvictableDoesNotInflateTheCount) { + // Replacer(2); SetEvictable(0, true) three times in a row; Evict() -> + // expect 0; Evict() -> expect nullopt. If the flip-check is missing, + // evictable_count_ is 3, the second Evict believes candidates remain, + // finds none, and never returns. + GTEST_SKIP(); +} + +TEST(ReplacerTest, RedundantClearOnFreshFrameIsHarmless) { + // Replacer(2); SetEvictable(0, false) on a frame that is already + // non-evictable (the initial state); then Evict() -> expect nullopt. + // Guards the other direction: without the flip-check, the size_t + // evictable_count_ underflows to a huge value and Evict hangs exactly + // as above. + GTEST_SKIP(); +} + +// --------------------------------------------------------------------------- +// Concurrency — run this under the tsan build +// --------------------------------------------------------------------------- + +TEST(ReplacerTest, HammerConcurrentAccessAndEviction) { + // The one test whose real assertions come from ThreadSanitizer: it + // exercises the lock-free RecordAccess path racing the locked Evict path, + // which is exactly the interleaving the atomic usage_count_ decision + // exists to make safe. A green run under build/debug proves little; + // build/tsan is the point. + // + // Shape: + // - Replacer(64); mark all frames evictable up front. + // - A std::vector> claimed(64), all false — the + // test's own shadow of "who owns this frame right now." + // - 4 accessor threads: loop ~100k times calling RecordAccess on a + // pseudo-random frame id. (Deliberately including currently-claimed + // frames — a stale RecordAccess racing an eviction is the benign + // race DD-002 explicitly accepts, so the test should generate it.) + // - 2 evictor threads: loop calling Evict(). On nullopt, just retry. + // On a victim v: EXPECT_FALSE(claimed[v].exchange(true)) — this is + // the invariant that matters, no frame handed to two threads at + // once — then claimed[v] = false and SetEvictable(v, true) to feed + // it back into the pool, the same recycle the BPM performs. + // - Join everything; no final state assertion needed — the exchange + // check and tsan's race detector are the verdict. + // Keep iteration counts high enough to force interleavings but low + // enough that the tsan build finishes in seconds, not minutes. + + constexpr std::size_t kFrames = 64; + Replacer replacer(kFrames); + std::vector> claimed(kFrames); + + for (frame_id_t i = 0; i < static_cast(kFrames); ++i) { + replacer.SetEvictable(i, true); + } + + for (int iter = 0; iter < 1000; ++iter) { + auto victim = replacer.Evict(); + if (!victim) continue; + const auto v = static_cast(*victim); + EXPECT_FALSE(claimed[v].exchange(true)); + claimed[v].store(false); + replacer.SetEvictable(*victim, true); + } + + auto evictor = [&]() { + for (int iter = 0; iter < 20000; ++iter) { + auto victim = replacer.Evict(); + if (!victim) continue; + const auto v = static_cast(*victim); + EXPECT_FALSE(claimed[v].exchange(true)); + claimed[v].store(false); + replacer.SetEvictable(*victim, true); + } + }; + + auto accessor = [&](unsigned seed) { + std::mt19937 rng(seed); + std::uniform_int_distribution dist(0, static_cast(kFrames) - 1); + for (int iter = 0; iter < 20000; ++iter) { + replacer.RecordAccess(dist(rng)); + } + }; + + std::vector threads; + threads.reserve(6); + for (int i = 0; i < 4; ++i) threads.emplace_back(accessor, static_cast(i + 1)); + for (int i = 0; i < 2; ++i) threads.emplace_back(evictor); + for (auto& t : threads) t.join(); +} + +} // namespace kernsql diff --git a/test/storage/disk_manager_test.cpp b/test/storage/disk_manager_test.cpp index 08ba4dd..b2bb5ed 100644 --- a/test/storage/disk_manager_test.cpp +++ b/test/storage/disk_manager_test.cpp @@ -5,10 +5,14 @@ #include #include +#include #include #include #include +#include #include +#include +#include #include "common/page_header.hpp" #include "common/status.hpp" @@ -28,6 +32,37 @@ class DiskManagerTest : public ::testing::Test { std::filesystem::path path_; }; +// Allocates until the file has to grow, returning every page_id the freelist handed +// back on the way — i.e. it drains the freelist and stops at the first allocation +// that extends the file. +// +// This exists because the obvious freelist test cannot see the failure that matters. +// "Deallocate twice, then allocate once" passes even when the second deallocation has +// threaded the page onto the chain a second time, because the first allocation off a +// self-linked page looks perfectly normal. The cycle only shows up on the *next* +// allocation, as either a repeated id or the Corruption error AllocatePage raises when +// a page on the freelist isn't stamped FREE. Draining catches both, and the iteration +// cap turns an actual infinite chain into a failed assertion instead of a hung test. +inline std::vector DrainFreelist(DiskManager& dm) { + std::vector reused; + const page_id_t page_count_before = dm.PageCount(); + + while (reused.size() <= 1024) { + auto id = dm.AllocatePage(); + if (!id.has_value()) { + ADD_FAILURE() << "AllocatePage failed while draining the freelist: " + << id.error().message(); + return reused; + } + // This allocation extended the file, so the freelist was already empty. + if (dm.PageCount() != page_count_before) return reused; + reused.push_back(id.value()); + } + + ADD_FAILURE() << "freelist did not drain after 1024 allocations — the chain has a cycle"; + return reused; +} + // --------------------------------------------------------------------------- // Open() — fresh file // --------------------------------------------------------------------------- @@ -43,7 +78,7 @@ TEST_F(DiskManagerTest, DiskManagerAllocatesMetaAndCatalogPage) { // to check what Open() actually put on disk. int fd = open(path_.c_str(), O_RDONLY); ASSERT_GE(fd, 0); - std::array meta_buf; + std::array meta_buf{}; ASSERT_EQ(pread(fd, meta_buf.data(), PAGE_HEADER_SIZE, 0), static_cast(PAGE_HEADER_SIZE)); close(fd); @@ -52,7 +87,7 @@ TEST_F(DiskManagerTest, DiskManagerAllocatesMetaAndCatalogPage) { // page 1 (catalog root) IS readable through the public API, so this part // goes through ReadPage like a real caller would. - std::array catalog_page; + std::array catalog_page{}; ASSERT_TRUE(dm.value()->ReadPage(CATALOG_ROOT_PAGE_ID, catalog_page).ok()); PageHeader catalog_header = PageHeader::ReadFrom(std::span(catalog_page).first()); @@ -72,7 +107,7 @@ TEST_F(DiskManagerTest, ContentOnCatalogPageSurvivesReopen) { auto dm = DiskManager::Open(DiskManagerTest::path_); ASSERT_TRUE(dm.has_value()); - std::array page; + std::array page{}; ASSERT_TRUE(dm.value()->ReadPage(CATALOG_ROOT_PAGE_ID, page).ok()); const char* text = "My Unique testing bytes"; @@ -86,7 +121,7 @@ TEST_F(DiskManagerTest, ContentOnCatalogPageSurvivesReopen) { dm = DiskManager::Open(DiskManagerTest::path_); ASSERT_TRUE(dm.has_value()); - std::array catalog_page; + std::array catalog_page{}; ASSERT_TRUE(dm.value()->ReadPage(CATALOG_ROOT_PAGE_ID, catalog_page).ok()); ASSERT_EQ(std::memcmp(catalog_page.data() + PAGE_HEADER_SIZE, text, std::strlen(text)), 0); @@ -137,7 +172,11 @@ TEST_F(DiskManagerTest, OpenRejectsSizeNotMultipleOfPageSize) { std::filesystem::resize_file(DiskManagerTest::path_, PAGE_SIZE - 6); dm = DiskManager::Open(DiskManagerTest::path_); - ASSERT_EQ(dm.error().code(), Status::Corruption("doesn't matter, only comparing codes").code()); + // Assert the failure *before* touching .error(): calling error() on an expected + // that holds a value is UB, so without this the failure mode of a regression here + // is a garbage read rather than a red test. + ASSERT_FALSE(dm.has_value()) << "Open() unexpectedly succeeded on a corrupt file"; + ASSERT_EQ(Status::Corruption("doesn't matter, only comparing codes").code(), dm.error().code()); } TEST_F(DiskManagerTest, OpenRejectsFileWithOnlyOnePage) { @@ -153,7 +192,11 @@ TEST_F(DiskManagerTest, OpenRejectsFileWithOnlyOnePage) { std::filesystem::resize_file(DiskManagerTest::path_, PAGE_SIZE); dm = DiskManager::Open(DiskManagerTest::path_); - ASSERT_EQ(dm.error().code(), Status::Corruption("doesn't matter, only comparing codes").code()); + // Assert the failure *before* touching .error(): calling error() on an expected + // that holds a value is UB, so without this the failure mode of a regression here + // is a garbage read rather than a red test. + ASSERT_FALSE(dm.has_value()) << "Open() unexpectedly succeeded on a corrupt file"; + ASSERT_EQ(Status::Corruption("doesn't matter, only comparing codes").code(), dm.error().code()); } TEST_F(DiskManagerTest, OpenRejectsCorruptMetaPageType) { @@ -174,7 +217,7 @@ TEST_F(DiskManagerTest, OpenRejectsCorruptMetaPageType) { PageHeader corrupt_header; corrupt_header.page_type = PageType::HEAP; - std::array meta_buf; + std::array meta_buf{}; corrupt_header.WriteTo(meta_buf); ASSERT_EQ(pwrite(fd, meta_buf.data(), PAGE_HEADER_SIZE, 0), @@ -183,7 +226,11 @@ TEST_F(DiskManagerTest, OpenRejectsCorruptMetaPageType) { close(fd); dm = DiskManager::Open(path_); - ASSERT_EQ(dm.error().code(), Status::Corruption("doesn't matter, only comparing codes").code()); + // Assert the failure *before* touching .error(): calling error() on an expected + // that holds a value is UB, so without this the failure mode of a regression here + // is a garbage read rather than a red test. + ASSERT_FALSE(dm.has_value()) << "Open() unexpectedly succeeded on a corrupt file"; + ASSERT_EQ(Status::Corruption("doesn't matter, only comparing codes").code(), dm.error().code()); } TEST_F(DiskManagerTest, OpenRejectsCorruptCatalogPageType) { @@ -200,7 +247,7 @@ TEST_F(DiskManagerTest, OpenRejectsCorruptCatalogPageType) { PageHeader corrupt_header; corrupt_header.page_type = PageType::HEAP; - std::array catalog_buf; + std::array catalog_buf{}; corrupt_header.WriteTo(catalog_buf); ASSERT_EQ(pwrite(fd, catalog_buf.data(), PAGE_HEADER_SIZE, PAGE_SIZE), @@ -209,7 +256,44 @@ TEST_F(DiskManagerTest, OpenRejectsCorruptCatalogPageType) { close(fd); dm = DiskManager::Open(path_); - ASSERT_EQ(dm.error().code(), Status::Corruption("doesn't matter, only comparing codes").code()); + // Assert the failure *before* touching .error(): calling error() on an expected + // that holds a value is UB, so without this the failure mode of a regression here + // is a garbage read rather than a red test. + ASSERT_FALSE(dm.has_value()) << "Open() unexpectedly succeeded on a corrupt file"; + ASSERT_EQ(Status::Corruption("doesn't matter, only comparing codes").code(), dm.error().code()); +} + +TEST_F(DiskManagerTest, OpenRejectsZeroedMetaPage) { + // An all-zero file is already caught, but not by the meta-page check: zeros read + // as page_type 0, and page 1's CATALOG check is what rejects it. So the meta page + // itself is unguarded against zeros, and this test isolates that by zeroing page 0 + // while leaving page 1 a perfectly valid catalog page. + // + // This is the case PageType::INVALID = 0 exists for. With META at enum value 0, a + // zeroed page 0 validated as a real meta page and Open() *succeeded* — then + // recovered freelist_head_ from the zeroed next_page_id, i.e. 0, putting the + // reserved meta page itself at the head of the freelist for the next + // AllocatePage() to hand out. + // + // Still a mitigation, not a format check: a file whose first byte happens to equal + // PageType::META passes regardless. A magic + version field in the meta page is + // the real fix and is not written yet. + int fd = open(path_.c_str(), O_RDWR | O_CREAT, 0644); + ASSERT_GE(fd, 0); + ASSERT_EQ(0, ftruncate(fd, 2 * static_cast(PAGE_SIZE))); + + PageHeader catalog_header; + catalog_header.page_type = PageType::CATALOG; + std::array catalog_buf{}; + catalog_header.WriteTo(catalog_buf); + ASSERT_EQ(pwrite(fd, catalog_buf.data(), PAGE_HEADER_SIZE, PAGE_SIZE), + static_cast(PAGE_HEADER_SIZE)); + fsync(fd); + close(fd); + + auto dm = DiskManager::Open(path_); + ASSERT_FALSE(dm.has_value()) << "Open() accepted a file whose meta page is all zeros"; + ASSERT_EQ(Status::Corruption("doesn't matter, only comparing codes").code(), dm.error().code()); } // --------------------------------------------------------------------------- @@ -223,7 +307,7 @@ TEST_F(DiskManagerTest, ReadPageRejectsMetaPage) { auto dm = DiskManager::Open(path_); ASSERT_TRUE(dm.has_value()); - std::array out; + std::array out{}; ASSERT_EQ(dm.value()->ReadPage(META_PAGE_ID, out).code(), Status::InvalidArgument("doesn't matter, only comparing codes").code()); } @@ -234,7 +318,7 @@ TEST_F(DiskManagerTest, WritePageRejectsMetaPage) { auto dm = DiskManager::Open(path_); ASSERT_TRUE(dm.has_value()); - std::array in; + std::array in{}; ASSERT_EQ(dm.value()->WritePage(META_PAGE_ID, in).code(), Status::InvalidArgument("doesn't matter, only comparing codes").code()); } @@ -249,14 +333,14 @@ TEST_F(DiskManagerTest, ReadWritePageRoundTripsOnCatalogPage) { auto dm = DiskManager::Open(path_); ASSERT_TRUE(dm.has_value()); - std::array catalog_buf; + std::array catalog_buf{}; ASSERT_TRUE(dm.value()->ReadPage(CATALOG_ROOT_PAGE_ID, catalog_buf).ok()); const char* text = "My Unique testing bytes"; std::memcpy(catalog_buf.data() + PAGE_HEADER_SIZE, text, strlen(text)); ASSERT_TRUE(dm.value()->WritePage(CATALOG_ROOT_PAGE_ID, catalog_buf).ok()); - std::array fresh_buf; + std::array fresh_buf{}; ASSERT_TRUE(dm.value()->ReadPage(CATALOG_ROOT_PAGE_ID, fresh_buf).ok()); ASSERT_EQ(0, std::memcmp(catalog_buf.data(), fresh_buf.data(), PAGE_SIZE)); } @@ -268,7 +352,7 @@ TEST_F(DiskManagerTest, ReadPageRejectsOutOfRangePageId) { auto dm = DiskManager::Open(path_); ASSERT_TRUE(dm.has_value()); - std::array buf; + std::array buf{}; Status st = dm.value()->ReadPage(INVALID_PAGE, buf); ASSERT_EQ(Status::InvalidArgument("doesn't matter").code(), st.code()); @@ -282,7 +366,7 @@ TEST_F(DiskManagerTest, WritePageRejectsOutOfRangePageId) { auto dm = DiskManager::Open(path_); ASSERT_TRUE(dm.has_value()); - std::array buf; + std::array buf{}; Status st = dm.value()->WritePage(INVALID_PAGE, buf); ASSERT_EQ(Status::InvalidArgument("doesn't matter").code(), st.code()); @@ -318,7 +402,7 @@ TEST_F(DiskManagerTest, AllocatedPageHeaderIsStampedAllocated) { auto res = dm.value()->AllocatePage(); ASSERT_TRUE(res.has_value()); - std::array buf; + std::array buf{}; ASSERT_TRUE(dm.value()->ReadPage(res.value(), buf).ok()); auto ph = PageHeader::ReadFrom(std::span(buf).first()); @@ -424,4 +508,169 @@ TEST_F(DiskManagerTest, DeallocatePageRejectsOutOfRangePageId) { ASSERT_EQ(Status::InvalidArgument("doesn't matter").code(), st.code()); } +// --------------------------------------------------------------------------- +// Freelist structure +// --------------------------------------------------------------------------- + +TEST_F(DiskManagerTest, FreelistReuseIsLifo) { + // The freelist is a stack threaded through each free page's next_page_id, with + // page 0 holding the head, so the most recently freed page must come back first. + // AllocateReusesFreedPageBeforeExtending only frees one page, which cannot tell + // LIFO from FIFO from "returns an arbitrary free page." + + auto dm = DiskManager::Open(path_); + ASSERT_TRUE(dm.has_value()); + DiskManager& disk = *dm.value(); + + std::vector allocated; + for (int i = 0; i < 3; i++) { + auto id = disk.AllocatePage(); + ASSERT_TRUE(id.has_value()) << id.error().message(); + allocated.push_back(id.value()); + } + + for (page_id_t id : allocated) { + ASSERT_TRUE(disk.DeallocatePage(id).ok()); + } + + std::vector expected(allocated.rbegin(), allocated.rend()); + EXPECT_EQ(expected, DrainFreelist(disk)); +} + +TEST_F(DiskManagerTest, DoubleDeallocateDoesNotCycleFreelist) { + // The already-FREE check in DeallocatePage is what stops a page being threaded + // onto the chain twice. DeallocatingAlreadyFreePageIsNoOp proves the second call + // returns OK and that one allocation still works; it does not prove the chain is + // intact, because a self-linked page survives exactly one allocation. Draining is + // what distinguishes them. + + auto dm = DiskManager::Open(path_); + ASSERT_TRUE(dm.has_value()); + DiskManager& disk = *dm.value(); + + auto first = disk.AllocatePage(); + ASSERT_TRUE(first.has_value()); + auto second = disk.AllocatePage(); + ASSERT_TRUE(second.has_value()); + + ASSERT_TRUE(disk.DeallocatePage(second.value()).ok()); + ASSERT_TRUE(disk.DeallocatePage(second.value()).ok()); // no-op, must not re-thread + + // Exactly one page on the list, once. A cycle would show up as a repeat, a + // Corruption error, or a drain that never terminates — DrainFreelist fails on all + // three. + EXPECT_EQ(std::vector{second.value()}, DrainFreelist(disk)); +} + +// --------------------------------------------------------------------------- +// Short/interrupted I/O +// --------------------------------------------------------------------------- + +TEST_F(DiskManagerTest, ReadPageOnTruncatedFileReportsCorruption) { + // Exercises full_read's EOF branch, which is the one case it must NOT treat as a + // short read: pread returning 0 with bytes outstanding means the file is smaller + // than page_count_ claims, and looping would never make progress. If this + // regresses, the symptom is a hung test rather than a failed one — which is + // precisely why the distinction is worth a test. + + auto dm = DiskManager::Open(path_); + ASSERT_TRUE(dm.has_value()); + DiskManager& disk = *dm.value(); + + auto id = disk.AllocatePage(); + ASSERT_TRUE(id.has_value()); + ASSERT_TRUE(disk.Sync().ok()); + + // Truncate the page away behind the open handle. page_count_ is in-memory state, + // so the bounds check still passes and the read runs straight off the end. + std::filesystem::resize_file(path_, static_cast(id.value()) * PAGE_SIZE); + + std::array buf{}; + Status st = disk.ReadPage(id.value(), buf); + EXPECT_FALSE(st.ok()); + EXPECT_EQ(Status::Corruption("doesn't matter").code(), st.code()); +} + +// --------------------------------------------------------------------------- +// Concurrency +// --------------------------------------------------------------------------- + +TEST_F(DiskManagerTest, ConcurrentAllocateWriteReadDeallocateIsRaceFree) { + // The point of this test is meta_latch_ and the atomic page_count_, so it must be + // run under tsan to mean anything — without it, an unsynchronized freelist_head_ + // passes here most of the time. + // + // Each thread only ever touches the page it currently owns, so the absence of + // per-page locking in DiskManager is deliberately not under test; that exclusion + // is the buffer pool's job. What is under test is the shared allocator state. + + auto dm = DiskManager::Open(path_); + ASSERT_TRUE(dm.has_value()); + DiskManager& disk = *dm.value(); + + constexpr int kThreads = 8; + constexpr int kIterations = 50; + + std::atomic failures{0}; + std::vector threads; + threads.reserve(kThreads); + + for (int t = 0; t < kThreads; t++) { + threads.emplace_back([&disk, &failures, t] { + for (int i = 0; i < kIterations; i++) { + auto id = disk.AllocatePage(); + if (!id.has_value()) { + failures++; + return; + } + + // Write a real header, not zeros: a zeroed header reads back as + // PageType::INVALID, and the DeallocatePage below would then be + // operating on a page whose type says something this test never + // intended. + std::array buf{}; + PageHeader header; + header.page_type = PageType::HEAP; + header.WriteTo(std::span(buf).first()); + buf[PAGE_HEADER_SIZE] = static_cast(t); + + if (!disk.WritePage(id.value(), buf).ok()) { + failures++; + return; + } + + std::array out{}; + if (!disk.ReadPage(id.value(), out).ok()) { + failures++; + return; + } + // Nobody else can be holding this page, so our own bytes must survive + // the round trip. A torn read here would mean two threads were handed + // the same page_id. + if (out[PAGE_HEADER_SIZE] != static_cast(t)) { + failures++; + return; + } + + if (!disk.DeallocatePage(id.value()).ok()) { + failures++; + return; + } + } + }); + } + + for (std::thread& thread : threads) { + thread.join(); + } + ASSERT_EQ(0, failures.load()); + + // Every page allocated above was freed again, so the chain must drain cleanly. + // Interleaved deallocations that lost an update would leave either a duplicate or + // a cycle. + std::vector reused = DrainFreelist(disk); + std::set distinct(reused.begin(), reused.end()); + EXPECT_EQ(reused.size(), distinct.size()) << "freelist handed out a duplicate page_id"; +} + } // namespace kernsql