feat(webapp,run-engine,core,clickhouse): surface combined concurrency in metrics and dashboard - #4830
Conversation
|
|
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:
WalkthroughThe change adds total concurrency fields to queue metrics, ClickHouse aggregates, queue presenters, API responses, and queue views. RunQueue now exposes total concurrency counts and limits, emits total-cap gauge values, and removes per-key limit override handling. Queue and dashboard queries use the Merge Risk: 🟡 Moderate · up to This change adds combined queue concurrency visibility, but the metrics migration can lose aggregate data during view replacement and queue-list pagination can return incomplete pages when ranked entries are stale. These issues should be addressed before merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
![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.
8f5ad5d to
c60df90
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: |
c60df90 to
2816d30
Compare
fe6772f to
6bc8bef
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
e802dab to
1627de4
Compare
matt-aitken
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
1627de4 to
f4b8109
Compare
f4b8109 to
ecfbdbb
Compare
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.
Observability mapAs of 19/100 over 458 measured of 476 entry points (base 19, no change) What this PR changed
3 entries removed FIX FIRST
AUDIT 3 of 50 sensitive mutations record an actor. 47 without one. What the score is made ofThe score and findings here are report-only and never gate the merge. Separately, a required test suite keeps this tool's symbol and route lists in sync with the code they name, and can fail a pull request that renames or removes a symbol they reference, or that adds the first route with a segment they anticipate. Each failure names the list to edit. The rules and their reasons: internal-packages/observability-map/README.md. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…g in the UI The sampled guard was queried but dropped when rows became chart points, so the back-fill guard always read zero and pre-cap history was still overwritten; the guard column is now copied onto each point. User-visible strings move from Combined to Total, matching the perKey/total vocabulary.
The env-level concurrency management page moves from /concurrency to /concurrency-limits, freeing the /concurrency URL for the upcoming queue concurrency page. Nothing outside the dashboard links to the old URL. chore(webapp): drop the old concurrency route left by the page rename
…ency-limits rename
…points Queue-level total limits are not part of the public API surface; managing totals arrives with the concurrency limits API instead.
The remaining gauge plumbing for the base scripts rides at this level because the shared total-limit memo was introduced here.
…eyless runs Keyless admits join the per-base-queue group set and are gated on the env-clamped total limit across the base enqueue fast path, the base dequeue and the queue mover, with the bounded reconcile at saturation and the release-side group mirror, matching the keyed tracked paths. Base-queue gauges carry the total running/limit fields.
…runs Keyless admits now join the same per-base-queue group set as keyed admits and are gated on the total limit, so a queue's total truly caps everything in flight together. The base enqueue fast path, the base dequeue and the queue mover all check the env-clamped total (with the bounded reconcile at saturation) and mirror the group membership on release, matching what the keyed tracked paths already did. Base-queue gauge snapshots now carry the total running/limit fields too.
The plain acknowledge, nack, dead-letter and release-concurrency scripts now mirror the group SREM off the base concurrency removal, exactly like their tracked keyed variants, so keyless holders admitted into the total pool always release their slot instead of lingering until a saturation reconcile prunes them.
clearMessageFromConcurrencySets now mirrors the group SREM like every other release path, so a stuck keyless run cleared by the repair sweep frees its total-concurrency slot instead of phantom-holding it until a saturation reconcile. The plain gauge test pins the new tail: zeroed CK fields plus the total pair.
…the environment The bare queue name is not a full queue key, so the FromQueue producer built a wrong group key for keyless repairs; the env-based producer matches the key the admit paths populate.
…et for keyless queues The defensive currentConcurrency removal in the TTL expiry script now mirrors into the base groupConcurrency set whether or not the queue is keyed, closing the last live release path where the mirror was conditional on the queue shape instead of the removal itself.
Every gate queue keeps a counter of runs that are queued and must clear it to execute: incremented per gate when a run enters a queue zset, decremented when it leaves (admit, ack while queued, nack re-queue, TTL expiry, dead-letter), always guarded on the zset transition so re-enqueues and already-removed members never double count, and floored at zero. Read via gateQueuedCountOfQueue and the batched variant; this backs the queued field on the upcoming concurrency limits API.
… a 24h TTL Gates sharing a base within one run count once, and each counter carries an absolute 24h TTL set at creation so drift from paths without the delta (a rolling deploy, a stale-entry cleanup) clears within a day: the reset counter converges back to exact through the zero floor as the pre-reset backlog drains.
A total-only limit no longer copies the total into the per-key column; the group set caps keyless runs, so the copy was redundant and made the row unable to say whether a per-key bound was declared. Reads of limit rows are now faithful: perKey and total columns hold exactly what the user declared.
Adds list, retrieve, override and reset endpoints for named concurrency limits. Limits resolve by name against the LIMIT-role rows; running is the limit's group cardinality (keyed and keyless holders together) and queued is the exact per-gate counter of runs that must clear the limit to execute. Overrides change only the given bounds, zero blocks every holder (how a limit is paused), and reset restores the declared values. Both bounds sync to the engine on every change.
…tations, activity-refreshed counter TTL Limit resets sync the engine to the declared base before clearing the markers, so an engine failure leaves the override intact and a retry converges. Both limit mutations carry the read markers in their where clause and surface a conflict instead of clobbering a concurrent change. The gate queued counter's TTL now refreshes on every delta, so an active gate's count never resets while drift from delta-less paths still clears once the gate goes quiet; the flag comment documents that total bounds enforce solely through the total-concurrency flag.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Covers partial overrides and base preservation, the zero-total pause, reset round trips, the enforce-first ordering under an engine failure (marker survives, retry converges), the optimistic conflict when the markers move underneath a mutation, and role/name resolution misses.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The optimistic guard now carries the row's updatedAt, so a deploy refreshing declared values (which deliberately keeps the markers) conflicts a concurrent mutation instead of losing its write. A reset whose persist fails after the engine already reverted re-syncs the engine from a fresh read, so a concurrent actor's override or pause is enforced again. Override bounds are validated against the environment maximum like queue overrides. Counter reads refresh the TTL so an observed gate never re-anchors while idle, and the routes drop a redundant decode that turned malformed names into 500s.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…e row The compensating engine re-sync now re-reads and re-syncs until the row's updatedAt stops moving (bounded), the same convergence the deploy sync uses, so a mutation landing mid-compensation is the last engine write instead of being overwritten by a stale snapshot.
…compensates The compensating re-sync preserves the caller's error (a conflict stays a 409 even when the compensation itself flakes) and also runs when the enforce-first engine sync partially fails, so a half-reverted engine is re-synced to the standing override. The optimistic guard carries the override markers alongside updatedAt, narrowing the same-millisecond window to writes that leave both untouched.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Both bounds' engine writes now settle before a sync step fails, so no write is still in flight when the compensating re-sync runs; a late sibling can never land after the compensation and leave one bound stale.
Resetting a limit with no override fails before any engine write, so the compensating re-sync is skipped for that error instead of issuing spurious reads and engine writes on a pure validation path.
1 participant
Summary
Queues with a
combinedConcurrencyLimitnow show how they use it, in the dashboard and the API. Stacked on #4829.On the queues list, a queue with a total cap shows it bracketed in the Limit column (e.g. 1 (10)) with the Running count turning warning-colored at the cap. The queue detail page gains a "Total concurrency" chart plotting total running against the effective cap over time. Queue retrieve and list API responses include the same totals under
concurrency.combined.Design
The queue metrics gauge (emitted from the dequeue and enqueue Lua scripts) is extended with two fields: total running (the group concurrency set's cardinality) and the raw stored total limit. They flow through the existing metrics stream into the ClickHouse raw table and are aggregated as
max_total_running/max_total_limitin the 10s and 5m rollup tiers, queryable from the dashboard's TRQL widgets. The per-key rollup additionally records the queue limit that applied to each key.Reading the live numbers is O(pages shown): the list presenter batches one SCARD per queue that has a cap.
Also in this PR
The self-serve concurrency management page moves from
/concurrencyto/concurrency-limits(retitled "Concurrency limits"), freeing the/concurrencyURL for the renamed queue concurrency page that lands here before merge. The combined-limit override and reset endpoints (POST /api/v1/queues/:queue/concurrency/combined/overrideand/reset) are removed: queue-level total limits are not part of the public API surface, and managing totals arrives with the concurrency limits API instead.