fix(db2-delta): pend_repoint failure fatal; delta fold no longer trusts a live WAL

- wo_wal_pend_repoint's failure was silently discarded (db.c); its
  own doc claimed replay reconciles a stale map — false, a second
  same-drain update chains past the lost one, permanently. Now
  fatal, like wo_wal_stage_fatal; comment corrected
- apply_delta dereferenced db->rt->wal unguarded — NULL rt + any
  DELTA record was a crash. Now refuses cleanly (-1)
- wo_wal_replay_ex lent its throwaway view only when rt->wal was
  unset, so a live wal's non-empty staging buffer could be folded
  against during replay. Now installs unconditionally whenever rt
  exists, saving/restoring whatever was there

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit fed9fe8b5fe022f0b22a4170c7fd68008700939f)
This commit is contained in:
shoney.arickathil 2026-08-30 11:58:09 +02:00
parent e37a10b70d
commit 061942c9a6
3 changed files with 42 additions and 7 deletions

View file

@ -315,8 +315,12 @@ void wo_db_exec_req(wo_vm *vm, wo_db_req *q) {
if (keys_res) { if (keys_res) {
/* recorded, not performed: this batch's barrier runs in the /* recorded, not performed: this batch's barrier runs in the
* drain (vm.c), and only then does the map move — mirrors * drain (vm.c), and only then does the map move — mirrors
* the insert arm's wo_wal_pend_drop exactly. */ * the insert arm's wo_wal_pend_drop in shape, but NOT in
(void)wo_wal_pend_repoint(w, q->cid, q->id, roff); * 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 { } else {
if (wo_wal_append_update(w, db, q->cid, q->id) != 0) wo_wal_stage_fatal(w); if (wo_wal_append_update(w, db, q->cid, q->id) != 0) wo_wal_stage_fatal(w);
} }

View file

@ -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); } 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) { void wo_wal_commit_fatal(wo_wal *w, uint32_t nrec) {
int staged = w->len != 0; int staged = w->len != 0;
int rc = wo_wal_commit(w); 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); uint64_t back_off = rd_u64(r);
const wo_classdesc *c = &db->classes[cid]; const wo_classdesc *c = &db->classes[cid];
if (r->bad || field_idx >= c->field_cnt) return -1; 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; uint64_t nv;
if (dec_val(r, db, c->kinds[field_idx], &nv) != 0 || (size_t)(r->end - r->p) != 0) if (dec_val(r, db, c->kinds[field_idx], &nv) != 0 || (size_t)(r->end - r->p) != 0)
return -1; 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 * 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 * 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; wo_wal view;
memset(&view, 0, sizeof view); memset(&view, 0, sizeof view);
view.fd = fd; view.fd = fd;
const void *saved_wal = NULL; const void *saved_wal = NULL;
int lent = 0; int lent = 0;
if (db->rt && !db->rt->wal) { if (db->rt) {
saved_wal = db->rt->wal; saved_wal = db->rt->wal;
db->rt->wal = &view; db->rt->wal = &view;
lent = 1; lent = 1;

View file

@ -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 /* 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 — * 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 * 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 * `repoint` field). 0 ok, -1 out of memory.
* was; a durable delta with a stale map is exactly what replay reconciles, *
* so this is safe, just deferred further than intended). */ * 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); 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 /* 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. */ * wo_wal_commit_fatal). Never returns. */
void wo_wal_stage_fatal(const wo_wal *w); 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. /* databasev2 4: commit, or END THE PROCESS.
* *
* The one rule this iteration introduces: once a statement has mutated RAM, * The one rule this iteration introduces: once a statement has mutated RAM,