Skip to content

Accept notified jobs for workers waiting on an empty buffer - #450

Open
salimepoint wants to merge 1 commit into
que-rb:masterfrom
LimePoint:accept_jobs_for_waiting_workers
Open

Accept notified jobs for workers waiting on an empty buffer#450
salimepoint wants to merge 1 commit into
que-rb:masterfrom
LimePoint:accept_jobs_for_waiting_workers

Conversation

@salimepoint

Copy link
Copy Markdown

The bug

JobBuffer#accept? decided how many notified jobs to take from the free buffer space alone:

start_index = _buffer_space

So a locker running with maximum_size: 0 accepted none of them. Its buffer is always empty, so there is never free space and never anything buffered to displace — every notified job was rejected and left to be found by the next poll instead, up to poll_interval late, or indefinitely with polling turned off.

maximum_size: 0 is a reasonable setting for a queue of long-running jobs, where a locker that pre-locks a job the running job goes on to wait for will deadlock. Those are exactly the deployments that then get no benefit from NOTIFY at all.

The fix

Fall back to the workers currently blocked waiting for a job when the buffer has no space to offer.

That's safe because push hands jobs straight to waiting workers (via priority_queues … pq.populate) before it trims the buffer to maximum_size — so a job claimed by an idle worker never occupies buffer space and is never pre-locked. available_priorities already sizes the poller's fetch as the waiting workers plus the buffer space, so accept? was the one place left that ignored them.

Behaviour is unchanged whenever the buffer has room to fill, since the fallback only engages when there is no space at all. With a full buffer, accept? now also takes what the waiting workers will accept on top of what it can displace; push places those and returns the remainder for push_jobs to unlock, as it already does for any other overage.

Priority awareness

The count has to be priority-aware, since a worker only accepts jobs its threshold admits. metajobs is sorted, and a threshold admits every job at least as important as the ones it rejects, so the eligible worker count only falls as the walk advances — the first job that can't be placed for certain ends it.

The walk is deliberately conservative rather than exact: push offers each job to the most permissive waiting worker first, so counting every placement a perfect assignment could make would accept jobs that push then has to displace.

Tests

New coverage in job_buffer_spec for: no worker waiting (still rejects); accepting no more than the waiting-worker count; an accepted job reaching the worker without ever being buffered; spare buffer space still capping acceptance regardless of waiting workers; a full buffer taking the waiting worker's job on top of what it displaces; and a worker whose priority threshold does and doesn't admit the offered job.

accept? decided how many notified jobs to take from the free buffer
space alone, so a locker running with maximum_size: 0 accepted none of
them. Its buffer is always empty, so there is never free space and never
anything buffered to displace, and every job waited for the next poll
instead - up to poll_interval late, or indefinitely with polling off. A
buffer size of zero is a reasonable setting for a queue of long running
jobs, where a locker that pre-locks a job the running job goes on to
wait for will deadlock.

Fall back to the workers currently blocked waiting for a job when the
buffer has no space to offer. That is safe because push hands jobs
straight to waiting workers, before it trims the buffer to maximum_size,
so a job claimed by an idle worker never occupies buffer space and is
never pre-locked. available_priorities already sizes the poller's fetch
as the waiting workers plus the buffer space, so accept? was the one
place left that ignored them.

Falling back only when there is no space leaves accept? behaving exactly
as it did before whenever the buffer has room to fill. With a full
buffer it now also takes what the waiting workers will accept on top of
what it can displace; push places those and returns the remainder for
push_jobs to unlock, as it does for any other overage.

The count is priority aware, since a worker only accepts jobs its
threshold admits. metajobs is sorted and a threshold admits every job at
least as important as the ones it rejects, so the eligible worker count
only falls as the walk advances and the first job that cannot be placed
for certain ends it. The walk is therefore conservative rather than
exact: push offers each job to the most permissive waiting worker first,
so counting every placement a perfect assignment could make would accept
jobs push then has to displace.
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