diff --git a/database/src/db.c b/database/src/db.c index 6101fa5..1e66e10 100644 --- a/database/src/db.c +++ b/database/src/db.c @@ -315,8 +315,12 @@ void wo_db_exec_req(wo_vm *vm, wo_db_req *q) { if (keys_res) { /* recorded, not performed: this batch's barrier runs in the * drain (vm.c), and only then does the map move — mirrors - * the insert arm's wo_wal_pend_drop exactly. */ - (void)wo_wal_pend_repoint(w, q->cid, q->id, roff); + * the insert arm's wo_wal_pend_drop in shape, but NOT in + * failure safety: the delta is already staged and RAM has + * already moved, so a lost re-point is unrecoverable (see + * wo_wal_pend_repoint's own doc) and must die here, not + * limp on with a permanently stale map. */ + if (wo_wal_pend_repoint(w, q->cid, q->id, roff) != 0) wo_wal_repoint_fatal(w); } else { if (wo_wal_append_update(w, db, q->cid, q->id) != 0) wo_wal_stage_fatal(w); } diff --git a/database/src/wal.c b/database/src/wal.c index 1c41fd6..c59751f 100644 --- a/database/src/wal.c +++ b/database/src/wal.c @@ -556,6 +556,13 @@ static void wal_die(const wo_wal *w, const char *op, uint32_t nrec) { void wo_wal_stage_fatal(const wo_wal *w) { wal_die(w, "staging a record", 1); } +/* IMPORTANT 1 (review finding): unlike wo_wal_pend_drop's failure, a lost + * pend_repoint is not safe to shrug off — the delta it was meant to + * re-point to is already staged, RAM has already moved (table.c's own + * fatal-below-this-line rule), and a second same-drain update to the same + * row would otherwise chain its delta past the lost one, permanently. */ +void wo_wal_repoint_fatal(const wo_wal *w) { wal_die(w, "recording a pending re-point", 1); } + void wo_wal_commit_fatal(wo_wal *w, uint32_t nrec) { int staged = w->len != 0; int rc = wo_wal_commit(w); @@ -897,6 +904,10 @@ static int apply_delta(wo_db *db, uint32_t cid, uint64_t id, rbuf *r) { uint64_t back_off = rd_u64(r); const wo_classdesc *c = &db->classes[cid]; if (r->bad || field_idx >= c->field_cnt) return -1; + /* IMPORTANT 2 (review finding): a DELTA can only be folded through + * db->rt->wal (wo_wal_replay_ex's lent view, at replay, or a live one + * otherwise) — refuse cleanly instead of dereferencing a NULL rt. */ + if (!db->rt) return -1; uint64_t nv; if (dec_val(r, db, c->kinds[field_idx], &nv) != 0 || (size_t)(r->end - r->p) != 0) return -1; @@ -1254,13 +1265,21 @@ int64_t wo_wal_replay_ex(const char *path, wo_db *db, uint32_t *volatile_cid) { * * Lend the runtime a read-only view over the fd already open here, for the * duration of the replay only, and restore whatever was there. Nothing in - * this window appends, so a view carrying just the descriptor is enough. */ + * this window appends, so a view carrying just the descriptor is enough. + * + * IMPORTANT 2 (review finding): this used to lend the view only when + * db->rt->wal was NOT already set — leaving a rt whose wal WAS already + * live to fold deltas against that LIVE wal's (possibly non-empty) + * staging buffer instead, which the design never allows. Install the + * throwaway view unconditionally whenever there is an rt to hold it; + * apply_delta refuses outright when there is no rt at all to install + * one into. */ wo_wal view; memset(&view, 0, sizeof view); view.fd = fd; const void *saved_wal = NULL; int lent = 0; - if (db->rt && !db->rt->wal) { + if (db->rt) { saved_wal = db->rt->wal; db->rt->wal = &view; lent = 1; diff --git a/database/src/wal.h b/database/src/wal.h index 840b136..ea6e850 100644 --- a/database/src/wal.h +++ b/database/src/wal.h @@ -175,9 +175,16 @@ int wo_wal_pend_drop(wo_wal *w, uint32_t cid, uint64_t id, uint64_t off); /* Task 4 (keys-resident delta updates): note a keys-resident row's id-map * entry that must move to [off] once the delta staged there is durable — * the update-arm counterpart of wo_wal_pend_drop, on its own list (see the - * `repoint` field). 0 ok, -1 out of memory (the map simply stays where it - * was; a durable delta with a stale map is exactly what replay reconciles, - * so this is safe, just deferred further than intended). */ + * `repoint` field). 0 ok, -1 out of memory. + * + * IMPORTANT 1 (review finding): unlike wo_wal_pend_drop, a failure here is + * NOT safe to ignore. Replay does NOT reconcile a lost re-point: if a + * second update to this row lands in the same drain, it finds no pending + * entry, falls back to the stale durable offset, and its delta chains PAST + * the one this call was meant to record — every reader, replay and + * compaction included, then agrees on the wrong value, permanently. + * Callers must treat a nonzero return as fatal (wo_wal_repoint_fatal), + * exactly like a failed wo_wal_stage_fatal. */ int wo_wal_pend_repoint(wo_wal *w, uint32_t cid, uint64_t id, uint64_t off); /* Task 4: the most recent PENDING re-point recorded for (cid, id), not yet @@ -248,6 +255,11 @@ int wo_wal_compact(wo_wal *w, wo_db *db); * wo_wal_commit_fatal). Never returns. */ void wo_wal_stage_fatal(const wo_wal *w); +/* IMPORTANT 1 (review finding): a pending re-point could not even be + * RECORDED — same unrecoverable position as wo_wal_stage_fatal, see + * wo_wal_pend_repoint's own doc. Never returns. */ +void wo_wal_repoint_fatal(const wo_wal *w); + /* databasev2 4: commit, or END THE PROCESS. * * The one rule this iteration introduces: once a statement has mutated RAM,