feat(run-engine,sdk,webapp): queue total concurrency limit across keys - #4823
feat(run-engine,sdk,webapp): queue total concurrency limit across keys#4823matt-aitken wants to merge 5 commits into
Conversation
🦋 Changeset detectedLatest commit: 6290352 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an optional Merge Risk: 🟡 Moderate · up to The change adds a gated queue-wide concurrency cap, but saturated queues may currently perform large repeated cleanup operations that delay processing, while rollout or missed cleanup can strand capacity and prevent runs from starting; enabling the cap with existing runs can also temporarily exceed it. The feature is disabled by default, but these bounded availability and rollout risks require explicit owner follow-up before broad enablement. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description provides a detailed and relevant summary of the feature and design, but it omits the template sections for issue closure, checklist completion, testing steps, changelog, and screenshots. ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2d1f985 to
b895c82
Compare
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
On a queue used with concurrencyKey, concurrencyLimit applies to each key value independently, so nothing bounds the queue as a whole short of the environment limit. The new totalConcurrencyLimit queue option caps in-flight runs across all keys of the queue while each key still gets at most concurrencyLimit. Enforcement lives in the concurrency-key dequeue and enqueue fast-path scripts, gated behind RUN_ENGINE_TOTAL_CONCURRENCY_LIMITS_ENABLED (default off). A per-base-queue groupConcurrency set tracks total in-flight; every release path mirrors its per-key removal into that set unconditionally so the set stays correct across flag toggles.
Strengthen the nack test so it proves the group slot is released (a second key's run must be admitted after the nack), document the enable-time convergence window on the flag, and correct the totalConcurrencyOfQueue doc to describe the drain-on-disable behavior.
468323d to
023c282
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal-packages/run-engine/src/run-queue/index.ts (1)
3827-3865: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd crumbs for aggregate-concurrency state changes.
Add
//@Crumbsmarkers or `// `#region` `@crumbsblocks around the new total-cap admission and group-set release paths. This includes the tracked CK enqueue, dequeue, acknowledgment, nack, dead-letter, release, and clear commands.As per coding guidelines, “Add crumbs as you write code — not just when debugging.”
Also applies to: 4639-4661, 5249-5725
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e99334b8-fb48-462b-a79f-b9683eac6e60
📒 Files selected for processing (1)
internal-packages/run-engine/src/run-queue/index.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: internal / 🧪 Unit Tests: Internal
🧰 Additional context used
📓 Path-based instructions (5)
**Prefer static imports over dynamic imports.** Only use dynamic `import()` when:
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal-packages/run-engine/src/run-queue/index.ts
Add crumbs as you write code — not just when debugging. Mark lines with
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal-packages/run-engine/src/run-queue/index.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
internal-packages/run-engine/src/run-queue/index.ts
Use types over interfaces for TypeScript
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
internal-packages/run-engine/src/run-queue/index.ts
When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
internal-packages/run-engine/src/run-queue/index.ts
A release path that misses the group-set mirror (an instance on an older build during rollout, or a future release script) would otherwise leave a member the gate counts forever. Every terminal release deletes the run's message key, so when a queue sits at its total the dequeue gate prunes members whose message key no longer exists, throttled to one pass per interval per queue. Members of re-queued runs keep their message key and clear through the mirrored ack when the run completes.
…otal limit The waitFor polls dequeued with the default 10s blocking pop, which could blow past the helper deadline on a slow runner; poll non-blocking instead. Document that totalConcurrencyLimit: 0 holds every keyed run, matching concurrencyLimit's zero semantics.
…er pass Reconciling a saturated queue's group set with SMEMBERS plus one EXISTS per member runs the whole traversal inside a single Lua call, which blocks Redis for the duration on a large set. Scan one bounded batch per pass instead, persisting the SSCAN cursor between passes so successive intervals cover the whole set. Covered by a test that drains a 1,200-member leaked backlog.
Summary
Adds a
totalConcurrencyLimitoption to queues. On a queue used withconcurrencyKey,concurrencyLimitapplies to each key value independently, so ten active keys with a limit of 5 can run 50 at once and nothing bounds the queue as a whole short of the environment limit.totalConcurrencyLimitcaps in-flight runs across all keys while each key still gets at mostconcurrencyLimit:Enforcement is gated behind
RUN_ENGINE_TOTAL_CONCURRENCY_LIMITS_ENABLED(default off) and applies to runs triggered with aconcurrencyKey. With the gate off, admit paths are unchanged.Design
The engine keeps a per-base-queue
groupConcurrencyset shared by every concurrency-key variant; its cardinality is the queue's total in-flight count. The concurrency-key dequeue script bounds each batch bymin(totalLimit, envLimit) - SCARD(group)and adds admitted runs to the set. The enqueue fast path checks the same gate and falls back to a normal enqueue when the queue is at its total.Every release path (ack, nack, dead-letter, concurrency release, TTL expiry, sweeper clear) removes a run from the group set whenever it removes it from the per-key
currentConcurrencyset. Those removals run regardless of the gate, so the set stays correct if the gate is later turned off, and the existing reconciliation sweep self-heals the group set because it is always a subset of the per-key sets it acks against.The stored limit is the raw declared value; readers clamp to the environment concurrency limit. The option flows through the queue manifest into a new nullable
TaskQueue.totalConcurrencyLimitcolumn (additive migration) and syncs to the engine on deploy.The group set self-heals against release paths that miss the removal (an instance on an older build during a rollout, or a future release script such as the one #4398 adds). Every terminal release deletes the run's message key, so when a queue sits at its total the dequeue gate prunes members whose message key no longer exists, throttled to one pass per interval per queue. Runs in flight before the gate is enabled are the opposite case: they are absent from the set, so a queue can transiently exceed its total by at most that count, converging as each one completes.
Not included here, planned as follow-ups: a runtime override API for the total limit, dashboard and metrics surfacing, and skipping queues at their total during fair-queue selection.