From 8b0b715213fb76fa9f8d169b32916634cb9cac42 Mon Sep 17 00:00:00 2001 From: "Claude (zset-rollback)" Date: Sat, 10 Oct 2026 18:28:18 -0700 Subject: [PATCH] fix(zset): mark a rebuild chunk's push as a step so a panicking push is counted (improvise-bw4p.5) Also renames the MemTrace clone-panic test to what it asserts. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- context/code/zset.md | 5 +++-- context/test-evidence/zset-ffi.md | 6 ++++++ context/test-evidence/zset.md | 9 ++++++++- crates/zset-ffi/tests/rollback_abi.rs | 24 ++++++++++++++++++++++++ crates/zset/src/circuit.rs | 1 + crates/zset/tests/clone_panic.rs | 2 +- crates/zset/tests/rollback_rebuild.rs | 23 +++++++++++++++++++++++ 7 files changed, 66 insertions(+), 4 deletions(-) diff --git a/context/code/zset.md b/context/code/zset.md index f1947d4b..ae025393 100644 --- a/context/code/zset.md +++ b/context/code/zset.md @@ -227,7 +227,7 @@ clause's wording stands; this is how the crate reads it). sweeps a panic over every key clone in a step). A step that dies half way has therefore logged exactly the keys it applied. A custom `Trace` must do the same: a panicking `apply` leaves the trace as it was. `MemTrace` does, since it makes every clone before it touches - its map (`c2_memtrace_value_clone_panic_leaves_an_empty_group`). + its map (`a_panicking_value_clone_leaves_a_memtrace_unchanged`). 5. **The edge's two copies.** An integrate stream's piece is also its output, so a second copy (the whole-tick value) only exists while a tick is split. `Slot::whole` is `None` otherwise and `current()` reads the piece. `piece()` lends a borrow rather than a clone @@ -241,7 +241,8 @@ clause's wording stands; this is how the crate reads it). ahead of the circuit. The clock cannot tell: `rollback` rewinds `ticks`, which can make a dead job's recorded tick current again. So the circuit also counts the rollbacks that undid a step or piece (`undone`, never rewound), and a job records the count when it first - runs, before its first chunk can panic. A later count makes `interrupted` true for good, + runs, before its first chunk can panic. The chunk's `push` is marked as a step too, since it + can overflow the pending delta before any step runs (`p1_a_chunk_whose_push_panics_in_a_txn_is_counted`). A later count makes `interrupted` true for good, finished or not (`rollback_rebuild.rs`: begun before the transaction and continued inside, finished inside, a first chunk that panicked, and a dead job revived by a later step). - An `Output::take` made inside the transaction: a rollback subtracts the step's output diff --git a/context/test-evidence/zset-ffi.md b/context/test-evidence/zset-ffi.md index 34b5689d..b7c34f1c 100644 --- a/context/test-evidence/zset-ffi.md +++ b/context/test-evidence/zset-ffi.md @@ -215,3 +215,9 @@ A job whose first chunk failed inside the transaction resumed without that chunk ### f3_begin_on_a_circuit_poisoned_outside_a_txn_is_poisoned `begin` is `ZS_ERR_POISONED`; `commit` and `rollback` are `ZS_ERR_TRANSACTION`. + +### f5_a_chunk_whose_push_overflows_in_a_txn_then_rollback_is_interrupted + +The same path through the ABI: `zs_rebuild_step` returns `ZS_ERR_OVERFLOW` from the chunk's +push, `zs_circuit_rollback` succeeds, and the next `zs_rebuild_step` was a silent success; it is +now `ZS_ERR_INTERRUPTED`. Rival: the mark before the push removed fails `[f5]`. diff --git a/context/test-evidence/zset.md b/context/test-evidence/zset.md index 91782077..1605fa25 100644 --- a/context/test-evidence/zset.md +++ b/context/test-evidence/zset.md @@ -566,7 +566,7 @@ key is only visible when it is touched again (a rival that mutated `DistinctStat cloning its entry survived the first version). Rivals, each failing it: `add_logged`, `DistinctState::step` and `Undoable::apply` each mutating before cloning the entry. -### c2_memtrace_value_clone_panic_leaves_an_empty_group +### a_panicking_value_clone_leaves_a_memtrace_unchanged `MemTrace::apply` inserted an empty group before cloning the value. Rival: that order restored fails `[c2]`. @@ -589,3 +589,10 @@ split the tick inside the transaction and never reached the save). Pins the limit `zset.md` states: a first continued piece in a transaction clones the edge's whole value, growing with the state. + +### p1_a_chunk_whose_push_panics_in_a_txn_is_counted + +Delta review of #355 at b2445fed. `Rebuild::step` takes the chunk from its iterator and pushes +it before stepping; a push that overflows the pending delta panicked with no step run, so +`undone` was not bumped and the resumed job silently dropped the chunk (key 1's `+1`). The step +is now marked before the push. Rival: that mark removed fails `[p1]`. diff --git a/crates/zset-ffi/tests/rollback_abi.rs b/crates/zset-ffi/tests/rollback_abi.rs index 67cf106a..9c0e0f18 100644 --- a/crates/zset-ffi/tests/rollback_abi.rs +++ b/crates/zset-ffi/tests/rollback_abi.rs @@ -167,3 +167,27 @@ fn f4_a_failed_rebuild_step_in_a_txn_then_rollback_then_resume() { "[f4] header: a rebuild stepped in a rolled-back txn reports ZS_ERR_INTERRUPTED; got rc={rc} done={done}" ); } + +#[test] +fn f5_a_chunk_whose_push_overflows_in_a_txn_then_rollback_is_interrupted() { + let (c, i, _s) = rig(); + let pending = zset(&[(b"a", i64::MAX)]); + assert_eq!(unsafe { zs_circuit_push(c, i, pending) }, OK); + let snap = zset(&[(b"a", 1), (b"b", 1)]); + let mut job = null_mut(); + assert_eq!(unsafe { zs_rebuild_new(c, i, snap, 1, &mut job) }, OK); + assert_eq!(unsafe { zs_circuit_begin(c) }, OK); + let mut done = 0; + assert_eq!( + unsafe { zs_rebuild_step(job, c, None, null_mut(), &mut done) }, + Code::Overflow as i32, + "the chunk's push overflows the pending delta" + ); + assert_eq!(unsafe { zs_circuit_rollback(c) }, OK); + let rc = unsafe { zs_rebuild_step(job, c, None, null_mut(), &mut done) }; + unsafe { zs_rebuild_free(job) }; + unsafe { zs_zset_free(snap) }; + unsafe { zs_zset_free(pending) }; + unsafe { zs_circuit_free(c) }; + assert_eq!(rc, Code::Interrupted as i32, "[f5] got rc={rc} done={done}"); +} diff --git a/crates/zset/src/circuit.rs b/crates/zset/src/circuit.rs index 05f50b6c..8aee5bbe 100644 --- a/crates/zset/src/circuit.rs +++ b/crates/zset/src/circuit.rs @@ -789,6 +789,7 @@ impl Rebuild { if chunk.is_zero() && self.began_at.is_some() { return Progress::Done; } + circuit.stepped |= circuit.open.is_some(); self.input.push(chunk); if self.began_at.is_some() { circuit.step_continue(); diff --git a/crates/zset/tests/clone_panic.rs b/crates/zset/tests/clone_panic.rs index 54de3c23..fae17b24 100644 --- a/crates/zset/tests/clone_panic.rs +++ b/crates/zset/tests/clone_panic.rs @@ -172,7 +172,7 @@ fn c1_a_clone_panic_anywhere_in_a_step_is_rolled_back() { } #[test] -fn c2_memtrace_value_clone_panic_leaves_an_empty_group() { +fn a_panicking_value_clone_leaves_a_memtrace_unchanged() { quiet(); let mut t: MemTrace = MemTrace::new(); CLONE_TRIP.with(|c| c.set(Some(0))); diff --git a/crates/zset/tests/rollback_rebuild.rs b/crates/zset/tests/rollback_rebuild.rs index dac1d892..88326bc6 100644 --- a/crates/zset/tests/rollback_rebuild.rs +++ b/crates/zset/tests/rollback_rebuild.rs @@ -142,3 +142,26 @@ fn b1_a_closure_that_reads_a_stream_during_eval_does_not_panic() { c.rollback(); c.step(); } + +#[test] +fn p1_a_chunk_whose_push_panics_in_a_txn_is_counted() { + let mut c = Circuit::new(); + let (input, s) = c.input::(); + let i = c.integrate(&s); + input.push(z(&[(1, i64::MAX)])); + let mut job = Rebuild::new(input.clone(), z(&[(1, 1), (2, 1)]), 1); + c.begin(); + let r = catch_unwind(AssertUnwindSafe(|| job.step(&mut c, || false))); + assert!(r.is_err(), "the push overflows"); + c.rollback(); + let interrupted = job.interrupted(&c); + let mut got = None; + if !interrupted { + while job.step(&mut c, || false) == Progress::Pending {} + got = Some(i.current()); + } + assert!( + interrupted, + "[p1] the chunk left the job unstepped; finishing it gives {got:?}" + ); +}