Fix localQueue running jobs from the same named queue concurrently - #622
Open
anavalo wants to merge 1 commit into
Open
Fix localQueue running jobs from the same named queue concurrently#622anavalo wants to merge 1 commit into
anavalo wants to merge 1 commit into
Conversation
With batching enabled (batchSize > 1), a single job fetch could lock multiple jobs belonging to the same named queue: the queue eligibility subquery locks each available queue row once, but every job in that queue then passes the membership check, so `limit batchSize` could take several of them. The local queue would hand them to workers concurrently, and per-queue concurrency became min(localQueue.size, concurrentJobs) instead of 1. Batch fetches now keep only the first job per named queue and discard the rest; the discarded rows' locks release at the end of the statement and the queue row remains locked until the kept job completes, so the serial execution guarantee holds again. The batchSize = 1 query is unchanged.
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.
Description
Fixes #621.
With
localQueueenabled, jobs from the same named queue could run concurrently, violating the documented serial execution guarantee for named queues. A batch fetch (batchSize > 1) could lock multiple jobs belonging to the same named queue in a single statement: the queue eligibility subquery locks each available queue row once, but every job in that queue then passes theinmembership check, solimit batchSizecould take several of them. The local queue would hand these out to idle workers concurrently, making per-queue concurrencymin(localQueue.size, concurrentJobs)instead of1. And once the first of those sibling jobs completed,completeJobswould unlock the queue row while the rest were still running, so other pools could lock yet more jobs from the same queue.This PR makes batch fetches keep only the first job (by
priority,run_at) per named queue and discard the rest. The discarded rows never getlocked_at/locked_byset and their row locks release when the statement completes, while the queue row remains locked until the one kept job completes — restoring the invariant that a locked queue has exactly one job in flight, whichcompleteJobs,returnJobs, and thebatchSize = 1fetch already rely on. Jobs that aren't in a named queue are unaffected, and thebatchSize = 1query text is byte-for-byte unchanged.The reproduction script from #621 now reports
1concurrent job (previously4), and the 12 jobs execute in insertion order.Two new tests in
__tests__/main.runTaskList.test.ts(the suite previously had no test exercisinglocalQueue):main;main(all four jobs start at once there), and guards against over-serialising the batch.Performance impact
localQueuedisabled (batchSize = 1): none — the query text is unchanged.distinct onprocesses at mostbatchSizealready-locked rows, which is negligible.limit batchSize, a fetch returns fewer thanbatchSizejobs whenever several of the selected rows share a named queue. The worst case is a backlog whose highest-precedence jobs all belong to one named queue while its tasks complete faster thanpollInterval: each poll's fetch then selects that queue's jobs again and discards the duplicates, yielding a single job, so the pool works that queue at roughly one job perpollIntervaland other ready jobs wait despite idle workers until the queue's backlog drains below the batch window. The effect is bounded by fetch timing: if the queue's job is still running at fetch time, its queue row is locked, its jobs are excluded, and the batch fills with jobs from other queues and unqueued jobs; a "new job" notification also triggers an immediate fetch with the same effect.localQueue.refetchDelaywith a non-zerothresholdmay see the refetch delay engage more often in workloads dominated by a few named queues.Security impact
None.
Checklist
yarn lint:fixpasses.yarn testpasses.I have detailed the new feature in the relevant documentation.Bug fix — the documentation already describes the (serial) behaviour this PR restores.RELEASE_NOTES.mdfile (if one exists).