Fix runWave swallowing parent cancellation (D39.17 errata): return parent.Err() when no worker errored, so a Ctrl-C resume surfaces context.Canceled instead of a nil-deref panic or false exit-0 partial success; regression-pinned
This commit is contained in:
parent
5be69939fc
commit
5fc4860d64
2 changed files with 34 additions and 1 deletions
|
|
@ -183,6 +183,15 @@ func (r *Runner) translateBookWaves(ctx context.Context, chunks []Chunk, stickyS
|
||||||
// race-clean. Results are written by the callback into a caller-owned slice indexed by i, so the result
|
// race-clean. Results are written by the callback into a caller-owned slice indexed by i, so the result
|
||||||
// ORDER is deterministic (item index) regardless of completion order — the golden capture sorts the wire/log
|
// ORDER is deterministic (item index) regardless of completion order — the golden capture sorts the wire/log
|
||||||
// side-effects separately. workers ≤ 0 is treated as 1 (LoadPipeline already clamps).
|
// side-effects separately. workers ≤ 0 is treated as 1 (LoadPipeline already clamps).
|
||||||
|
//
|
||||||
|
// PARENT CANCELLATION (Ctrl-C / SIGTERM — the CLI wires a signal.NotifyContext) is surfaced as an error too,
|
||||||
|
// NOT a silent nil: the feeder drops the remaining items on ctx.Done(), and a worker need not observe the
|
||||||
|
// cancellation to return (a $0 resume worker resolves entirely through its own opContext store reads, never
|
||||||
|
// touching the cancellable ctx), so firstErr can stay nil while items are left UNDONE. Returning nil then
|
||||||
|
// would let the caller index uninitialised result slots — a nil-deref on the edit-wave `*oc` assembly, or a
|
||||||
|
// draft-only run reporting empty translations as exit-0 success. So after the workers drain, a still-nil
|
||||||
|
// firstErr defers to parent.Err(): nil on a clean run, context.Canceled on an interrupted one (the durable
|
||||||
|
// store stays correct — the undone items were never checkpointed, so a re-run resumes them).
|
||||||
func (r *Runner) runWave(parent context.Context, workers, n int, work func(ctx context.Context, i int) error) error {
|
func (r *Runner) runWave(parent context.Context, workers, n int, work func(ctx context.Context, i int) error) error {
|
||||||
if workers < 1 {
|
if workers < 1 {
|
||||||
workers = 1
|
workers = 1
|
||||||
|
|
@ -224,7 +233,12 @@ func (r *Runner) runWave(parent context.Context, workers, n int, work func(ctx c
|
||||||
}
|
}
|
||||||
close(idxCh)
|
close(idxCh)
|
||||||
wg.Wait()
|
wg.Wait()
|
||||||
|
if firstErr != nil {
|
||||||
return firstErr
|
return firstErr
|
||||||
|
}
|
||||||
|
// No worker failed, but the PARENT may have been cancelled (Ctrl-C), which the feeder honoured by
|
||||||
|
// dropping items — surface that as an error so the driver never assembles a partial result as success.
|
||||||
|
return parent.Err()
|
||||||
}
|
}
|
||||||
|
|
||||||
// stageSeqResult is the outcome of running one chunk/unit through an ORDERED list of stages (one wave):
|
// stageSeqResult is the outcome of running one chunk/unit through an ORDERED list of stages (one wave):
|
||||||
|
|
|
||||||
|
|
@ -133,6 +133,25 @@ func TestWaveMultiChunkEditUnit(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestRunWaveSurfacesParentCancellation pins the cancellation contract: when the PARENT context is
|
||||||
|
// cancelled (a Ctrl-C / SIGTERM during a live run — main.go wires a signal.NotifyContext), runWave must
|
||||||
|
// surface that as an ERROR, never a silent nil. The feeder drops the remaining items on ctx.Done(), and a
|
||||||
|
// $0 resume worker never touches the cancellable ctx (its store ops use their own opContext), so firstErr
|
||||||
|
// stays nil — without a parent.Err() check runWave returns nil with items UNDONE, and the driver then
|
||||||
|
// indexes uninitialised result slots (an edit pipeline nil-derefs *oc; a draft-only run reports empty
|
||||||
|
// translations as exit-0 success). A cancelled parent → a non-nil error is the fix's whole point.
|
||||||
|
func TestRunWaveSurfacesParentCancellation(t *testing.T) {
|
||||||
|
r := &Runner{} // runWave touches no Runner field
|
||||||
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
cancel() // the operator hit Ctrl-C before/at the wave boundary
|
||||||
|
// Workers return nil (a $0 resume worker never observes the cancellable ctx) — so firstErr stays nil
|
||||||
|
// and ONLY a parent-cancellation check can turn this into the error the driver needs.
|
||||||
|
err := r.runWave(ctx, 2, 8, func(context.Context, int) error { return nil })
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("runWave returned nil on a cancelled parent ctx — items are left undone and the driver indexes uninitialised result slots (nil-deref panic on an edit pipeline / silent exit-0 partial success on draft-only)")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestWaveParallelWorkersMoneyConserved runs the wave driver with several parallel workers over a book of
|
// TestWaveParallelWorkersMoneyConserved runs the wave driver with several parallel workers over a book of
|
||||||
// many chunks and asserts the money invariant survives concurrency: committed spend == the sum of the
|
// many chunks and asserts the money invariant survives concurrency: committed spend == the sum of the
|
||||||
// settled checkpoints (the single-writer store serializes Reserve/Settle), and the per-unit result is
|
// settled checkpoints (the single-writer store serializes Reserve/Settle), and the per-unit result is
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue