Uh oh!
There was an error while loading. Please reload this page.
fix(world-postgres): serialize keyed deliveries across workers - #3657
Closed
joeyhotz wants to merge 1 commit into
Closed
fix(world-postgres): serialize keyed deliveries across workers#3657joeyhotz wants to merge 1 commit into
joeyhotz wants to merge 1 commit into
Conversation
Assign idempotent jobs to bounded Graphile named queues so a locked delivery and its durable replacement cannot execute concurrently across worker processes. Keep default replacement semantics for delayed retries and crash recovery. Signed-off-by: Joey Hotz <joeyhotz1@gmail.com>
🦋 Changeset detectedLatest commit: 163dbf4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
@joeyhotz is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
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 freeto 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.
Description
@workflow/world-postgrescurrently passesQueueOptions.idempotencyKeyto Graphile Worker asjobKey. With Graphile's defaultreplacemode, adding the same key while its current job is locked clears the locked job's key, exhausts that row, and inserts a successor. The existinginflightMessagesmap prevents overlap inside one process, but separate Postgres World processes can claim the current job and its successor at the same time.We observed this after a self-hosted deployment scaled out: startup recovery replayed active runs and redispatched their keyed steps. In one staging burst, 7 of 34 materialisation steps received a second start without an intervening failure (41 executions total). The same startup-recovery/duplicate-start signature appeared in production. This behavior is present in
@workflow/world-postgres4.3.3 and currentmain.This PR assigns every keyed Graphile job to a deterministic named queue. A named queue is Graphile's native cross-worker concurrency primitive: while one worker owns the queue, a replacement remains pending. After the first delivery commits and releases the queue, the successor can replay the terminal state instead of overlapping the original side effect.
The implementation deliberately:
replacemode. This preserves the durable successor created by delayed rescheduling and crash recovery.idempotencyKey. Unkeyed orchestrator messages and public World interfaces are unchanged.jobPrefix. Exact per-key queue names would grow Graphile's persistent queue table without bound; hashing the job task name keeps prefixes isolated and the physical name below Graphile's 128-character limit.MessageData.idempotencyKey. Legacy unkeyed rows usedmessageIdassingleton_keyand remain unqueued.GC_JOB_QUEUES; Graphile 0.16.6 cleanup raced concurrent producers in a local stress test and left 398 of 7,680 jobs orphaned (5.18%).jobKeyMode: 'unsafe_dedupe'is not safe here. The handler enqueues a delayed successor before its current locked job returns;unsafe_dedupewould discard that successor, after which completing the current row can strand the workflow. It can also suppress the only replacement for a locked job that later dies on its final attempt.This serializes competing deliveries; it does not claim exactly-once execution. The runtime's terminal-state replay remains the duplicate-suppression layer after the queue releases.
Operational trade-offs:
queueNamedo not own the new queue. Deployments must drain old keyed jobs and producers for a fully protected handoff.A shuffled three-repetition Graphile 0.16.6 no-op benchmark at concurrency 50 / pool 8 measured median throughput of 181.5 jobs/s with no named queues, 141.1 jobs/s with all 2,048 buckets populated, and 108.2 jobs/s with 4,096. The VPS timings were noisy, so these are directional rather than a production capacity claim; 2,048 was consistently the better balance. Dropping to 1,024 would double expected collision loss at the default concurrency to roughly 2.4%.
This complements #3119 and #3162 but does not replace them. Those address selecting parked runs and accumulating startup-recovery jobs; this PR closes the separate locked-keyed-delivery concurrency gap. The
queueNameseam is independent of the HTTP loopback and can be carried through the in-process execution refactor in #3322.How did you test your changes?
Added unit coverage for:
jobKey, named queue,runAt, and 49-attempt budget.jobPrefixvalues receiving isolated queue scopes.Added a real-PostgreSQL Testcontainers regression with two independent pools and two
createQueue()instances. It blocks worker A's HTTP delivery, enqueues the same key through worker B, waits through the polling window, and asserts that both rows share one named queue while only the first is locked. Before the fix, the same test observed two concurrent HTTP deliveries (maxActiveRequests: 2); with the fix it observes one (maxActiveRequests: 1) and then drains both jobs sequentially.Verification:
pnpm exec vitest run packages/world-postgres/src(3 files, 30 tests)pnpm --filter @workflow/world-postgres typecheckpnpm --filter @workflow/world-postgres buildqueue.ts/queue.test.ts)pnpm changeset statusAdditional recovery probe with Graphile Worker 0.16.6: a child worker was killed while holding the named queue, a same-key replacement was added, and after
forceUnlockWorkersonly the replacement executed.PR Checklist - Required to merge
pnpm changesetwas run to create a changelog for this PR@workflow/world-postgres.git commit --signoffon your commits)@vercel/workflowin a comment once the PR is ready, and the above checklist is completeDraft for author review. No reviewers have been requested.