Repository navigation
Description
Follow-up from #74. Low priority: consistency with PostgreSQL and future-proofing, not a merge blocker. Line numbers are at 112dcf0.
Problem
#74 changed the SQLite worker fetch to ORDER BY q.visible_at, q.id, so work that becomes ready late no longer goes ahead of work that was ready earlier. The orchestrator queue still picks the ready row with the lowest id: ORDER BY q.id in src/providers/sqlite.rs:781 (with a capability filter) and :800 (without).
PostgreSQL already orders this fetch by readiness: duroxide-pg migrations/0025_fetch_orchestration_lease_at_claim.sql:85 and :95 use ORDER BY q.visible_at, q.id. The two providers disagree, and no provider validation test checks the order.
Two kinds of rows become ready long after they get their id:
TimerFiredrows are inserted when the timer is scheduled, withvisible_at = fire_at(sqlite.rs:1587-1590). A one-hour timer carries an id from an hour earlier.abandon_orchestration_itemwith a delay keeps the row and its id and movesvisible_at(sqlite.rs:2248-2250). The dispatcher does this for an incompatible version (src/runtime/dispatchers/orchestration.rs:442), history that fails to deserialize (:585), a continue-as-new successor that is not visible yet (:624), an unregistered orchestration (:745), and exhausted failure-commit or ack retries (:1048,:1239).
Under a backlog, these rows go ahead of instances that have been ready longer.
Impact
Low. Every orchestration abandon uses up an attempt, so a failing instance can jump the queue at most max_attempts times before it is poisoned, and a timer fires once. This is priority inversion, not starvation. Correctness is not affected: one fetch takes all ready messages of the chosen instance (sqlite.rs:876-889), in id order (:896). It matters most in bursts, such as a deploy that drops an orchestration registration, or many timers firing together.
Proposed change
ORDER BY q.visible_at, q.idin both orchestrator queries (sqlite.rs:781,:800).- A migration with
CREATE INDEX IF NOT EXISTS idx_orch_ready_order ON orchestrator_queue(visible_at, id);. File-backed databases get indexes only throughmigrations/(sqlx::migrate!atsqlite.rs:172), as f8d4db7 did for the worker queue. The existingidx_orch_visible(visible_at, lock_token)breaks ties bylock_token, notid. - A provider validation test, the orchestration counterpart of
test_worker_deferred_item_does_not_starve_ready_work(src/provider_validation/queue_semantics.rs:121):- Abandon: start instances A and B, fetch A, abandon it with a 1 ms delay, and wait 20 ms. The next fetch must return B.
- Timer: enqueue a
TimerFiredfor A that fires in 10 ms, then a message for B. After 20 ms, the next fetch must return B, which became ready first.
By code reading, SQLite fails the timer case today and PostgreSQL passes it.
Filed for Affan Dar (@affandar) from the #74 review.
Activity