fix(storage): converge canonical drain for perpetually snoozing jobs (#456) - #458
fix(storage): converge canonical drain for perpetually snoozing jobs (#456)#458hardbyte wants to merge 3 commits into
Conversation
…456) During a storage transition, the re-scheduling completions of canonical-claimed jobs (snooze, retry backoff, retry-after) wrote the job back into canonical `scheduled_jobs`. A workload whose handlers snooze on every run therefore replenished `canonical_live_backlog` forever, and `awa storage finalize --wait` could never pass. `awa_model::reschedule` now branches on `awa.active_queue_storage_schema()`: canonical routing performs the same hot -> scheduled move as before, while `mixed_transition` / `active` takes the canonical row and re-inserts the attempt into the active queue-storage schema's `deferred_jobs`. Every canonical job leaves the canonical plane at its next completion, so the existing drain/finalize flow converges without operator intervention. Implemented as runtime SQL against the existing v040 schema rather than a migration: the ADR-037 gate refuses `awa migrate` on exactly the unfinalized clusters this fix serves, so a migration would be unreachable by them, and this keeps the fix backportable to the 0.6 line. Stale-completion protection is unchanged and strictly stronger: the take of the canonical row carries the same `state = 'running' AND run_lease = $n` guard as the historical UPDATE, and the successor takes a fresh job id from the queue-storage sequence, so a late completion from a rescued attempt matches no canonical row and reports stale. Maintenance leaders also promote the other plane's deferred backlog for the duration of the transition: a queue-storage leader promotes due canonical `scheduled_jobs`, and a canonical-resolved (pre-flip) leader promotes queue-storage `deferred_jobs` through its configured-but-inactive runtime. Drain and post-flip execution no longer depend on which runtime happens to hold leadership. Both paths are guarded no-ops on finalized and fresh installs. Tests: the rolling-transition rehearsal gains a snooze-forever workload on its own queue (the existing `snooze_once` handler completes on re-run, so it could not surface this), `canonical_backlog_count` now counts `scheduled_jobs` as its doc comment already claimed, and a focused test covers canonical routing, migrated routing, unique-claim hand-off, payload preservation, and both stale paths.
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughCanonical job rescheduling now uses shared model logic that routes successors to queue-storage ChangesCanonical drain convergence
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Worker
participant Executor
participant Reschedule
participant Canonical
participant QueueStorage
Worker->>Executor: complete canonical job
Executor->>Reschedule: reschedule with run lease
Reschedule->>Canonical: remove guarded running row
Reschedule->>QueueStorage: insert successor into deferred_jobs
QueueStorage-->>Worker: promote successor after transition
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
the
full-ci
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1334ad6f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let active_schema: Option<String> = | ||
| sqlx::query_scalar("SELECT awa.active_queue_storage_schema()") | ||
| .fetch_one(&mut *conn) | ||
| .await | ||
| .map_err(map_sqlx_error)?; |
There was a problem hiding this comment.
When storage_abort() runs concurrently with a canonical completion during mixed_transition, this unlocked lookup can observe queue storage after the abort has locked the transition row and counted the queue-storage tables as empty. The completion can then delete the canonical row and commit a new deferred row after the abort restores canonical routing, permanently stranding that job in the inactive queue-storage schema. Read and lock the transition-state row for the duration of the reschedule transaction so abort cannot validate and flip routing between this decision and the insert.
Useful? React with 👍 / 👎.
| let mut row = job.clone(); | ||
| row.id = job_id; | ||
| row.state = state; | ||
| row.run_at = run_at; |
There was a problem hiding this comment.
For a retryable handler error when lifecycle events are enabled, this helper clones the pre-completion running row and updates only routing fields, while the database reschedule also appends the new error and sets finalized_at. Consequently JobEvent::Retried.job no longer represents the persisted post-retry row—hooks inspecting job.errors miss the failure that triggered this event and may see stale finalization data. Pass the error/finalization changes into the synthesized row (or materialize the actual deferred representation) before emitting it.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
awa-worker/src/maintenance.rs (1)
2177-2201: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCross-plane promotion runs unconditionally on every promote tick. Both new blocks add steady-state DB work to every leader at the promote cadence (default 250ms, twice per tick) with no transition-state gate, so clusters that finalized long ago keep paying for mixed-transition machinery.
awa-worker/src/maintenance.rs#L2177-L2201: gate the canonical mirror-drain on the transition state (or a cached "canonical plane drained" flag) instead of runningpromote_due_batchunconditionally.awa-worker/src/maintenance.rs#L2208-L2215: resolve/cacheawa.active_queue_storage_schema()once per pass (or with a coarse refresh) rather than per state per tick.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@awa-worker/src/maintenance.rs` around lines 2177 - 2201, The promote tick performs unnecessary steady-state database work. In the canonical promotion block around promote_due_batch, gate mirror draining on the active storage-transition state or a cached canonical-plane-drained flag; also update the per-state logic at awa-worker/src/maintenance.rs lines 2208-2215 to resolve or cache awa.active_queue_storage_schema() once per pass or via a coarse refresh instead of once per state per tick.awa-model/tests/sql_only_storage_upgrade_test.rs (3)
447-456: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
UNION ALL … ORDER BY 1 DESC LIMIT 1reads as a max-of-two-counts trick.It works (max is 0 only when both are 0), but a single summed scalar is clearer and asserts the same thing.
♻️ Clearer query
- let canonical_left: i64 = sqlx::query_scalar( - "SELECT count(*)::bigint FROM awa.jobs_hot WHERE id = $1 \ - UNION ALL SELECT count(*)::bigint FROM awa.scheduled_jobs WHERE id = $1 \ - ORDER BY 1 DESC LIMIT 1", - ) + let canonical_left: i64 = sqlx::query_scalar( + "SELECT (SELECT count(*)::bigint FROM awa.jobs_hot WHERE id = $1) \ + + (SELECT count(*)::bigint FROM awa.scheduled_jobs WHERE id = $1)", + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@awa-model/tests/sql_only_storage_upgrade_test.rs` around lines 447 - 456, Update the canonical_left query in the SQL-only storage upgrade test to use one scalar that sums the counts from awa.jobs_hot and awa.scheduled_jobs for the bound job2, instead of UNION ALL with ORDER BY and LIMIT. Preserve the existing fetch and assert_eq! behavior.
401-408: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
count(*) OVER ()assertion is tautological.
WHERE id = $1on a primary key can never yield more than one row, socount(*) OVER ()is always1whenfetch_onesucceeds. If the intent is to assert the canonical plane holds exactly onereschedule_testrow, count without the id filter; otherwise just selectstate.♻️ Simplify
- let (state, count): (String, i64) = sqlx::query_as( - "SELECT state::text, count(*) OVER () FROM awa.scheduled_jobs WHERE id = $1", - ) - .bind(job1) - .fetch_one(&pool) - .await - .expect("scheduled row after canonical snooze"); - assert_eq!((state.as_str(), count), ("scheduled", 1)); + let state: String = + sqlx::query_scalar("SELECT state::text FROM awa.scheduled_jobs WHERE id = $1") + .bind(job1) + .fetch_one(&pool) + .await + .expect("scheduled row after canonical snooze"); + assert_eq!(state, "scheduled");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@awa-model/tests/sql_only_storage_upgrade_test.rs` around lines 401 - 408, Update the query and assertion around the canonical snooze check to avoid counting rows filtered by the primary-key id. Count the canonical reschedule_test rows without the id predicate if verifying exactly one row, or select only state if row-count validation is not intended; keep the existing scheduled-state assertion.
528-544: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCleanup/finalize only runs on the success path.
Every assertion above uses
assert!/panic!, so any failure leaves the shared test database inmixed_transitionwith leftover deferred rows and claims — which, per your own comment, makes a latermigrations::runhit the ADR-037 unfinalized-cluster gate and turns one failure into cascading failures in unrelated tests. Consider running the body viaAssertUnwindSafe(...).catch_unwind()(or a drop guard that finalizes) and re-raising after cleanup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@awa-model/tests/sql_only_storage_upgrade_test.rs` around lines 528 - 544, The cleanup and storage finalization after the reschedule test must also run when assertions panic. Wrap the test body containing the assertions in an unwind-catching mechanism such as AssertUnwindSafe(...).catch_unwind(), perform the existing deferred-job, claims, canonical-work, and storage_finalize cleanup afterward, then re-raise the original panic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@awa-model/src/reschedule.rs`:
- Around line 279-288: Update promote_due around the payload construction and
migrated DELETE ... RETURNING flow to preserve canonical lease callback state,
including callback_id, callback_timeout_at, callback_filter,
callback_on_complete, callback_on_fail, and callback_transform, on a compatible
successor claim; if that cannot be migrated, explicitly skip jobs with callback
wiring instead of promoting them as plain queued jobs.
- Around line 336-340: Update the rescheduling query around the run_at-producing
SELECT and its finalized_at bind so the Retryable timestamp is generated by
PostgreSQL using the same DB clock, rather than Utc::now() in the worker. Fold
the finalized_at value into that SELECT and bind the returned value, preserving
None for non-Retryable states.
In `@awa-worker/src/executor.rs`:
- Around line 1501-1521: Update rescheduled_event_row to accept the newly
recorded error entry and the appropriate finalized_at value, then assign both to
the cloned JobRow so the Retried event matches the previously re-read row.
Ensure the synthesized row includes the appended errors entry and clears or
updates finalized_at for the rescheduled state.
In `@awa/tests/rolling_transition_rehearsal_test.rs`:
- Around line 639-650: Give the queue_storage_target client in the
SnoozeForeverWorker registration its own Arc<AtomicI64> counter instead of
reusing snooze_performs, mirroring the separate rust_handled and
qs_target_handled counters. Update the related phase-7 assertion to track this
queue-storage-specific counter so post-finalize progress is attributed to queue
storage.
---
Nitpick comments:
In `@awa-model/tests/sql_only_storage_upgrade_test.rs`:
- Around line 447-456: Update the canonical_left query in the SQL-only storage
upgrade test to use one scalar that sums the counts from awa.jobs_hot and
awa.scheduled_jobs for the bound job2, instead of UNION ALL with ORDER BY and
LIMIT. Preserve the existing fetch and assert_eq! behavior.
- Around line 401-408: Update the query and assertion around the canonical
snooze check to avoid counting rows filtered by the primary-key id. Count the
canonical reschedule_test rows without the id predicate if verifying exactly one
row, or select only state if row-count validation is not intended; keep the
existing scheduled-state assertion.
- Around line 528-544: The cleanup and storage finalization after the reschedule
test must also run when assertions panic. Wrap the test body containing the
assertions in an unwind-catching mechanism such as
AssertUnwindSafe(...).catch_unwind(), perform the existing deferred-job, claims,
canonical-work, and storage_finalize cleanup afterward, then re-raise the
original panic.
In `@awa-worker/src/maintenance.rs`:
- Around line 2177-2201: The promote tick performs unnecessary steady-state
database work. In the canonical promotion block around promote_due_batch, gate
mirror draining on the active storage-transition state or a cached
canonical-plane-drained flag; also update the per-state logic at
awa-worker/src/maintenance.rs lines 2208-2215 to resolve or cache
awa.active_queue_storage_schema() once per pass or via a coarse refresh instead
of once per state per tick.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f283bba9-e73e-4bc2-8526-9b488b77fe3a
📒 Files selected for processing (9)
CHANGELOG.mdawa-model/src/lib.rsawa-model/src/reschedule.rsawa-model/tests/sql_only_storage_upgrade_test.rsawa-worker/src/client.rsawa-worker/src/executor.rsawa-worker/src/maintenance.rsawa/tests/migration_test.rsawa/tests/rolling_transition_rehearsal_test.rs
…ment it (#456) Addresses both review findings on the cross-plane re-schedule, extends the corner-case coverage, and brings the TLA+ model and operator docs in line. Lock the transition singleton FOR SHARE for the duration of the reschedule transaction before deciding where the attempt goes. `storage_abort`, `storage_enter_mixed_transition`, and `storage_finalize` all take FOR UPDATE on that row, so the share lock serializes against them. Without it an abort could validate the queue-storage tables as empty and restore canonical routing between the routing decision and the insert, stranding the job in a schema that is no longer active — lost work, which the delivery contract puts above every fast path. A missing singleton still resolves to canonical rather than erroring. Carry the persisted mutations into the synthesized event row. The re-schedule appends the triggering error and stamps `finalized_at`, so a row that only tracked the routing fields made `JobEvent::Retried.job` disagree with the stored row: hooks reading `job.errors` missed the failure that caused the retry. `lifecycle_hook_test` now asserts both. Corner cases added, several of which found real gaps in the first cut: - `state = prepared` must still route canonical. Migrating before the routing flip would put rows in a schema that is not yet authoritative and would break the `storage abort` interlock. - A `unique_states` mask that does not claim the destination state leaves the successor unclaimed; a newer duplicate already holding the key keeps it and the successor proceeds unclaimed without losing the job. - `RetryAfter` honours the caller's delay instead of computing backoff, and does not consume an attempt. - A missing transition singleton routes canonical. - The canonical and queue-storage id sequences are independent and can hand out the same number, so the successor-id assertions now advance `job_id_seq` first instead of passing on a coincidence. TLA+ (`AwaStorageTransition`): add `MigrateCanonicalRescheduleToQueueStorage` for the cross-plane move, which was previously unmodelled — canonical rows could only leave via `DrainCanonical`. Modelling the workload required tightening `DrainCanonical` to exclude a re-scheduling job, since such a job is executed but never completed; without that the bug is invisible to the model. New checks: `NoQueueRowsUnderCanonicalRouting`, `SnoozerImpliesCanonicalWork`, `MixedTransitionCanReduceCanonicalBacklog`, and the action property `RescheduleMigrationConservesWork`. A witness config models the pre-fix behaviour and violates the convergence invariant, showing mixed transition wedged with canonical work outstanding and `CanFinalize` unreachable. Convergence is encoded as safety, not liveness: the model legitimately permits a cluster never to prepare, to abort, or to stop every runtime, so no unconditional temporal property holds even with the fix. 768 distinct states (was 480); suite green at 22 checks. MAPPING.md records the FOR SHARE / action-atomicity correspondence and the refinement obligations the model cannot express (attempt identity, claim handoff). Docs: name the convergence mechanism in the substrate and upgrade guides, add the drain signal to the rollout watch list (it had no backlog metric), record the stall and its manual workaround under Known issues, and note that the leader promotes both planes' backlogs mid-transition. Also correct a pre-existing error while in the area: snooze re-schedules to `scheduled`, not `retryable`, in both planes.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/upgrade-0.5-to-0.6.md`:
- Line 174: The documented manual backlog migration must be atomic. Update the
workaround to require one transaction, lock each scheduled job and its
corresponding awa.job_unique_claims row before deletion, and then reinsert
through awa.insert_job_compat(...), preserving the original job payload and
uniqueness metadata; alternatively explicitly require all producers to be
stopped.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 600ca76a-07cd-4d83-b56e-2dbcfebc938c
📒 Files selected for processing (17)
awa-model/src/reschedule.rsawa-model/tests/sql_only_storage_upgrade_test.rsawa-worker/src/executor.rsawa/tests/lifecycle_hook_test.rscorrectness/run-tlc-suite.shcorrectness/storage/AwaStorageTransition.cfgcorrectness/storage/AwaStorageTransition.tlacorrectness/storage/AwaStorageTransitionCurrentGate.cfgcorrectness/storage/AwaStorageTransitionMigrate07Ungated.cfgcorrectness/storage/AwaStorageTransitionRescheduleStaysCanonical.cfgcorrectness/storage/MAPPING.mddocs/architecture.mddocs/configuration.mddocs/queue-storage-substrate.mddocs/upgrade-0.5-to-0.6.mddocs/upgrade-0.6-to-0.7.mdskills/awa-operations/SKILL.md
|
|
||
| - **Python workers without claim-ring knobs.** Older Python wheels don't accept `queue_storage_claim_slot_count` / `queue_storage_claim_rotate_interval_ms` on `client.start()`. They'll still run with default values and the rollout will work, but operators wanting non-default ring sizing need a wheel that exposes those kwargs. | ||
| - **Held-tx during finalize.** A long-running canonical transaction (e.g., reporting query) blocks vacuum, which can stall partition prune in the queue-storage tables. `awa_maintenance_prune_attempts_total{awa_ring_outcome="blocked"}` will rise. Identify and terminate the held-tx; prune resumes on the next maintenance tick. | ||
| - **Perpetually snoozing jobs stall the drain on older builds.** A handler that ends every run in `JobResult::Snooze` (recurring per-entity polls, heartbeat-style jobs) used to re-enter *canonical* `scheduled_jobs` after each post-flip execution, so `canonical_live_backlog` replenished itself and `awa storage finalize --wait` blocked indefinitely. Fixed in [#456](https://github.com/hardbyte/awa/issues/456): once routing has flipped, those re-schedules land in the prepared schema's `deferred_jobs`, so each job leaves the canonical plane at its next completion. On a build without the fix the workaround is to migrate the backlog by hand after the flip — take each `awa.scheduled_jobs` row, delete it and its `awa.job_unique_claims` entry, and re-insert it through `awa.insert_job_compat(...)`, which routes to queue storage post-flip. (A plain `INSERT` into the `awa.jobs` compatibility view does **not** route; it is canonical-only.) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make the manual backlog move atomic.
Deleting awa.job_unique_claims before reinserting permits a concurrent unique enqueue to take the key. Require a single transaction with appropriate locking (or require producers stopped), and preserve the original row payload/uniqueness metadata through the move.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/upgrade-0.5-to-0.6.md` at line 174, The documented manual backlog
migration must be atomic. Update the workaround to require one transaction, lock
each scheduled job and its corresponding awa.job_unique_claims row before
deletion, and then reinsert through awa.insert_job_compat(...), preserving the
original job payload and uniqueness metadata; alternatively explicitly require
all producers to be stopped.
…456) Second round of review findings on the cross-plane re-schedule. **Callback wiring would have been silently dropped.** `deferred_jobs` has no callback columns — queue storage keeps that state on the lease — so migrating a job that carries `callback_id` or any of the CEL expressions would have left the callback unresolvable with no way to resume it. The migrating DELETE now excludes such rows and they fall through to the canonical write, which applies the same `run_lease` guard: a callback-carrying job re-schedules in place, and a genuinely lost attempt still reports stale. This is a deliberate, narrow carve-out — a callback job is not the perpetual-snooze shape this path exists for, and it reaches a terminal state on its own. **`finalized_at` now comes from the database clock.** It was stamped with `Utc::now()` while `run_at` came from Postgres `now()`. Retention and cleanup compare `finalized_at` against the DB clock, so worker skew could shift those decisions; both values now come from one statement. **The rehearsal's phase-7 assertion was attributable to the wrong runtime.** Both clients registered the snoozer against one shared counter, so the post-finalize check only proved *some* runtime kept executing the bank — not that queue storage did, which is the property #456 is about. The queue_storage_target client now has its own counter, mirroring `rust_handled` / `qs_target_handled`. Adds a regression test for the callback carve-out: the job stays canonical under its own id, no queue-storage row is created, the callback wiring survives on the canonical successor, and a wrong-`run_lease` call still reports stale rather than being quietly re-scheduled by the fallback.
1 participant
Fixes #456.
The bug
canonical_live_backlogcounts all ofawa.scheduled_jobs, and the drain phase assumes that backlog converges as jobs come due and finish. For workloads whose handlers snooze on every run (recurring per-entity heartbeats implemented asJobResult::Snooze) it never does: the canonical executor's snooze path writes the job straight back into canonicalscheduled_jobs, so the backlog self-replenishes andawa storage finalize --waitblocks forever.Reproduced on a restored production-shaped snapshot with tens of thousands of jobs on a ~24h snooze cycle. After
enter-mixed-transition, new inserts routed to queue storage correctly, but every canonical job that came due was promoted to canonicaljobs_hot, executed, and re-snoozed back into canonicalscheduled_jobs. Queue-storagedeferred_jobsstayed empty;canonical_live_backlogstayed flat.This affects 0.7 as well as 0.6 — the ADR-037 gate blocks
awa migrateon an unfinalized cluster, but it does not stop a 0.7 worker from executing canonical work (canonical still works in 0.7, perdocs/upgrade-0.6-to-0.7.md, and is removed in 0.8).The fix
awa_model::rescheduleowns the snooze / retry-backoff / retry-after write and branches onawa.active_queue_storage_schema():jobs_hot -> scheduled_jobsmove as before, so pre-flip behaviour is byte-for-byte unchanged.mixed_transition/active— takes the canonical row and re-inserts the attempt into the active queue-storage schema'sdeferred_jobs, carrying over attempt, errors, progress,run_lease, timestamps, and the unique claim.Every canonical job therefore leaves the canonical plane at its next completion, and the existing drain/finalize flow converges for perpetual snoozers with no operator intervention.
No migration, deliberately
This ships as runtime SQL against the existing v040 schema rather than a v044 function. A migration would be unreachable by the clusters that need it: the ADR-037 gate refuses
awa migrateon exactly the unfinalized clusters where this bug manifests. It also keeps the fix backportable to the 0.6 line, whose migration numbering 0.7 already owns past v040. (Aqueue-storage-onlyvariant of this fix was prototyped as v044 first and discarded for these reasons.)run_leasesemanticsUnchanged, and strictly stronger. The take of the canonical row carries the same
state = 'running' AND run_lease = $nguard as the historical UPDATE. The successor takes a fresh id from the queue-storage sequence (the two planes have different id sequences, so reusing the canonical id could collide with a later queue-storage insert), which means a late completion from a rescued attempt matches no canonical row and reports stale. Both stale paths are covered by tests.Adjacent fix: leadership no longer gates drain
Two related stalls, both observed in the same rehearsal:
scheduled_jobs, anddeferred_jobs— so re-scheduled jobs that had already migrated sat unpromoted until leadership moved.Each maintenance leader now promotes the other plane's deferred backlog for the duration of the transition; the pre-flip case goes through a new
standby_queue_storagehandle, which is the configured-but-inactive queue-storage runtime a canonical-resolved worker already carries. Both paths are guarded no-ops on finalized and fresh installs (active_queue_storage_schema()/ empty canonical tables), so steady-state cost is one cheap query per promote tick.Tests
rolling_transition_rehearsal_test.rsgains a snooze-forever workload on its own queue. This is the gap that let #456 through: the existingsnooze_oncehandler snoozes 100ms once and then completes, so the drain converges and the perpetual case is unrepresented.canonical_backlog_countnow also countsscheduled_jobs, as its doc comment already claimed it did. The rehearsal asserts the snoozer bank has left the canonical plane and keeps executing on queue storage after finalize — it fails on the pre-fix code (at_finalize=763 after=763) and passes with it.A focused test in
sql_only_storage_upgrade_test.rscovers canonical routing (id preserved, snooze does not count the attempt), a stale wrong-run_leasecall, migrated routing (fresh id,retryablestate, payload metadata/tags/errors/progress preserved, unique claim re-pointed at the successor), the stale post-migration re-call, and a canonical backlog of zero with nothing having completed.migration_test.rsalso swaps two hardcoded43s forCURRENT_VERSION(with the version-floor rewinds pinned to 43 deliberately, so the pending range keeps crossing v043). Behaviour-neutral today; it stops the next migration bump from failing those tests spuriously.Verification
cargo test --workspace— 75 suites, 713 tests, clean run from a fresh schemacargo test -p awa --test rolling_transition_rehearsal_test -- --ignored— passes end-to-endpytest— 315 passed, 1 skippedcargo fmt --allandcargo clippy --all-targets --all-features -- -D warningscleanTwo environment notes for anyone reproducing locally, neither a code issue: the test Postgres needs
max_locks_per_transaction=2048(the substrate installs many partitions), and a staleawa_health_testdatabase carrying a leftover future-version row failshealth_endpoint_testuntil dropped.A 0.6.x backport of the fix (without the
migration_test.rshardening) follows separately againstrelease/0.6.3.Summary by CodeRabbit
Bug Fixes
Tests
Documentation