Skip to content

fix(qb): window candidates before locking - #774

Merged
janbjorge merged 1 commit into
mainfrom
fix/dequeue-window-before-locking
Aug 30, 2026
Merged

fix(qb): window candidates before locking#774
janbjorge merged 1 commit into
mainfrom
fix/dequeue-window-before-locking

Conversation

@janbjorge

@janbjorge janbjorge commented Aug 30, 2026

Copy link
Copy Markdown
Owner

Part 1 of the #761 fix, the half that needs no migration and so does not wait on #751.

Workers dequeuing concurrently can exceed concurrency_limit. The capacity count reads its own statement snapshot, so another worker's uncommitted claim is invisible, and LIMIT ... FOR UPDATE SKIP LOCKED slides past the rows that worker holds onto further queued rows: a full batch picked on top of a limit already spent.

Limited entrypoints now fix their candidate window before taking any lock. The window's LIMIT keeps the lock node out from under a LIMIT, where the slide happens, and each window row is re-fetched by primary key in its own LATERAL and locked there. SKIP LOCKED can then only skip rows inside the window, so a row another worker holds shrinks this pick instead of pushing it over the limit. Unlimited entrypoints keep the direct scan, where the slide costs throughput but not correctness. Only the two capacity-gated snapshots move; the ungated ones stay byte-identical.

This holds while both workers derive the same candidate list. It stops holding once the ordering changes under them: a higher-priority job arriving mid-claim gives the second worker a window the first never locked. That needs a capacity slot on the row, and a migration for it, so it waits for #751. Hence Refs #761, not Closes.

test_concurrency_limit_holds_across_concurrent_dequeues reproduces the race: worker A holds a full batch in an open transaction, worker B must claim nothing. It polls pg_stat_activity rather than sleeping, so it does not depend on whether B skips, waits, or retries. Fails on main, passed here five runs straight.

118 passed across shapes, concurrency, plan regression, and queries. mypy 156, ruff, import-linter 4/4.

concurrency_limit could be exceeded by workers dequeuing
concurrently. The capacity count reads its own statement snapshot,
so a concurrent uncommitted claim is invisible, and LIMIT ... FOR
UPDATE SKIP LOCKED slides past the rows another worker holds onto
further queued rows.

Limited entrypoints now fix their candidate window before any
locking. The window's LIMIT stops the planner pulling the subquery
up, which would put the lock node back under a LIMIT and restore
the slide-down. Each window row is re-fetched by primary key in its
own LATERAL and locked there, so SKIP LOCKED can only skip rows
inside the window: a row a concurrent worker holds shrinks this
pick rather than pushing it over the limit.

That closes the reported race, because both workers derive the same
window and collide on the same row locks. It stops holding as soon
as the ordering changes under them: a higher-priority job arriving
inside another worker's in-flight claim gives the second worker a
window the first never locked. Closing that needs a capacity slot
on the row, which needs a migration, so it lands separately once
#751 is in.

Unlimited entrypoints keep the direct scan -- the slide is pure
throughput there. The ungated shapes render byte-identical SQL,
which is what proves this touches only entrypoints that carry a
limit.

Refs #761
@janbjorge
janbjorge force-pushed the fix/dequeue-window-before-locking branch from 248c9bd to 55a31dd Compare August 30, 2026 07:57
@janbjorge
janbjorge merged commit 53ea72d into main Aug 30, 2026
22 checks passed
@janbjorge
janbjorge deleted the fix/dequeue-window-before-locking branch August 30, 2026 08:01
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.

1 participant