Add limit option for bounded prefetch, and drain for graceful shutdown - #9
Draft
himynameisjonas wants to merge 1 commit into
Draft
Conversation
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.
We run this processor in production for LLM jobs (long-running, IO-bound, ~25k jobs/day) and hit two operational issues that this PR addresses.
Bounded prefetch (
limit:). The dequeue loop currently fetches jobs as fast as they arrive, so any backlog accumulates as spawned tasks inside the server process rather than in Redis. In our deployment this had three effects: the ready list read zero while ~1,100 claimed jobs were parked in-process (we were watching queue depth to detect backlog, and it lied), memory grew with the backlog since each parked job holds its deserialized payload, and every shutdown abandoned the whole pile at once, to be requeued and re-run by the recovery mechanism. Withlimit: nthe server acquires a semaphore slot beforeBRPOPLPUSH, so at mostnjobs are claimed at a time and backlog stays in the ready list where it is visible and cheap. Without the option, behavior is unchanged.Graceful drain (
#drain). Stopping the server cancels running jobs mid-flight; the abandoned-job recovery then requeues and re-runs them. For at-least-once processing that is correct but expensive when jobs are costly to repeat (in our case, one LLM call per job, plus duplicated side effects, on every deploy).drain(timeout:)stops the fetch loop and waits for running jobs to finish, up to the timeout. Jobs that do not finish in time are cancelled and recovered exactly as before. To support this, job tasks now run on their own barrier hosted outside the fetch loop's task tree, so stopping the loop does not cancel them;#stopstops both, preserving current semantics.Both features are in production for us (as a subclass carrying these exact changes): deploys went from cancelling and re-running 60-920 jobs each to finishing the running jobs in ~10s and leaving the backlog in the ready list.
Tests included for both. Happy to split this into two PRs if you prefer, or rework the approach.