Repository navigation
fix(speculation): exhaust cancelled viable paths - #1036
sbalabanov wants to merge 3 commits into
Conversation
…eues
Summary:
Provider-cancelled executions cannot be rebuilt. Treat Failed and Cancelled viable paths as exhausted only after dependencies settle, without rewriting cancellation facts or changing user cancellation and successful alternatives.
Add an explicitly queue-scoped, dry-run-default recovery command using normal speculation messages and a stable operator recovery ID. No schema, provider contract, automatic retry, or planner authority changes.
Test Plan:
Repro (performed against isolated MySQL and ordinary orchestrator consumers):
1. Seed queue quiet-cancelled: request 1 processing v1; batch 2 speculating v1 containing it, no dependencies; SHA-256("2") path cancelled at attempt 1; no queued messages.
2. Run ./tool/bazel run //tool/submitqueue/recovercancelledpaths -- --queue quiet-cancelled --recovery-id demo-run --dry-run with MYSQL_DSN configured. Observed affected-head=2 and zero writes. Configure QUEUE_MYSQL_DSN, replace --dry-run with --apply, and repeat: exactly one speculate nudge with ID 2/cancelled-path-recovery/demo-run.
3. Start the ordinary orchestrator on a free local port with fake extension configuration and MQ_TENANTS=quiet-cancelled. Observed batch failed v2, request error v2, path still cancelled/attempt 1, and terminal log "every viable build failed or was cancelled".
TestRun_ProviderCancellationExhaustsHead proves persistence ordering and CAS/publish fault windows. Focused speculate, sticky, buildsignal and recovery tests passed. Deploy the policy and drain old consumers before applying recovery.
Revert Plan:
Revert this commit and stop applying recovery. Do not reverse terminal outcomes or rewrite Cancelled facts; older consumers may strand heads again.
|
[independent-review] Reviewed
Local mechanical checks/builds and affected test targets passed; all 12 assigned files were reviewed. Draft code-CI skips are not passing code validation. Generated with the repository's |
A one-shot recovery command must exit when interrupted even while its queue backend blocks in context-free MySQL initialization. Keep default SIGINT and SIGTERM handling; an interrupted invocation remains safe to retry using its existing recovery ID. Keep one straight-line provider-cancellation regression covering observed Cancelled/attempt 1, persistence order, and conclusion. The shared controller fault-window tests already cover failed writes and publications. Validation: focused speculate/recovery Bazel tests and recovery build pass; fmt, lint, tidy and Gazelle checks pass. With a TCP peer accepting but never sending a MySQL handshake, SIGINT and SIGTERM each terminate the command in 0.001 seconds. Isolated MySQL recovery replay still produces one stable nudge and converges through ordinary consumers without rewriting Cancelled facts.
|
[agent-response SQC-013] Addressed both findings from #1036 (comment) in 555ed2e. Removed signal interception from the one-shot CLI. With a TCP peer accepting the queue connection but sending no MySQL handshake, SIGINT exited with -2 in 0.001s and SIGTERM with -15 in 0.001s. Stable recovery-ID behavior is unchanged; the isolated MySQL replay still deduplicated two applies into one nudge and converged through ordinary consumers. Reduced the provider-cancellation regression to one straight-line proof of Cancelled/attempt 1, persistence order, and conclusion; shared fault-window tests retain CAS/publish coverage. Both affected Bazel test targets passed. The Test Plan sentence was corrected accordingly. |
Remove the historical recovery command, its tests and Bazel targets, the command-only queue-contract visibility grant, and its operational guide. Historical cleanup is not required for this change. Keep the runtime exhaustion rule, accurate diagnostics and behavioral tests unchanged: cancelled attempts stay Cancelled and are not retried, while only settled dependencies and exhausted viable paths allow a failed batch outcome.
TL;DR: Requests no longer stay stuck when their last viable build is cancelled.
Summary
This fixes “nothing left to build, but the request never finishes”: a cancelled last viable build was neither retried nor counted as exhausted. A batch now finishes as failed only after its dependencies settle and every viable path failed or was cancelled; other viable paths can continue, and valid passed results can proceed when their prerequisites allow. Cancelled builds are not retried, and their recorded outcome remains Cancelled.
Repro
Test Plan
TestRun_ProviderCancellationExhaustsHeadchecks that the last cancelled path stays Cancelled while the batch is marked failed before its requests are concluded.TestHasNoViableFutureandTestDecidecheck that unfinished dependencies, other viable work, and valid passed results are not treated as exhausted.Re-ran the focused speculation, sticky-allocator, and build-signal tests for this runtime-only update; all three targets passed.
Revert Plan
Revert this PR. Do not reverse terminal outcomes or rewrite Cancelled facts; older consumers may strand heads again.
Issues