Skip to content

Fix keyed job scheduling race that can return no job under contention - #644

Open
shin4141 wants to merge 1 commit into
graphile:mainfrom
shin4141:codex/graphile-580-keyed-add-jobs
Open

shin4141 wants to merge 1 commit into
graphile:mainfrom
shin4141:codex/graphile-580-keyed-add-jobs

Conversation

@shin4141

Copy link
Copy Markdown

Fixes #580.

When concurrent callers schedule a job with the same key while an earlier job is
locked, one caller can finish the locked-job UPDATE before another transaction
inserts a locked row for that key. The following INSERT ... ON CONFLICT DO UPDATE ... WHERE jobs.locked_at IS NULL then returns no row. add_job can return
a null ID, leaving the later scheduling request without a queued replacement.

This adds a transaction advisory lock for each distinct non-null job key before
the existing UPDATE and INSERT. Batches acquire their key locks in sorted
order. Unkeyed jobs acquire no new advisory lock. The change is a new SQL
migration; existing migrations and the public API stay unchanged. The generated
SQL, schema snapshot, migration-count test, and patch changeset are included.

The new test uses a test-only BEFORE INSERT barrier to force the exact gap. It
fails on the existing SQL with Received: null for the later call and passes
with migration 000021.

Local preflight on Node 24 and PostgreSQL 14–18 passed the upstream CI functional
test stages on each version (24 suites, 106 tests, 19 snapshots per version).
The PostgreSQL 14 alternate-schema suite, TypeScript build, full lint, schema
dump, and Changesets status also passed. A separate bounded probe observed no
null IDs in 200 concurrent same-key calls; different keys proceeded independently;
and 40 reverse-order two-key batches returned all 80 rows without deadlock.

Each distinct keyed job in a batch holds one advisory lock until transaction
end. Isolated 3,000-key batches passed on PostgreSQL 14–18, and three concurrent
3,000/3,000/4,000-key transactions passed on PostgreSQL 17. These measurements
do not establish a production throughput or universal batch-size limit; the
shared advisory-lock pool and large-batch behavior deserve review. A transaction
with an older snapshot at REPEATABLE READ or SERIALIZABLE can still receive
SQLSTATE 40001; the same scenario does so on the existing function, and callers
at those levels need whole-transaction retry.

The repository's local yarn test wrapper stops at depcheck because it reports
the existing @changesets/changelog-github devDependency as unused. The CI
functional test stages above completed independently. Hosted CI and maintainer
review have not run on this proposed patch.

This branch has not been deployed

No deployments
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.

Race in add_job/add_jobs with keyed jobs can drop scheduling requests (and returns no row under contention)

1 participant