feat(db): wo_wal_read_row_at — materialise a row from a log offset

Task 5b of docs/superpowers/plans/2026-08-26-table-residency.md, whose Task 5
is now split 5a-5d (plan updated in this commit).

- the offset twin of wo_row_read (table.c:721): same out-gate contract —
  every value handed back is a FRESH VM allocation — but resolved from a file
  position instead of the id hash
- fits entirely in wal.c because everything it needs was already public:
  scan_record and dec_val are local, and wo_val_decode_vm / wo_db_val_free
  are exported at table.h:183-187. Two decode stages, since the record and
  the VM speak different dialects: dec_val -> engine slots -> VM copies, with
  the engine slots freed as scratch on every path
- ZERO storage change. Nothing calls it yet; that is the point of separating
  it from 5c, so the read path can be proven before the slabs are touched
- refuses rather than guessing, each case distinguishable: no intact record
  at the offset, a malformed header, a decode failure, trailing bytes, and a
  REMOVE tombstone. That last one matters most — handing a tombstone back as
  a row would read a deleted row as live

Tested by deep field comparison, not by "it parsed": 24 rows with a nil Text
every third row, each read back BY OFFSET and compared field by field,
including the string bytes. Plus all three refusal paths — tombstone, a
mid-record offset (the silent-wrong-row failure this guards), and past the
intact prefix.

The free-on-every-path claim is VERIFIED, not assumed: removing the free
produced 3 LeakSanitizer reports; restoring it returns to 0. Worth doing
because "ASan is clean" only means something if the harness would have
complained.

PLAN SPLIT: Task 5's storage half was written as if it were plumbing. Measured
instead: wo_row_ptr returns a db_row* into a slab with 11 call sites, table.c
has 37 slab references, db.c:105-181 scans slabs directly, enc_val serialises
FROM the slab, and no operation exists that drops a payload while keeping
index entries. Note this is the OPPOSITE half from the earlier retraction —
the record FORMAT needed nothing, the record STORAGE genuinely is deep. 5c
(id->offset map + drop-payload-keep-index) and 5d (rewiring the call sites,
scans, @unique/FK across the boundary) get their own write-ups.

Gates: test_wal 3654/0 (was 3428), all 18 runtime suites 0 fail under
ASan+UBSan, oop-e2e 119/0, residency 8/0, employee 8/0, db-actor 8/0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
shoney.arickathil 2026-08-27 10:27:40 +02:00
parent 3290c7d117
commit 18a56f24ec
4 changed files with 249 additions and 44 deletions

View file

@ -450,6 +450,85 @@ static int apply_record(wo_db *db, const uint8_t *payload, uint32_t len) {
return 0;
}
/* databasev2 2: materialise a row straight from a log offset.
*
* The offset twin of wo_row_read (table.c): same out-gate contract — every
* value handed back is a FRESH VM allocation, never a pointer into anything
* the engine owns — but resolved from a file position instead of the id hash.
* This is what a `resident: keys` table's read path will call once 5c gives
* it an id->offset map; nothing calls it yet, deliberately.
*
* Two decode stages, because the record and the VM speak different dialects:
* dec_val yields ENGINE-owned slots (db_text and friends, exactly what a slab
* row holds), then wo_val_decode_vm copies each into the VM. The engine slots
* are scratch and are always freed before returning, on every path. */
int wo_wal_read_row_at(wo_wal *w, wo_db *db, wo_rt *rt, uint64_t off,
uint32_t *class_out, uint64_t *id_out, uint64_t *out_vals,
const char **msg) {
uint32_t len = 0;
uint8_t *payload = NULL;
if (scan_record(w->fd, off, &len, &payload) != 0) {
*msg = "no intact record at that offset";
return -1;
}
rbuf r = {payload, payload + len, 0};
uint8_t kind = rd_u8(&r);
uint32_t cid = rd_u32(&r);
uint64_t id = rd_u64(&r);
if (r.bad || cid >= db->class_cnt) {
free(payload);
*msg = "record header is malformed";
return -1;
}
/* A REMOVE tombstone carries no field payload. Handing one back as a row
* would be the worst failure available here — the caller would read a
* deleted row as live — so it is refused explicitly, not decoded. */
if (kind != WO_WAL_INSERT && kind != WO_WAL_UPDATE) {
free(payload);
*msg = "record at that offset is a tombstone, not a row";
return -1;
}
const wo_classdesc *c = &db->classes[cid];
uint64_t *slots = c->field_cnt ? calloc(c->field_cnt, sizeof *slots) : NULL;
if (c->field_cnt && !slots) {
free(payload);
*msg = "out of memory reading a row";
return -2;
}
int rc = 0;
uint32_t done = 0;
for (; done < c->field_cnt; done++) {
if (dec_val(&r, db, c->kinds[done], &slots[done]) != 0) {
*msg = "record at that offset does not decode";
rc = -1;
break;
}
}
if (rc == 0 && (size_t)(r.end - r.p) != 0) { /* trailing bytes = corrupt */
*msg = "record at that offset has trailing bytes";
rc = -1;
}
if (rc == 0) {
int ok = 1;
for (uint32_t i = 0; i < c->field_cnt; i++) {
out_vals[i] = wo_val_decode_vm(db, rt, c->kinds[i], slots[i], &ok, msg);
if (!ok) {
rc = -2;
break;
}
}
}
/* engine slots are scratch: free every one that was built, on EVERY path */
for (uint32_t i = 0; i < done; i++) wo_db_val_free(db, c->kinds[i], slots[i]);
free(slots);
free(payload);
if (rc == 0) {
if (class_out) *class_out = cid;
if (id_out) *id_out = id;
}
return rc;
}
int64_t wo_wal_replay_ex(const char *path, wo_db *db, uint32_t *volatile_cid) {
int fd = open(path, O_RDONLY);
if (fd < 0) return errno == ENOENT ? 0 : -1; /* no WAL yet = fresh boot */

View file

@ -110,6 +110,25 @@ int64_t wo_wal_replay(const char *path, wo_db *db);
* callers and the 156 WAL unit checks are untouched. */
int64_t wo_wal_replay_ex(const char *path, wo_db *db, uint32_t *volatile_cid);
/* databasev2 2: read one row straight from a log offset — the offset twin of
* wo_row_read. [out_vals] must have room for the class's field_cnt values and
* receives FRESH VM allocations (the out-gate: always copies). [class_out] and
* [id_out] are optional. Offsets come from wo_wal_next_offset, recorded at
* append time.
*
* 0 ok
* -1 no intact record at that offset, a malformed header, a record that
* does not decode, trailing bytes, or a REMOVE tombstone (which carries
* no fields — refused rather than decoded, since returning a deleted row
* as live is the worst outcome available here)
* -2 out of memory (*msg set)
*
* Nothing in the engine calls this yet: it is the read half of `resident:
* keys`, landed ahead of the storage change so it can be tested alone. */
int wo_wal_read_row_at(wo_wal *w, wo_db *db, wo_rt *rt, uint64_t off,
uint32_t *class_out, uint64_t *id_out, uint64_t *out_vals,
const char **msg);
/* Offline verification (no engine): scan [path], count intact records.
* *intact_bytes (optional) = where the intact prefix ends. -1 = open
* failure. */

View file

@ -202,7 +202,32 @@ create fixtures under `tests/corpus/run/`.
---
## Task 5 — the `resident: keys` read path
## Task 5 — the `resident: keys` read path (split into 5a–5d)
> **SPLIT 2026-08-27, after 5a shipped.** This task was written as if the
> storage side were plumbing on existing functions. It is not, and that was
> measured rather than guessed: `wo_row_ptr` returns a `db_row *` into a slab
> and has **11 call sites**; `table.c` has 37 slab references; the query path
> walks slabs directly (`db.c:105-181`); `enc_val` serialises *from* the slab,
> so an insert must materialise, append, commit and only then drop the payload;
> and **no operation exists that drops a row's payload while keeping its index
> entries** — `wo_row_remove` removes from the indexes too.
>
> Note this is the *opposite* half from the earlier retraction. The record
> FORMAT genuinely needed nothing (retracted, correctly). The record STORAGE
> genuinely is deep, and leaving the step list reading as light plumbing was
> the residual error.
>
> | Sub-task | Scope | State |
> | --- | --- | --- |
> | **5a** | `wo_wal_next_offset` — exact record offsets, unit-proven | ✅ landed `ac7d8af` |
> | **5b** | `wo_wal_read_row_at` — materialise a row from an offset into VM values. **Zero storage change**, so it is additive and independently testable | this section |
> | **5c** | the id→offset map for `resident: keys` classes, plus the missing drop-payload-keep-index operation | not started |
> | **5d** | rewiring `wo_row_ptr`'s call sites, the slab scans, and `@unique`/FK across the residency boundary | not started |
>
> Only 5b is described below. 5c and 5d need their own task write-ups once 5b
> has shown what the read path actually costs.
> **Retraction, 2026-08-26.** This plan originally had a Task 5 that rewrote
> `db_val_encode`/`db_val_decode` into a "self-contained, offset-based" record
@ -232,50 +257,40 @@ slab path), `database/src/db.c` (insert/read/update/delete), `database/src/wal.c
- Consumes: Task 3's descriptor fields, and `wal.c`'s existing `enc_val`/`dec_val`/`scan_record` — see the note below.
- Produces: a table whose rows are not resident, serving reads by offset.
- [ ] **Offset capture first, before any map exists.** Make the staging path in
`wal.c` able to report the file offset a record will occupy. Decide between
computing it as the buffer's base file offset plus the record's position
within the buffer, or deferring the report until flush; whichever is chosen,
the offset must be wrong in *no* case — a wrong offset reads a neighbouring
record and passes its CRC.
- [ ] Prove offset capture in isolation before it has a consumer: a unit test
that appends records straddling a buffer boundary, flushes, then reads each
back by its reported offset via `scan_record` and asserts the recovered id
matches the one appended. Include a failed-commit case, where no offset must
be published for a record that never reached disk.
- [ ] For a `resident: keys` table, replace the slab retention with an
id→offset map. Keep every index resident: the id map, each secondary index,
and each `@unique` shadow. That residency is what makes the arithmetic work
(≈3.8 GB of index for a 120 GB table) and what makes the constraints
correct.
- [ ] Insert: append the record as Task 4 leaves it, then record id→offset
instead of retaining a slab row.
- [ ] Read by id: map lookup, `pread` at the offset, verify the CRC the frame
already carries, decode via `dec_val`. Use `pread` and **not** `O_DIRECT` — the
kernel page cache is deliberately the hot copy.
- [ ] Update: append a new record, repoint the offset. The superseded record
becomes garbage; do not attempt reclamation here — that is databasev2 3.
- [ ] Delete: append a tombstone, drop the id from the map and from every
index.
- [ ] Scan: walk the log sequentially rather than issuing one `pread` per row,
because a sequential walk is the case this layout is best at and a per-row
read would make scans pathological.
- [ ] Boot: rebuild the id→offset map by replaying the log. Correct, and
O(entire history) — record that cost in the iteration and note that
databasev2 3's snapshot must persist the map so boot stops rescanning.
- [ ] Confirm the constraints hold across the boundary, with a fixture each:
`@unique` refuses a duplicate whose conflicting row is not resident, and
FK-restrict refuses a delete whose only referrer is not resident. These two
are the correctness core; a constraint that silently checks only resident
rows must never ship.
- [ ] Confirm `ref` navigation still works, at the cost of a read per hop.
- [ ] Add a fixture: a table whose row count exceeds any plausible resident
budget, read back by id and scanned in full, byte-identical.
- [ ] Verify: `just oop-e2e`, `just employee`, `just db-actor` green; ASan
clean; **`just db-bench` shows the `resident: all` read baseline unmoved**.
- [ ] Commit. Draft: `feat(db): resident:keys — rows read from the log by offset`.
### 5b — read a row from an offset
---
**Files:** modify `database/src/wal.c` (beside `scan_record` at :272 and
`dec_val` at :167), `database/src/wal.h`; extend `runtime/test/test_wal.c`.
**Interfaces:**
- Consumes: 5a's `wo_wal_next_offset`, plus the already-public
`wo_val_decode_vm` and `wo_db_val_free` (`table.h:183-187`).
- Produces: `wo_wal_read_row_at`, which 5c's map consumes as its read path.
- [ ] Add `wo_wal_read_row_at` in `wal.c`, mirroring `wo_row_read`'s contract
(`table.c:721`) but resolving from a file offset instead of the id hash:
`scan_record` the record, parse the `kind | class_id | id` header, `dec_val`
each field into engine-owned slots, convert each to a fresh VM value with
`wo_val_decode_vm`, then free the engine slots. The out-gate rule is
unchanged — always a copy, never a pointer into anything.
- [ ] Return distinguishable outcomes: 0 ok, -1 no intact record at that
offset or a record whose kind carries no payload (a REMOVE tombstone), -2
OOM with `*msg` set. Silently treating a tombstone as a row would be the
worst failure available here.
- [ ] Free every engine slot on **every** path including the partial-decode
error path, matching what `dec_val`'s own callers already do at `wal.c:204`.
ASan is the check, not inspection.
- [ ] Do not change any storage behaviour. Nothing calls this yet; it is
additive, which is the whole point of separating it from 5c.
- [ ] Unit-test it against 5a's offsets: insert rows of mixed kinds
(scalar, Text, and a nil Text), record each offset, then read every row
back **by offset** and deep-compare field by field to what was inserted.
Include a read at a deliberately wrong offset and at a tombstone, asserting
the documented refusals rather than a crash.
- [ ] Verify: `make -C runtime test` and `test-iso`, both ASan+UBSan clean;
`just oop-e2e`, `just residency`, `just employee`, `just db-actor`
unchanged, since nothing calls the new function yet.
- [ ] Commit. Draft: `feat(db): wo_wal_read_row_at — materialise a row from a log offset`.
## Task 6 — the two runtime refusals

View file

@ -439,6 +439,97 @@ static void test_offset_after_failed_commit(void) {
wo_rt_destroy(&rt);
}
/* databasev2 2 (5b): read rows back BY OFFSET and deep-compare.
*
* The point is not that a record parses — test_offset_capture already showed
* the offsets are right. The point is that the VALUES come back intact,
* including a nil Text, and that the two refusal paths refuse instead of
* handing back something plausible. */
static void test_read_row_at(void) {
char path[128];
snprintf(path, sizeof path, "%s/readat.wal", g_dir);
wo_rt rt;
T_EQ(wo_rt_init(&rt, 1 << 20, CLASSES, 1), 0);
wo_db db;
T_EQ(wo_db_init(&db, CLASSES, 1, 0, 1), 0);
wo_wal w;
T_EQ(wo_wal_open(&w, path, 1 << 16), 0);
const char *msg = "";
enum { N = 24 };
uint64_t ids[N], offs[N];
const char *labels[N];
for (int i = 0; i < N; i++) {
/* every third row has a NIL Text, so the nil path is covered */
wo_str *s = NULL;
if (i % 3 != 0) {
char lbl[24];
int ln = snprintf(lbl, sizeof lbl, "row-%d", i);
s = wo_str_new(&rt, lbl, (uint32_t)ln);
T_CHECK(s != NULL);
}
uint64_t vals[2] = {(uint64_t)(i * 3 + 1), (uint64_t)(uintptr_t)s};
ids[i] = wo_row_insert(&db, 0, vals, &msg, NULL);
T_CHECK(ids[i] != 0);
labels[i] = (i % 3 != 0) ? "set" : "nil";
offs[i] = wo_wal_next_offset(&w);
T_EQ(wo_wal_append_insert(&w, &db, 0, ids[i]), 0);
if (s) wo_str_free(&rt, s);
}
T_EQ(wo_wal_commit(&w), 0);
/* read each row back by offset and compare field by field */
for (int i = 0; i < N; i++) {
uint64_t got[2] = {0, 0};
uint32_t cid = 0xFFFFFFFFu;
uint64_t id = 0;
T_EQ(wo_wal_read_row_at(&w, &db, &rt, offs[i], &cid, &id, got, &msg), 0);
T_EQ(cid, 0u);
T_EQ(id, ids[i]);
T_EQ(got[0], (uint64_t)(i * 3 + 1));
if (labels[i][0] == 'n') {
T_EQ(got[1], 0u); /* nil Text stays nil through the round trip */
} else {
wo_str *back = (wo_str *)(uintptr_t)got[1];
T_CHECK(back != NULL);
char want[24];
int wl = snprintf(want, sizeof want, "row-%d", i);
T_EQ((int)back->len, wl);
T_EQ(memcmp(back->data, want, (size_t)wl), 0);
wo_str_free(&rt, back); /* out-gate: the VM value is ours to free */
}
}
/* refusal 1: a tombstone is refused, not decoded as a live row */
T_EQ(wo_row_remove(&db, 0, ids[0]), 0);
uint64_t tomb_off = wo_wal_next_offset(&w);
T_EQ(wo_wal_append_remove(&w, 0, ids[0]), 0);
T_EQ(wo_wal_commit(&w), 0);
{
uint64_t got[2] = {0, 0};
T_EQ(wo_wal_read_row_at(&w, &db, &rt, tomb_off, NULL, NULL, got, &msg), -1);
}
/* refusal 2: a wrong offset (mid-record) refuses rather than returning a
* neighbouring row -- the silent-wrong-row failure this guards */
{
uint64_t got[2] = {0, 0};
T_EQ(wo_wal_read_row_at(&w, &db, &rt, offs[5] + 3u, NULL, NULL, got, &msg), -1);
}
/* refusal 3: past the end of the intact prefix */
{
uint64_t got[2] = {0, 0};
T_EQ(wo_wal_read_row_at(&w, &db, &rt, w.off + 4096u, NULL, NULL, got, &msg), -1);
}
wo_wal_close(&w);
wo_db_destroy(&db);
wo_rt_destroy(&rt);
}
int main(void) {
snprintf(g_dir, sizeof g_dir, "/tmp/wo-wal-test-XXXXXX");
if (!mkdtemp(g_dir)) return 1;
@ -447,6 +538,7 @@ int main(void) {
test_float_bytes_replay();
test_offset_capture();
test_offset_after_failed_commit();
test_read_row_at();
test_crash_battery();
/* leave the dir for a failed run's forensics only */
if (!t_fail) {