Repository navigation
feat(benchsdk-runner): deadlineMs — a run-level deadline drains and finishes, not fails - #518
Merged
Merged
Conversation
… end, not a failure Snapshot/Fork run #5 paged as 'Job failed' when the workflow's `timeout 60m` wrapper SIGKILLed the bench at iteration ~57/100 on the neon and cloudflare-r2 legs (exit 124). The cap was user-defined, so the job shouldn't have failed — and timeout(1) can't be made graceful anyway: it returns 124 no matter how the process exits. Give the runner a first-class deadline instead. `deadlineMs` on the bench config (or --deadline-ms) sets a wall-clock budget for task execution: on expiry the schedulers stop launching tasks, in-flight ones finish, and the run still flushes, reports, and exits 0. Plumbing is RunWorkerOptions.stopRequested — the platform worker polls it before each new task start. BenchmarkRunOutcome.stoppedEarly flags truncation. snapshot-fork-benchmarks.yml swaps `timeout 60m` for --deadline-ms 3600000 with a `timeout 75m` hard backstop under the job's 90m cap.
Contributor
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
HeyGarrison
marked this pull request as ready for review
October 17, 2026 11:57
Contributor License AgreementAll contributors are covered by a CLA. |
Contributor
There was a problem hiding this comment.
✅ Devin resolved all 3 findings on 4f6ed55
Fixed by Devin (3)
- Staggered tasks start after deadline
- Round participants launch after deadline
- Skipped participants vanish from partial runs
…participants - local + platform stagger sleeps now re-check expired() before launching - platform path throws BenchmarkTaskSkippedError so the worker drops the record entirely (never launched, not an error) - round mode re-checks after the round sleep and before each queued participant's task - participants that never started are still reported (empty records) so a deadline-stopped run marks stoppedEarly and doesn't silently drop them
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
Snapshot/Fork run #5 paged as
Job failedbecause the workflow'stimeout 60mwrapper SIGKILLed the bench mid-run at ~iteration 57/100 on theneonandcloudflare-r2legs (exit 124). That cap is user-defined, so it isn't a proper error — andtimeout(1)can never be made graceful from the inside anyway: it returns 124 regardless of how the process exits. The fix is to make the deadline a first-class runner concept instead of a shell wrapper.deadlineMsonBenchmarkConfig(or--deadline-mson the CLI) sets a wall-clock budget for task execution:groupBy: 'participant','round', and the local no-ingest path) stops launching new tasks; in-flight tasks finish normally.BenchmarkRunOutcome.stoppedEarlyflags the truncation.RunWorkerOptions.stopRequestedis a predicaterunWorker's task pool polls before each new start; the runner passes the deadline'sexpired()through.The workflow then swaps the kill for the knob:
timeout 75mstays as a hard backstop under the job's 90-minute cap — if the drain itself hangs, that's a real failure worth paging on.Verified:
--deadline-ms 1000on a 10-iteration local bench ran 4 tasks, printedstopped at deadline — partial results, exited 0. Changeset covers@benchsdk/runner,@benchsdk/worker,@benchsdk/api(patch).Link to Devin session: https://app.devin.ai/sessions/ce0748a72a10427db3c2930e20d3395f
Open in Devin Desktop: https://app.devin.ai/desktop/session/ce0748a72a10427db3c2930e20d3395f?variant=devin
Requested by: @HeyGarrison