Repository navigation
Conversation
There was a problem hiding this comment.
馃煛 Changes recommended
Verified shutdown timing and signal-wakeup issues can leave the dispatcher blocked or delay its exit.
2 open issues
What changed in this PR
Replaces job-dispatch polling with event-driven wakeups to reduce fetched-job latency.
Changes:
- Signals the dispatcher when jobs enter the queue.
- Waits for queued jobs, task completion, or shutdown.
- Adds latency tests for idle dispatch, busy dispatch, and shutdown.
| File | Description |
|---|---|
| tests/鈥媡est_serverless/鈥媡est_rp_scale.py | Adds dispatch-latency and prompt-shutdown tests. |
| runpod/鈥媠erverless/鈥媘odules/鈥媟p_scale.py | Replaces polling with event-based dispatch waits. |
Review effort: Balanced
Comment on lines
+419
to
+422
| waiters = {asyncio.create_task(self._jobs_available.wait())} | ||
| if self.is_alive(): | ||
| # Once shutting down, the set event would wake this loop instantly. | ||
| waiters.add(asyncio.create_task(self._shutdown_event.wait())) |
Comment on lines
+424
to
+425
| done, _ = await asyncio.wait( | ||
| tasks | waiters, return_when=asyncio.FIRST_COMPLETED |
deanq
force-pushed
the
deanq/sls-810-event-driven-dispatch
branch
from
October 19, 2026 17:21
dcff6bc to
cc57fc5
Compare
run_jobs slept on a 100ms poll between queue checks, so a fetched job waited up to 100ms before its handler started. On cold start this delay landed on every worker's first job. - get_jobs sets an asyncio.Event after queuing jobs - run_jobs waits on job completion, that event, or shutdown - Event instead of queue.get(): set_scale() replaces jobs_queue, which would strand a pending getter on the old queue Linear: SLS-810
deanq
force-pushed
the
deanq/sls-810-event-driven-dispatch
branch
from
October 19, 2026 17:47
cc57fc5 to
1c53709
Compare
Collaborator
|
Didn't find any new issues beyond Copilot's 2 comments. |
This branch has not been deployed
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.
Summary
JobScaler.run_jobschecked the job queue on a 100ms poll (asyncio.sleep(0.1)/asyncio.wait(timeout=0.1)). A job thatget_jobshad already fetched could wait up to 100ms before its handler started. On cold start, every worker's first job hit this delay.get_jobssets anasyncio.Eventafter putting jobs on the queuerun_jobsblocks on whichever comes first: a running job finishing, that event, or shutdownWhy an event rather than
await jobs_queue.get():set_scale()replacesself.jobs_queue, so a getter waiting on the old queue would never wake.Impact
Local deployed-worker simulation (fake job API, fresh container): time from job fetch to handler call went from ~100ms to ~1ms.
Test plan
test_rp_scale.pyrepeated 5x, no flakesmake quality-check: 1482 passed, 91.65% coverageLinear: SLS-810