Conversation
… slots Rate-limited tasks keep their GROUP_ROUND_ROBIN slot filled while parked in v1_rate_limited_queue_items. Two gaps let those slots deadlock a queue permanently: the queuer only requeued parked items when it had already seen rate limits in-process, so a scheduler restart never requeued them; and the schedule-timeout pass only deleted unfilled slots, so the filled zombies never released. Always requeue parked items, and time out any slot past schedule_timeout_at that has no v1_task_runtime row (i.e. is not actually running).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThe PR prevents rate-limited tasks from permanently occupying group round-robin concurrency slots after scheduler restarts.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Reviews (2): Last reviewed commit: "test(scheduler): drop duplicate queuer r..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
After removing the hasRateLimits gate, the "already observed" test exercised the same path as the fresh-process case.
juliusgeo
reviewed
Aug 26, 2026
| checkpoint := start | ||
| var err error | ||
|
|
||
| if q.hasRateLimits { |
Contributor
There was a problem hiding this comment.
why are we removing this conditional?
| strategy_id = @strategyId::bigint AND | ||
| schedule_timeout_at < NOW() AND | ||
| is_filled = FALSE | ||
| -- Filled slots are only running if they have a v1_task_runtime row. |
Contributor
There was a problem hiding this comment.
slightly confused by the second line of this comment--the fix here is making it so that is_filled is predicated on whether a v1_task_runtime row exists, otherwise the slot isn't filled? Isn't this logic an inversion of that, where it will only be set to is_filled if there isnt a runtime row?
Contributor
Benchmark resultsCompared against |
|
We put together a standalone reproduction for both defects covered by this PR: hatchet-rate-limit-repro. The same tests run against the pinned PR base and this PR’s proposed fix:
It uses the real scheduler and PostgreSQL repositories. To make the trigger deterministic, it seeds the parked-task state directly and constructs a fresh scheduling pool; it does not submit through an SDK or kill a full engine process. The recovery test checks that the queuer completed assignment passes, then confirms a direct repository call can recover the stuck row. The timeout test checks cancellation of the expired holder and queuing of the follow-up. Run This gives us a repeatable check for both halves of the fix. Hope it helps with review and verification. |
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
3 participants
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
Tasks that hit a rate limit while holding a filled
GROUP_ROUND_ROBINconcurrency slot could deadlock their queue permanently: parked queue items were only requeued when the queuer had already observed rate limits in-process, so after a scheduler restart they never came back, and the schedule-timeout pass only deleted unfilled slots, so the filled slots never released. New runs then waited on the full group until they hit their scheduling timeout, indefinitely. Observed on a production tenant.Type of change
What's Changed
RequeueRateLimitedItemseach loop (removed the in-memoryhasRateLimitsgate), so items parked inv1_rate_limited_queue_itemsare requeued even by a fresh queuer after a restart or lease changeRunGroupRoundRobin/RunChildGroupRoundRobinschedule-timeout now cancels any slot pastschedule_timeout_atwith nov1_task_runtimerow (not actually running), instead of only unfilled slotsChecklist
Changes have been:
Testing
go test ./pkg/scheduling/v1/ -run TestQueuerRequeuesRateLimitedItemsgo test -tags integration ./pkg/scheduling/v1/(full suite, all 22 tests pass; the new test and unit tests fail onmain)🤖 AI Disclosure
I acknowledge that an LLM was used in the creation of this Pull Request, in accordance with Hatchet's AI_POLICY.md.
Details: Root-cause analysis, fix, and tests written with Cursor (Claude), reviewed by a human.