Skip to content

fix(speculation): exhaust cancelled viable paths and recover quiet queues - #1036

Draft
sbalabanov wants to merge 2 commits into
mainfrom
wt/sqc013
Draft

sbalabanov wants to merge 2 commits into
mainfrom
wt/sqc013

Conversation

@sbalabanov

@sbalabanov sbalabanov commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

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 cancellation persistence and conclude ordering; existing shared tests cover 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.

Issues

…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.
@sbalabanov

Copy link
Copy Markdown
Contributor Author

[independent-review]

Reviewed 71a96b195a206c41863d676c11d5f65331f79ae2 independently. Two changes would make this smaller and safer:

  1. Fix termination during queue initialization — tool/submitqueue/recovercancelledpaths/main.go:89,119 intercepts SIGINT/SIGTERM, but NewQueue blocks in context-free DB.Ping(). A TCP peer that accepts without completing the MySQL handshake leaves the command running after both signals; this was independently reproduced. The simplest fix is to retain default signal termination for this one-shot command rather than add a broader initialization abstraction. Stable recovery IDs already make interrupted retries safe. Confidence: 95%. Verify with the stalled-handshake scenario and the existing tool test target.
  2. Reduce test-only branching — speculate/run_test.go:799 repeats shared path-set/batch CAS fault coverage. Keep one straight-line provider-cancellation regression that asserts Cancelled/attempt 1, persistence order, and conclusion; existing shared CAS tests already cover the failure mechanics. No additional tests are needed. Confidence: 90%.

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 .agents/skills/autoreview workflow; findings rechecked at the SHA above.

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.
@sbalabanov

Copy link
Copy Markdown
Contributor Author

[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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants