feat(sdk,core,webapp,run-engine): task concurrency option and shareable named limits - #4827
matt-aitken wants to merge 29 commits into
Conversation
🦋 Changeset detectedLatest commit: a0642e9 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 |
|
Important Review skippedThe saved review history does not include the base for the last reviewed commit. This saved history cannot establish the base for an incremental review. Comment You can disable this status message by setting the Use the checkbox below for a quick retry:
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (50)
🧰 Additional context used📓 Path-based instructions (8)**Import subpaths only** (never root).📄 CodeRabbit inference engine (AGENTS.md) Files:
**Prefer static imports over dynamic imports.**📄 CodeRabbit inference engine (AGENTS.md) Files:
Add crumbs as you write code — not just when debugging.📄 CodeRabbit inference engine (AGENTS.md) Files:
Use zod for validation in packages/core and apps/webapp📄 CodeRabbit inference engine (.github/copilot-instructions.md) Files:
Never import the root package (`@trigger.dev/core`).📄 CodeRabbit inference engine (packages/core/CLAUDE.md) Files:
Use function declarations instead of default exports📄 CodeRabbit inference engine (.github/copilot-instructions.md) Files:
Use types over interfaces for TypeScript Avoid using enums; prefer string unions or const objects instead📄 CodeRabbit inference engine (.github/copilot-instructions.md) Files:
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 Do not use high-cardinality attributes in OTEL metr...📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc) Files:
🔇 Additional comments (2)
WalkthroughThe change adds task-level inline and named concurrency limits with per-key and total caps. It replaces queue gate tuple configuration with explicit concurrency and trigger options. It persists and validates queue gates across task metadata, runs, queues, and replay flows. It adds concurrency metadata to worker manifests and resource catalogs. It also renames queue concurrency ceilings to Merge Risk: 🟡 Moderate · up to This change exposes queue gates and concurrency limits across task triggering and worker manifests, but unresolved SDK and metadata behaviors can cause requested concurrency controls to be ignored, truncated, or interpreted differently than configured. Resolve these issues before merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description gives a detailed and relevant summary of the concurrency changes, but it does not follow the repository template. It omits the issue-closing line, checklist, testing section, changelog section, and screenshots section. ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
03c5b34 to
d0d9e38
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
d0d9e38 to
512ff9c
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
@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: |
1300356 to
92dbd83
Compare
92dbd83 to
4e2b5a7
Compare
4e2b5a7 to
9af8f2e
Compare
9af8f2e to
d604cb3
Compare
8f2cf5f to
11d93a6
Compare
6b562dc to
9e6929d
Compare
9e6929d to
72773fe
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
f5e0b0f to
ad1fb33
Compare
Two definitions of the same queue that differ only in combinedConcurrencyLimit now warn and keep the first definition instead of letting registration order pick the deployed cap.
A bare string home (alone or as a tuple's first element) no longer registers queue metadata, so module evaluation order can no longer shadow a declared queue's limits with an empty definition.
Both gate parsers now drop gates with empty or over-length queue names, drop gates whose literal key exceeds the manifest cap, and treat an empty-string key as omitted so it inherits the run's key instead of pinning an empty pool.
The webapp and the enqueue system carried identical copies of the gate contract with nothing enforcing their parity. parseGates now lives in the run-engine package with unit tests covering every rule (bounds kept inclusive at 128, empty keys inherit, malformed entries dropped, capped at two), and both callers delegate to it.
Tasks declare concurrency directly: an inline { perKey, total } shape
caps the task, and concurrencyLimit() declares a named, shareable limit
that tasks hold via the same option (at most one inline plus two named).
Trigger calls switch a run's named limits with their own concurrency
option, strings only like queue. The queue tuple syntax and
combinedConcurrencyLimit never ship: queue() is a line again, its
concurrencyLimit deprecated in place, and the manifest carries the new
declarations for the server to compile.
Removes the combined override client methods the previous commit intended to drop, moves react-hooks trigger options off the queue tuple onto the concurrency names, merges a duplicated type import, and marks the new type-only imports as such for the linter.
Passing undefined for a gateless original fell back to the task's current gates, so old runs replayed after new limits were added would silently gain them.
The index workers emit them into WorkerManifest, which is a separate schema from the deploy metadata and also needs the field.
The option was silently dropped by the seven single-task and subscribe/wait paths and doubled at four by-id batch sites. A named limit object written inline in a task's concurrency now registers as a declaration instead of losing its caps, and trigger-time limit names must be non-empty strings.
Names are 1-122 characters of letters, numbers, underscores and hyphens, checked by the factory, task declarations and trigger options alike, so distinct names always stay distinct after queue name sanitization.
…d limits The end-to-end gate capacity is three (the anonymous inline-limit gate plus two named limits), so trigger-time replacement can no longer lose a named limit to a downstream cap. Limit shapes also start at 1: block everything is a pause or an override, not a declared limit, so a stored zero always reads as no limit.
Gate capacity is four end to end (three requested gates plus the task's anonymous inline-limit gate), so replaying a three-gate run against a task that later gained an inline limit resolves to four and still enqueues. A run stored without gates now replays with the task's currently declared limits instead of a fabricated empty array that silently cleared them, matching how replays adopt the current queue and retry config.
c2ebb50 to
ff350ef
Compare
Manage named concurrency limits at runtime: list and retrieve report each limit's bounds plus its live running and queued counts, override changes only the given bounds (total zero pauses the limit), and reset restores the declared values.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ting Matches the SDK's trigger validation, and the shared changeset now versions react-hooks alongside sdk and core.
…ntry type requires
matt-aitken
changed the title
The index step collected concurrencyLimits into the worker manifest, but both the dev session and the deploy index controller dropped the field when building the create-background-worker request, so declared named limits registered with no bounds and were never enforced.
|
Closing: this stack was flattened into a single change that has now been merged and will land in this repository via the automated sync. All review feedback from these PRs is incorporated, along with further review rounds on the flattened change. |
2 participants
Summary
Stacked on #4826. Adds the
concurrencyoption totask(): an inline shape caps the task itself, andconcurrencyLimit()declares a named limit any task can hold (up to two named limits per task, shared across every holder). A trigger call can switch a run's named limits with its ownconcurrencyoption; the task's inline limit always applies.perKeycaps eachconcurrencyKeypool (runs without a key share one pool) andtotalcaps every run holding the limit, keys or not. Queues stay pure ordering: the queue-levelconcurrencyLimitoption keeps working unchanged and is deprecated in favor ofconcurrency. Limit names use a queue-safe charset (letters, numbers, underscores, hyphens; 1-122 characters), enforced by the factory, at deploy time and at trigger time, so distinct names can never merge after sanitization.The runtime management surface (the
concurrencyLimitsnamespace: list, retrieve, override, reset) rides this contract; its server side lands higher in the stack.Design
The SDK normalizes
concurrencyinto the task manifest (an inline shape plus named-limit references); the deploy compiler materializes named limits as dedicated queue rows and compiles inline limits into the task's own queue or an anonymous limit, so the trigger path keeps its current query count. Trigger-time names resolve server-side into the run's gate list, which is stored on the run (replays reuse it) and enforced by the engine mechanism from #4826. A run holds at most four gates end to end: three requested plus the task's anonymous inline-limit gate, so replays survive a task gaining an inline limit after the run was created.Also renames the public queue option from
totalConcurrencyLimittocombinedConcurrencyLimit(introduced lower in this stack, renamed here before anything ships). Engine internals and storage keep their existing names.