Skip to content

Commit 48b3bcd

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 a556513 commit 48b3bcd

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
@@ -3001,6 +3001,7 @@ export class RunEngine {
30013001
{
30023002
select: {
30033003
queue: true,
3004+
concurrencyKey: true,
30043005
},
30053006
},
30063007
this.prisma
@@ -3022,6 +3023,7 @@ export class RunEngine {
30223023
runId,
30233024
orgId: latestSnapshot.organizationId,
30243025
queue: taskRun.queue,
3026+
concurrencyKey: taskRun.concurrencyKey ?? undefined,
30253027
env: {
30263028
id: latestSnapshot.environmentId,
30273029
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;
@@ -1549,6 +1548,7 @@ export class RunQueue {
15491548
runId: string;
15501549
orgId: string;
15511550
queue: string;
1551+
concurrencyKey?: string;
15521552
env: RunQueueKeyProducerEnvironment;
15531553
}) {
15541554
return this.#callClearMessageFromConcurrencySets(params);
@@ -3084,18 +3084,30 @@ export class RunQueue {
30843084
runId,
30853085
orgId,
30863086
queue,
3087+
concurrencyKey,
30873088
env,
30883089
}: {
30893090
runId: string;
30903091
orgId: string;
30913092
queue: string;
3093+
concurrencyKey?: string;
30923094
env: RunQueueKeyProducerEnvironment;
30933095
}) {
30943096
const messageId = runId;
30953097
const messageKey = this.keys.messageKey(orgId, messageId);
3096-
const queueCurrentConcurrencyKey = this.keys.queueCurrentConcurrencyKey(env, queue);
3098+
/**
3099+
* Callers pass the bare TaskRun queue name plus its concurrencyKey; the run's
3100+
* slots live on the ck variant, and the tracked clear additionally mirrors the
3101+
* group set and counters that only keyed queues maintain.
3102+
*/
3103+
const fullQueue = concurrencyKey ? this.keys.queueKey(env, queue, concurrencyKey) : queue;
3104+
const queueCurrentConcurrencyKey = this.keys.queueCurrentConcurrencyKey(
3105+
env,
3106+
queue,
3107+
concurrencyKey
3108+
);
30973109
const envCurrentConcurrencyKey = this.keys.envCurrentConcurrencyKey(env);
3098-
const queueCurrentDequeuedKey = this.keys.queueCurrentDequeuedKey(env, queue);
3110+
const queueCurrentDequeuedKey = this.keys.queueCurrentDequeuedKey(env, queue, concurrencyKey);
30993111
const envCurrentDequeuedKey = this.keys.envCurrentDequeuedKey(env);
31003112

31013113
this.logger.debug("Calling clearMessageFromConcurrencySets", {
@@ -3110,15 +3122,15 @@ export class RunQueue {
31103122
service: this.name,
31113123
});
31123124

3113-
if (queue.includes(":ck:")) {
3125+
if (fullQueue.includes(":ck:")) {
31143126
return this.redis.clearMessageFromConcurrencySetsTracked(
31153127
queueCurrentConcurrencyKey,
31163128
envCurrentConcurrencyKey,
31173129
queueCurrentDequeuedKey,
31183130
envCurrentDequeuedKey,
3119-
this.keys.queueRunningCounterKeyFromQueue(queue),
3120-
this.keys.ckIndexKeyFromQueue(queue),
3121-
this.keys.queueGroupConcurrencyKeyFromQueue(queue),
3131+
this.keys.queueRunningCounterKeyFromQueue(fullQueue),
3132+
this.keys.ckIndexKeyFromQueue(fullQueue),
3133+
this.keys.queueGroupConcurrencyKeyFromQueue(fullQueue),
31223134
messageKey,
31233135
messageId,
31243136
this.options.redis.keyPrefix ?? "",
@@ -4143,8 +4155,6 @@ if enableFastPath == '1' then
41434155
tonumber(redis.call('GET', queueConcurrencyLimitKey) or '1000000'),
41444156
envLimit
41454157
)
4146-
if totalConcurrencyEnabled then
4147-
end
41484158
41494159
if queueCurrent < queueLimit then
41504160
-- Total-cap gate: a fast-path admit consumes a group slot, so it must
@@ -4318,8 +4328,6 @@ if enableFastPath == '1' then
43184328
tonumber(redis.call('GET', queueConcurrencyLimitKey) or '1000000'),
43194329
envLimit
43204330
)
4321-
if totalConcurrencyEnabled then
4322-
end
43234331
43244332
if queueCurrent < queueLimit then
43254333
-- Total-cap gate: see enqueueMessageCkTracked.
@@ -4785,6 +4793,10 @@ else
47854793
redis.call('ZADD', masterQueueKey, earliestMessage[2], queueName)
47864794
end
47874795
4796+
-- Re-sample the gauge so the emitted snapshot includes this batch's admissions;
4797+
-- the top-of-script sample only covers the early returns where nothing was admitted.
4798+
${QUEUE_METRICS_GAUGE_LUA}
4799+
47884800
-- Return results as a flat array: [messageId1, messageScore1, messagePayload1, messageId2, messageScore2, messagePayload2, ...]
47894801
return __qmret(results)
47904802
`,

0 commit comments

Comments
 (0)