Skip to content

Commit d9ad085

Browse files
committed
fix(run-engine,webapp,clickhouse): review fixes for the metrics tier
The plain dequeue gauge re-samples after admissions like the keyed one, the repair path clears a keyed run's variant and group slots by concurrency key, pause responses include the combined limit, stale wording and a leftover changeset from before the rename are cleaned up, and two empty flag blocks are removed from the fast-path scripts.
1 parent f391197 commit d9ad085

5 files changed

Lines changed: 32 additions & 14 deletions

File tree

apps/webapp/app/v3/querySchemas.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1426,7 +1426,7 @@ const queueMetricsByKeySchema: TableSchema = {
14261426
name: "max_limit",
14271427
...column("UInt32", {
14281428
description:
1429-
"The queue concurrency limit that applied to this key in the bucket. Aggregate with max().",
1429+
"The queue concurrency limit that applied to this key in the bucket (1000000 = no explicit limit). Aggregate with max().",
14301430
fillMode: "carry",
14311431
}),
14321432
},

apps/webapp/app/v3/services/pauseQueue.server.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,9 @@ export class PauseQueueService extends BaseService {
9292
concurrencyLimitOverriddenAt: updatedQueue.concurrencyLimitOverriddenAt ?? null,
9393
concurrencyLimitOverriddenBy: queue.concurrencyLimitOverriddenBy ?? null,
9494
paused: updatedQueue.paused,
95+
totalConcurrencyLimit: updatedQueue.totalConcurrencyLimit ?? null,
96+
totalConcurrencyLimitBase: updatedQueue.totalConcurrencyLimitBase ?? null,
97+
totalConcurrencyLimitOverriddenAt: updatedQueue.totalConcurrencyLimitOverriddenAt ?? null,
9598
}),
9699
};
97100
} catch (error) {

internal-packages/clickhouse/schema/042_add_queue_metrics_combined_concurrency.sql

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@
33
-- Total-concurrency gauges: combined_running is the in-flight count across ALL
44
-- concurrency-key variants of a queue (the groupConcurrency set), combined_limit the
55
-- RAW stored total cap (0 = none, readers clamp against max_env_limit). Emitted on
6-
-- base-queue gauge rows only. Per-key gauge rows now carry the EFFECTIVE per-key
7-
-- limit in queue_limit, surfaced in the ck tier as max_limit.
6+
-- base-queue gauge rows only. Per-key gauge rows carry the queue concurrency
7+
-- limit that applied in queue_limit, surfaced in the ck tier as max_limit
8+
-- (1000000 = no explicit limit).
89

910
ALTER TABLE trigger_dev.queue_metrics_raw_v1
1011
ADD COLUMN IF NOT EXISTS combined_running UInt32 DEFAULT 0,

internal-packages/run-engine/src/engine/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2993,6 +2993,7 @@ export class RunEngine {
29932993
{
29942994
select: {
29952995
queue: true,
2996+
concurrencyKey: true,
29962997
},
29972998
},
29982999
this.prisma
@@ -3014,6 +3015,7 @@ export class RunEngine {
30143015
runId,
30153016
orgId: latestSnapshot.organizationId,
30163017
queue: taskRun.queue,
3018+
concurrencyKey: taskRun.concurrencyKey ?? undefined,
30173019
env: {
30183020
id: latestSnapshot.environmentId,
30193021
type: latestSnapshot.environmentType,

internal-packages/run-engine/src/run-queue/index.ts

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,6 @@ export type RunQueueOptions = {
331331
* the total cap covering releases from builds without the mirror.
332332
*/
333333
gatesEnabled?: boolean;
334-
/** Cap on per-concurrency-key limit overrides stored per queue. Default 1000. */
335334
workerOptions?: {
336335
pollIntervalMs?: number;
337336
immediatePollIntervalMs?: number;
@@ -1537,6 +1536,7 @@ export class RunQueue {
15371536
runId: string;
15381537
orgId: string;
15391538
queue: string;
1539+
concurrencyKey?: string;
15401540
env: RunQueueKeyProducerEnvironment;
15411541
}) {
15421542
return this.#callClearMessageFromConcurrencySets(params);
@@ -3068,18 +3068,30 @@ export class RunQueue {
30683068
runId,
30693069
orgId,
30703070
queue,
3071+
concurrencyKey,
30713072
env,
30723073
}: {
30733074
runId: string;
30743075
orgId: string;
30753076
queue: string;
3077+
concurrencyKey?: string;
30763078
env: RunQueueKeyProducerEnvironment;
30773079
}) {
30783080
const messageId = runId;
30793081
const messageKey = this.keys.messageKey(orgId, messageId);
3080-
const queueCurrentConcurrencyKey = this.keys.queueCurrentConcurrencyKey(env, queue);
3082+
/**
3083+
* Callers pass the bare TaskRun queue name plus its concurrencyKey; the run's
3084+
* slots live on the ck variant, and the tracked clear additionally mirrors the
3085+
* group set and counters that only keyed queues maintain.
3086+
*/
3087+
const fullQueue = concurrencyKey ? this.keys.queueKey(env, queue, concurrencyKey) : queue;
3088+
const queueCurrentConcurrencyKey = this.keys.queueCurrentConcurrencyKey(
3089+
env,
3090+
queue,
3091+
concurrencyKey
3092+
);
30813093
const envCurrentConcurrencyKey = this.keys.envCurrentConcurrencyKey(env);
3082-
const queueCurrentDequeuedKey = this.keys.queueCurrentDequeuedKey(env, queue);
3094+
const queueCurrentDequeuedKey = this.keys.queueCurrentDequeuedKey(env, queue, concurrencyKey);
30833095
const envCurrentDequeuedKey = this.keys.envCurrentDequeuedKey(env);
30843096

30853097
this.logger.debug("Calling clearMessageFromConcurrencySets", {
@@ -3094,15 +3106,15 @@ export class RunQueue {
30943106
service: this.name,
30953107
});
30963108

3097-
if (queue.includes(":ck:")) {
3109+
if (fullQueue.includes(":ck:")) {
30983110
return this.redis.clearMessageFromConcurrencySetsTracked(
30993111
queueCurrentConcurrencyKey,
31003112
envCurrentConcurrencyKey,
31013113
queueCurrentDequeuedKey,
31023114
envCurrentDequeuedKey,
3103-
this.keys.queueRunningCounterKeyFromQueue(queue),
3104-
this.keys.ckIndexKeyFromQueue(queue),
3105-
this.keys.queueGroupConcurrencyKeyFromQueue(queue),
3115+
this.keys.queueRunningCounterKeyFromQueue(fullQueue),
3116+
this.keys.ckIndexKeyFromQueue(fullQueue),
3117+
this.keys.queueGroupConcurrencyKeyFromQueue(fullQueue),
31063118
messageKey,
31073119
messageId,
31083120
this.options.redis.keyPrefix ?? "",
@@ -4127,8 +4139,6 @@ if enableFastPath == '1' then
41274139
tonumber(redis.call('GET', queueConcurrencyLimitKey) or '1000000'),
41284140
envLimit
41294141
)
4130-
if totalConcurrencyEnabled then
4131-
end
41324142
41334143
if queueCurrent < queueLimit then
41344144
-- Total-cap gate: a fast-path admit consumes a group slot, so it must
@@ -4302,8 +4312,6 @@ if enableFastPath == '1' then
43024312
tonumber(redis.call('GET', queueConcurrencyLimitKey) or '1000000'),
43034313
envLimit
43044314
)
4305-
if totalConcurrencyEnabled then
4306-
end
43074315
43084316
if queueCurrent < queueLimit then
43094317
-- Total-cap gate: see enqueueMessageCkTracked.
@@ -4769,6 +4777,10 @@ else
47694777
redis.call('ZADD', masterQueueKey, earliestMessage[2], queueName)
47704778
end
47714779
4780+
-- Re-sample the gauge so the emitted snapshot includes this batch's admissions;
4781+
-- the top-of-script sample only covers the early returns where nothing was admitted.
4782+
${QUEUE_METRICS_GAUGE_LUA}
4783+
47724784
-- Return results as a flat array: [messageId1, messageScore1, messagePayload1, messageId2, messageScore2, messagePayload2, ...]
47734785
return __qmret(results)
47744786
`,

0 commit comments

Comments
 (0)