refactor: unify queue and workflow runtime#4
Open
cmilesio wants to merge 26 commits into
Open
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
cmilesio
marked this pull request as ready for review
July 19, 2026 02:13
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
This PR collapses the overlapping queue and workflow models into one queue-owned runtime.
queue.Queueis the canonical application surface,internal/workflow.Engineowns orchestration, drivers own physical delivery, andbusremains as a deprecated compatibility facade.Fresh review hardening
The three independent review rounds found 3, 6, and 5 primary findings. All 14 are fixed. A final post-fix audit by API, runtime-concurrency, and CI-evidence specialists found five narrower continuity edges; those are fixed too, and the reviewers report no surviving issue.
A separate changed-line audit by NATS, RabbitMQ, Redis, and documentation specialists added the one remaining deterministic production contract: a real NATS subscription failure must leave startup retryable. The other uncovered markers require impossible fixed-shape serialization errors, invalid dependency wiring, global entropy hooks, concrete-client transport fault injection, or artificial process timeouts, so no production-only test seams were added.
The important before-and-after changes from those reviews are concrete:
process_failedfollowed bysettlement_failedcould decrementActivetwice or consume a newer duplicateActivelive and produce no failed-attempt telemetryprocess_failedis emitted, correlation state closes, and the original panic value is rethrown so backend semantics stay intactObserverFuncsignaturefunc(context.Context, queue.Event)and a guard pins their exact manual textGOWORK=off; an always-run fan-in preserves the historical requiredracecheckSettlement-aware metrics now have one unambiguous ownership rule:
flowchart TD START["process_started with opaque<br/>physical identity"] --> ACTIVE["Open exactly one Active execution"] TERMINAL{"Terminal fact"} ACTIVE --> TERMINAL TERMINAL -->|process success or failure<br/>with same identity| CLOSE["Close that exact execution"] TERMINAL -->|settlement failure<br/>with same identity| CLOSE TERMINAL -->|identity missing| KEEP["Leave identity-bearing Active unchanged"] CLOSE --> COUNTERS["Settlement failure changes neither<br/>Processed nor application Failed"] KEEP --> SAFE["Conservative overcount is safer than<br/>undercounting a newer live execution"]The nil path changed from forwarding an invalid target:
to preserving the existing registry exactly:
The reliability evidence now has an explicit provenance chain:
flowchart LR SRC["Integration and root tagged sources"] --> HASH["SHA-256 evidence manifest"] FULL["All-backend executed run count"] --> HASH HASH --> BADGE["README integration badge"] GEN["README, examples, counts,<br/>and benchmark generators"] --> PARITY["Checked-in parity plus<br/>second-run idempotency"] JSON["Exact go test JSON event"] --> OUTCOME{"Terminal action"} OUTCOME -->|pass| PASS["Executed pass"] OUTCOME -->|skip| SKIP["Capability skip"] OUTCOME -->|fail| FAIL["Executed failure"] OUTCOME -->|absent| MISSING["Missing event and failing job"]Architecture before and after
flowchart LR subgraph BEFORE["Before: split ownership"] direction TB BAPP["Application"] --> BQ["queue.Queue facade"] BAPP -. legacy calls .-> BB["bus surface"] BQ --> BQR["Queue registry, lifecycle,<br/>and observer"] BQ --> BWE["bus-owned workflow engine, models,<br/>store, middleware, fake, and observer"] BB --> BWE BQR --> BRT["Runtime seam"] BWE --> BRT BRT --> BDR["Queue drivers"] end subgraph AFTER["After: queue-owned composition"] direction TB AAPP["Application"] --> AQ["queue.Queue"] ALEG["Legacy bus caller"] --> AB["Deprecated bus facade"] AB --> AQ AQ --> AOWN["Shared registry, middleware,<br/>and lifecycle"] AQ --> AWE["Canonical workflow engine"] AQ --> AOBS["Unified queue.Event stream"] AAPP -. tests .-> AFAKE["Root queue fake"] AFAKE --> AWE AWE --> AST["Root workflow models and store"] AOWN --> ART["Runtime seam"] AWE --> ART ART --> ADR["Queue drivers"] endThe root queue already exposed chains and batches before this PR. The change is ownership and continuity: those calls now use root
queuetypes and one shared engine instead of crossing into a second public model. The low-level raw runtime route remains for compatibility, but it is not the recommended application path.Application code before and after
Before, root workflow configuration still depended on public
busmodels:After, the same application uses one root model.
Event.Layerdistinguishes queue and workflow facts without creating a second observer pipeline:The queue constructor and workflow call stay familiar:
An option-free
bus.New(existingQueue)now returns a compatibility view over this same engine. It shares handlers, middleware, stores, workflow state, observers, and shutdown state. Options that would create conflicting ownership are rejected withbus.ErrQueueOptionsUnsupportedand must be applied when the root queue is constructed.Direct jobs now remain direct
This application call is unchanged:
Before, the driver received a workflow protocol message and a JSON wrapper:
{ "schema_version": 1, "dispatch_id": "dsp_0123456789abcdef", "kind": "job", "job_id": "job_0123456789abcdef", "attempt": 0, "job": { "type": "emails:send", "payload": "eyJpZCI6MX0=", "options": { "Queue": "critical", "Delay": 0, "Timeout": 0, "Retry": 0, "Backoff": 0, "UniqueFor": 0 } } }After, the driver receives the application type and exact application bytes. Correlation travels beside the payload in versioned metadata:
{ "schema_version": 1, "dispatch_id": "dsp_0123456789abcdef", "job_id": "job_0123456789abcdef", "queue": "critical" }Arbitrary bytes are preserved, and an absent payload stays absent. Chains, batches, callbacks, reserved
bus:*types, and the raw runtime route intentionally retain the version-one workflow envelope.flowchart LR A["Queue.Dispatch"] --> B{"Legacy mode or<br/>reserved type?"} B -->|Yes| C["Version 1 workflow envelope"] B -->|No| D["Application type<br/>and exact payload"] D --> E["Versioned correlation metadata<br/>beside the payload"] C --> OLD["Old and new workers"] E --> NEW["New workers only"] OLD --> PIPE["Canonical handler and middleware pipeline"] NEW --> PIPE PIPE --> SETTLE["Backend settlement boundary"] SETTLE --> OBS["Best-effort observer facts"]Compatibility is intentionally one-way during rollout. New workers read old and new deliveries; old workers cannot safely consume the new direct format. For non-SQL backends, deploy upgraded workers while producer instances keep
queue.WithLegacyDirectEnvelope(), then remove that producer option after old consumers are gone. SQL requires a fleet replacement: quiesce every old SQL worker before any new SQL worker starts, even while legacy producer emission remains enabled.One runtime behavior map
busmodels and orchestrationqueueowns the public model; one internal engine owns orchestrationbus.New(existingQueue)shares the root enginequeue.Eventstream usesEvent.Layer, one effective physical queue name, and opaque identity for settlement-aware Active gaugesbus:jobJSON envelopeSQL receipt-backed handler replay suppression
On the built-in SQL queue plus SQL workflow-store path, the logical workflow outcome and an immutable transition receipt commit in one database transaction. The memory store implements the same contract when complete delivery provenance is supplied, but its receipts are process-local. SQL settlement is separately generation-fenced. If a failed settlement leaves or restores a recoverable row, a later delivery can prove that application work already committed without pretending queue settlement was atomic with it.
flowchart TD A["SQL worker claims a delivery<br/>and processing generation"] --> B["Run workflow handler"] B --> C["Store transaction commits<br/>workflow outcome plus receipt"] C --> D["Attempt successor work<br/>and physical settlement"] D --> E{"Settlement commits?"} E -->|Yes| F["Publish eligible observer facts<br/>best effort"] E -->|No| R{"Delivery remains or<br/>becomes recoverable?"} R -->|No| X["No receipt-based replay<br/>on that path"] R -->|Yes| G["Redelivery enters recovery"] G --> H["Validate receipt version, identity,<br/>fingerprint, and workflow state"] H --> I{"Proof valid?"} I -->|No| J["Return uncommitted<br/>no ack and no effects"] I -->|Yes| K["Suppress duplicate handler execution"] K --> L["Restore eligible outcome<br/>and retry settlement"] L --> DThis is not an exactly-once claim. Receipts do not make settlement, successor enqueue, closure callbacks, batch fanout, or observer delivery part of the store transaction. Only the exact physical receipt owner can reconstruct predecessor success facts. Lineage repair is best effort, and chain successor repair remains at least once.
Compatibility and rollout
metadata_jsonandprocessing_tokenbus_workflow_transition_receiptsand validates connected MySQL identity columnsRetry(0)now means zero instead of Asynq's default 25Retry(25)explicitly if that was the intended policybus.Observerandbus.Eventtoqueue.Observerandqueue.Eventqueue.ObserverFuncand useEvent.Layer; the legacy bus facade still translates eventsbusruntime.DeliverySettlementIdentity; identity-less terminal facts cannot close an identity-bearing startprocess_failedbefore the original panic is rethrown; backend panic recovery and retry behavior is unchangedUniqueForidentity/busto root/queueFailChainkeeps the first terminal causeWorkflowStorecallsSchema, durability, and test boundaries
queue.NewSQLStoreWithManagedSchemaperforms no workflow DDL, so managed deployments must precreate the complete receipt table. Incompatible existing MySQL workflow identities require a quiescent persisted-schema migration and a fresh store; compatible established rows remain readable. Queue database migration remains enabled by default and can be disabled withDisableAutoMigrate. Existing queue rows remain readable after the two nullable queue columns are added. Automatic migration also adds the SQL queue uniqueness-lock expiry index needed for bounded pruning.plan.md.The branch includes literal legacy wire and SQL fixtures, public compile contracts, race coverage, store and lifecycle fault injection, and real SQLite, MySQL, and PostgreSQL recovery scenarios. CI validates all 12 Go modules and runs null, Sync, Workerpool, Redis, NATS, SQS, RabbitMQ, MySQL, PostgreSQL, and SQLite independently. Coverage fans in every buildable module plus all ten backend profiles, rejects incomplete or duplicate inputs before upload, and submits one complete report under the repository's Codecov project and patch policy.
The complete fan-in also drove focused scripted SQL and driver tests for migration races, settlement repair, uniqueness compensation, retry decisions, malformed deliveries, readiness failures, and shutdown boundaries. A follow-up pass proves NATS shutdown waits for accepted handlers, Redis ownership reaches real command semantics, and RabbitMQ immediate retry reaches attempt one, commits, and leaves its broker queue empty. Defensive invariant tests verify ambiguous SQL claim results roll back and absent SQS client results fail closed without presenting those results as production client behavior. Fixed-structure serialization, entropy-source failures, and failures emitted only inside concrete broker clients remain explicit instead of adding production hooks solely for coverage.
Why
The repository had two application models over the same delivery layer. Root
queueowned direct jobs and worker lifecycle, whilebusseparately owned workflow types, stores, middleware, observers, fakes, and orchestration state. The same physical queue could therefore have two registries, two observer pipelines, and different retry or shutdown behavior depending on the facade a caller used.That duplication also blurred three separate facts: application state committed, the physical delivery settled, and an observer received an event. During retries or finalization failures those facts can diverge. Treating them as one fact risks repeated handlers, contradictory workflow outcomes, premature success events, or lost continuation work.
This PR gives each fact one owner. Durable store state determines workflow truth, the driver boundary determines physical completion, and observation remains telemetry. The result is one ergonomic root API with explicit compatibility and durability boundaries instead of two partially overlapping systems.