Aller au contenu
login
arrow_backRetour aux issues
Tnsor-Labs/brokoli #267

Prove cross-worker crash recovery for real, then wire paginated dispatch into it

ecoDébutant good first issue priority/now milestone-ready area/backend

descriptionDescription

## What this is about `ADR-015` and `ADR-017` already made the big call here: a pipeline run's work is split into durable, independently-leased, fenced physical instances, so a lost worker's in-flight work can be reclaimed and retried by a *different* worker without disturbing successful siblings. That's not aspirational — `models.PhysicalPlan`/`PhysicalInstance`/`ExecutionAttempt` are real, persisted, and load-bearing in a real multi-pod Kubernetes deployment (see `ADR-017`'s 2026-08-12 updates). Fencing correctness at the instance level — a stale, previously-fenced-out attempt writing after a newer claim has taken over — has real coverage: `TestExecuteInstanceJob_StaleAttemptDoesNotClobberWinningArtifact` and the bug `#240` found and fixed. What's still missing is named explicitly, in the same words, in both ADRs' own "still open" sections: **the literal crash-harness proof — kill a real worker process mid-instance and watch a different worker reclaim and finish it — has never actually been run.** Today's coverage (`engine/crash_harness_test.go`) is a unit-level simulation: it asserts recovery reconstructs the right state from a database left in a given shape, but nothing in the test suite has ever spawned a real `brokoli serve` worker process and sent it a real `SIGKILL` mid-instance. ## Where things stand today Worth being precise about what already exists, so this isn't confused with building the mechanism from scratch — the mechanism is built; only the end-to-end proof is missing: - **The fencing/lease model is real and tested.** `ExecutionAttempt` records a claim, heartbeat, and fencing generation; `engine/instance_worker_test.go`'s stale-attempt test proves a fenced-out attempt's write is rejected once a newer claim exists — but that test drives the race directly at the store/execution layer, not through an actual killed OS process. - **`engine/crash_harness_test.go` simulates crash points, it doesn't cause one.** Its own doc comment says the design is "assert recovery's classification is correct with no other live-process signal available" — it writes a database into the shape a crash at a given boundary would leave, then calls `RecoverNonTerminalRuns` directly. That's real, valuable coverage of the reconstruction logic, and it's exactly why the ADR is careful to call the *literal* process-kill proof a separate, still-open item rather than "basically done." - **The next consumer of this mechanism is already scoped and waiting.** `#143` describes wiring paginated `source_api` work into the same durable-instance dispatch path (each page becomes its own leasable, fenceable physical instance instead of an in-process retry loop). It's unassigned today and explicitly depends on the instance-dispatch mechanism this issue proves out actually working end to end — a good second step once M1 gives you a real mental model of how a lease is claimed, fenced, and reclaimed. ## What we're proposing Per `ADR-015`'s Follow-ups ("Prove lost-worker recovery with the crash harness already in the repository... as an acceptance gate for durable dispatch, not a follow-up") and `ADR-017`'s Deferred section (the same item, same wording): 1. **A real integration test that spawns an actual `brokoli serve` worker process, kills it (`SIGKILL`, not a graceful shutdown) while it holds a lease on an in-flight physical instance, and asserts a second worker process claims the fenced-out instance and completes it** — without disturbing any sibling instance that already finished successfully. 2. Once that's proven, **wire `#143`'s paginated work units into the same dispatch path**, so a lost worker mid-page doesn't cost the whole node's progress. 3. The bigger, not-yet-decided piece both `ADR-015` and `ADR-018` call out: **the durable scheduler has no fairness/tenant-quota/priority model at all** — job claiming today is plain creation order, so one org's burst of work can starve another's. Both ADRs explicitly say this "needs a dedicated ADR" tha
codeOuvre sur GitHub