fix(db2-keys): a logged delete must replay on a keys-resident table

- wo_row_remove's keys arm borrows the row from the log to find its
  index entries, and a borrow reads through db->rt->wal. At boot that
  pointer is not wired yet: main.c replays first (main.c:226) and
  assigns rt.wal afterwards (main.c:268)
- so the borrow found no log, the remove failed, and replay reported a
  valid tombstone as CORRUPTION. An UPDATE record would have failed the
  same way, since replay applies it as remove-then-recreate
- replay now lends the runtime a read-only view over the fd it already
  has open, for the replay's duration only, and restores what was there
- broken by the delete fix in 76b8fd9 — deletes worked in-process but
  their tombstones broke the next boot. Unreachable in production only
  because the loader still refuses the annotation
- pinned by test_keys_resident_delete_then_replay, verified failing
  against the unfixed code (2 failures) and clean with it
- found by asking whether the read-modify-append plan was ready, not by
  a gate

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit dc25462461b9f79d70c803f7174adc90fa16c90e)
This commit is contained in:
shoney.arickathil 2026-08-30 05:42:27 +02:00
parent 516bd8362d
commit 7b39da7eb6
3 changed files with 88 additions and 2 deletions

View file

@ -863,6 +863,33 @@ int64_t wo_wal_replay_ex(const char *path, wo_db *db, uint32_t *volatile_cid) {
if (fd < 0) return errno == ENOENT ? 0 : -1; /* no WAL yet = fresh boot */
uint64_t off = 0;
int64_t applied = 0;
/* databasev2 2: replay must be able to READ rows back, not only write them.
* A REMOVE (or an UPDATE, which replays as remove-then-recreate) on a
* keys-resident table reaches wo_row_remove, whose keys arm borrows the row
* out of the log to find its index entries — and a borrow reads through
* db->rt->wal. At boot that pointer is not wired yet: main.c replays first
* and assigns rt.wal afterwards, so the borrow found no log, the remove
* failed, and replay reported a perfectly good tombstone as CORRUPTION.
*
* 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. */
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) {
saved_wal = db->rt->wal;
db->rt->wal = &view;
lent = 1;
}
#define REPLAY_RETURN(v) \
do { \
if (lent) db->rt->wal = (void *)saved_wal; \
return (v); \
} while (0)
for (;;) {
uint32_t len;
uint8_t *payload;
@ -887,7 +914,7 @@ int64_t wo_wal_replay_ex(const char *path, wo_db *db, uint32_t *volatile_cid) {
if (rc != 0) {
close(fd);
if (rc == -2 && volatile_cid) *volatile_cid = rec_cid;
return rc == -2 ? -2 : -1;
REPLAY_RETURN(rc == -2 ? -2 : -1);
}
if ((rec_kind == WO_WAL_INSERT || rec_kind == WO_WAL_UPDATE) && rec_id &&
wo_table_is_keys_resident(db, rec_cid))
@ -896,7 +923,8 @@ int64_t wo_wal_replay_ex(const char *path, wo_db *db, uint32_t *volatile_cid) {
applied++;
}
close(fd);
return applied;
REPLAY_RETURN(applied);
#undef REPLAY_RETURN
}
int64_t wo_wal_replay(const char *path, wo_db *db) {

View file

@ -139,6 +139,18 @@ Outstanding:
**This is why the loader refusal earns its keep.** The gap was not one
missing operation but a second one that corrupted memory silently, found
only by auditing every reader of the id map.
- **Given** a logged `delete` on a `resident: keys` table, **when** the process
restarts, **then** the tombstone replays. ✅ **fixed 2026-08-30 — and it was
broken by the delete fix itself.** `wo_row_remove`'s keys arm borrows the row
out of the log to find its index entries, and a borrow reads through
`db->rt->wal`. At boot that pointer is not wired yet — `main.c` replays first
and assigns `rt.wal` afterwards — so the borrow found no log, the remove
failed, and replay reported a valid tombstone as CORRUPTION. Replay now lends
the runtime a read-only view over the fd it already has open. Pinned by
`test_keys_resident_delete_then_replay`, which fails against the unfixed code.
Found by asking whether the read-modify-append plan was ready, not by a gate —
it is unreachable today only because the loader refuses the annotation.
- **Given** an `update` to a row on a `resident: keys` table, **when** it runs,
**then** it is applied. ❌ **refused explicitly** by
`wo_row_update_field{,_slot}`. A keys row lives in the log with no slab slot

View file

@ -633,6 +633,51 @@ static void test_keys_resident_delete(void) {
wo_rt_destroy(&rt);
}
static void test_keys_resident_delete_then_replay(void) {
/* Does a keys-resident table survive a RESTART after a delete? The tombstone
* has to replay, and replay reaches wo_row_remove, whose keys arm borrows
* the row from the log to find its index entries. Replay runs BEFORE
* rt->wal is wired (main.c sets it after), so the borrow has no log to read
* and the remove fails — which replay reports as corruption. */
char path[128];
snprintf(path, sizeof path, "%s/keysdelreplay.wal", g_dir);
wo_rt rt;
T_EQ(wo_rt_init(&rt, 1 << 20, KEYS_CLASSES, 1), 0);
wo_db db;
T_EQ(wo_db_init(&db, KEYS_CLASSES, 1, 0, 1), 0);
wo_wal w;
T_EQ(wo_wal_open(&w, path, 1 << 16), 0);
db.rt = &rt; rt.wal = &w; rt.db = &db;
const char *msg = "";
uint64_t ids[2];
for (int i = 0; i < 2; i++) {
wo_str *sv = wo_str_new(&rt, "dr", 2);
uint64_t vals[2] = {(uint64_t)(i + 900), (uint64_t)(uintptr_t)sv};
ids[i] = wo_row_insert(&db, 0, vals, &msg, NULL);
uint64_t off = wo_wal_next_offset(&w);
T_EQ(wo_wal_append_insert(&w, &db, 0, ids[i]), 0);
T_EQ(wo_wal_commit(&w), 0);
T_EQ(wo_row_drop_payload(&db, 0, ids[i], off), 0);
}
/* delete one, logging the tombstone the way the request path does */
T_EQ(wo_row_remove(&db, 0, ids[0]), 0);
T_EQ(wo_wal_append_remove(&w, 0, ids[0]), 0);
T_EQ(wo_wal_commit(&w), 0);
wo_wal_close(&w);
wo_db_destroy(&db);
/* the restart: replay has no rt->wal yet, exactly as main.c orders it */
wo_db db2;
T_EQ(wo_db_init(&db2, KEYS_CLASSES, 1, 0, 1), 0);
db2.rt = &rt; rt.wal = NULL; rt.db = &db2;
int64_t n = wo_wal_replay(path, &db2);
T_CHECK(n >= 0); /* NOT corruption: a logged delete must replay */
T_CHECK(db2.tables[0].count == 1); /* one survivor */
wo_db_destroy(&db2);
wo_rt_destroy(&rt);
}
static void test_keys_resident_survives_compaction(void) {
/* databasev2 2 (5d): the obligation recorded at wo_wal_compact. Two ways
* to fail it, both checked here:
@ -1175,6 +1220,7 @@ int main(void) {
test_keys_resident_replay();
test_keys_resident_survives_compaction();
test_keys_resident_delete();
test_keys_resident_delete_then_replay();
test_stale_compact_temp_is_removed();
test_should_compact_policy();
test_compact_refuses_with_staged_records();