Skip to content

Commit 1bfab30

Browse files
committed
fix(run-engine): bound the ck idle set by rank, not just by floor
The at-or-below-floor reap on :ckVtimeIdle is worth nothing while the floor is pinned, and a workload that keeps minting fresh concurrency keys pins it indefinitely: each new key registers at the floor and is served at it, so minServableTag never rises. A resource benchmark caught the set growing by the drain count every round and never shrinking, passing ckIndex in size by round 50 and reaching 12000 entries (1.77MB) over 60 rounds, with only the 24h state TTL bounding it. The mechanism was measured rather than inferred: a probe sampling the floor found it at 0 on every round while the lowest parked tag was 1, so ZREMRANGEBYSCORE could never match. Adds a rank cap, keeping the highest idleMaxEntries tags (default 10000, configurable), which does not depend on the floor moving. Trimming the lowest tags first drops the entries nearest the floor, whose remembered credit is worth least. Verified against an explicit cap of 3000: the set rises to it and stays flat there across 12000 drains with the floor still pinned at 0. The new ARGV is inserted before the metrics gauge arg, which has to stay last because the gauge fragment reads ARGV[#ARGV]. Also drops the node:test describe import from the new test file, which shadows vitest's own under globals:true. That is a wider pattern in this directory and is left alone elsewhere.
1 parent f1fd460 commit 1bfab30

2 files changed

Lines changed: 85 additions & 3 deletions

File tree

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,13 @@ export type RunQueueOptions = {
228228
scanWindowMultiplier?: number;
229229
/** EXPIRE applied to ckVtime/ckVtimeFloor on every write. Default 86400. */
230230
stateTtlSeconds?: number;
231+
/**
232+
* Hard cap on remembered tags in ckVtimeIdle, enforced by rank so it holds even
233+
* when the floor is pinned and the at-or-below-floor reap cannot fire. The lowest
234+
* tags are dropped first: they sit nearest the floor, so they are the entries whose
235+
* credit is worth least. Default 10000.
236+
*/
237+
idleMaxEntries?: number;
231238
};
232239
};
233240

@@ -317,6 +324,7 @@ export class RunQueue {
317324
readonly #ckVtimeQuantum: number;
318325
readonly #ckVtimeWindowMultiplier: number;
319326
readonly #ckVtimeStateTtl: number;
327+
readonly #ckVtimeIdleMaxEntries: number;
320328

321329
constructor(public readonly options: RunQueueOptions) {
322330
this.shardCount = options.shardCount ?? 2;
@@ -335,6 +343,10 @@ export class RunQueue {
335343
1,
336344
Math.floor(options.ckVirtualTimeScheduling?.stateTtlSeconds ?? 86400)
337345
);
346+
this.#ckVtimeIdleMaxEntries = Math.max(
347+
1,
348+
Math.floor(options.ckVirtualTimeScheduling?.idleMaxEntries ?? 10000)
349+
);
338350
this.retryOptions = options.retryOptions ?? defaultRetrySettings;
339351
this.redis = createRedisClient(options.redis, {
340352
onError: (error) => {
@@ -2647,6 +2659,7 @@ export class RunQueue {
26472659
String(this.#ckVtimeQuantum),
26482660
String(this.#ckVtimeWindowMultiplier),
26492661
String(this.#ckVtimeStateTtl),
2662+
String(this.#ckVtimeIdleMaxEntries),
26502663
// Must stay last: the gauge fragment reads ARGV[#ARGV].
26512664
metricsGaugeArg
26522665
)
@@ -5330,6 +5343,7 @@ local maxCount = tonumber(ARGV[6] or '1')
53305343
local quantum = tonumber(ARGV[7] or '1')
53315344
local windowMultiplier = tonumber(ARGV[8] or '3')
53325345
local stateTtl = tonumber(ARGV[9] or '86400')
5346+
local idleMaxEntries = tonumber(ARGV[10] or '10000')
53335347
${QUEUE_METRICS_GAUGE_PRELUDE}
53345348
${QUEUE_METRICS_CK_DEQUEUE_GAUGE_LUA}
53355349
@@ -5593,6 +5607,13 @@ if dequeuedCount > 0 then
55935607
-- NEW: an idle entry at or below the floor confers no credit, because registration takes
55945608
-- max(floor, idleTag). Dropping it bounds the idle set at one op per serving call.
55955609
redis.call('ZREMRANGEBYSCORE', ckVtimeIdleKey, '-inf', tostring(floor))
5610+
-- NEW: the reap above is worth nothing while the floor is pinned, which a workload that
5611+
-- keeps minting fresh concurrency keys does indefinitely (each one registers at the floor
5612+
-- and is served at it, so minServableTag never rises). Measured: the set grew by the drain
5613+
-- count every round and never shrank. Cap by rank as well, which does not depend on the
5614+
-- floor moving. Trimming the lowest tags first drops the entries nearest the floor, whose
5615+
-- remembered credit is worth least.
5616+
redis.call('ZREMRANGEBYRANK', ckVtimeIdleKey, 0, -(idleMaxEntries + 1))
55965617
if redis.call('EXISTS', ckVtimeKey) == 1 then
55975618
redis.call('EXPIRE', ckVtimeKey, stateTtl)
55985619
end
@@ -7376,6 +7397,7 @@ declare module "@internal/redis" {
73767397
quantum: string,
73777398
windowMultiplier: string,
73787399
stateTtlSeconds: string,
7400+
idleMaxEntries: string,
73797401
metricsEnabled: string,
73807402
callback?: Callback<[string[] | null, number[] | null]>
73817403
): Result<[string[] | null, number[] | null], Context>;

internal-packages/run-engine/src/run-queue/tests/ckVtimeStarvation.test.ts

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { redisTest } from "@internal/testcontainers";
22
import { trace } from "@internal/tracing";
33
import { Logger } from "@trigger.dev/core/logger";
44
import { Decimal } from "@trigger.dev/database";
5-
import { describe } from "node:test";
65
import { FairQueueSelectionStrategy } from "../fairQueueSelectionStrategy.js";
76
import { RunQueue } from "../index.js";
87
import { RunQueueFullKeyProducer } from "../keyProducer.js";
@@ -51,12 +50,19 @@ const baseEnv = {
5150

5251
const QUEUE = "task/my-task";
5352

54-
function createQueue(redisContainer: any, keyPrefix: string, vtimeEnabled: boolean) {
53+
function createQueue(
54+
redisContainer: any,
55+
keyPrefix: string,
56+
vtimeEnabled: boolean,
57+
idleMaxEntries?: number
58+
) {
5559
return new RunQueue({
5660
...testOptions,
5761
masterQueueConsumersDisabled: true,
5862
workerOptions: { disabled: true },
59-
...(vtimeEnabled ? { ckVirtualTimeScheduling: { enabled: true } } : {}),
63+
...(vtimeEnabled
64+
? { ckVirtualTimeScheduling: { enabled: true, ...(idleMaxEntries ? { idleMaxEntries } : {}) } }
65+
: {}),
6066
queueSelectionStrategy: new FairQueueSelectionStrategy({
6167
redis: { keyPrefix, host: redisContainer.getHost(), port: redisContainer.getPort() },
6268
keys: testOptions.keys,
@@ -386,4 +392,58 @@ describe("CK vtime starvation by drain-and-re-register", () => {
386392
expect(on.idleSize).toBe(0);
387393
}
388394
);
395+
396+
// The idle set is trimmed two ways. The at-or-below-floor reap is the cheap one, but it
397+
// is worth nothing while the floor is pinned, and a workload that keeps minting fresh
398+
// concurrency keys pins it indefinitely: each new key registers at the floor and is
399+
// served at it, so minServableTag never rises. A resource benchmark caught the set
400+
// growing by the drain count every round and never shrinking. The rank cap is the bound
401+
// that does not depend on the floor moving.
402+
redisTest(
403+
"the idle set stays capped when fresh keys keep the floor pinned",
404+
async ({ redisContainer }) => {
405+
const CAP = 25;
406+
const DRAINS = 400;
407+
const keyPrefix = `runqueue:test:idlecap:`;
408+
const queue = createQueue(redisContainer, keyPrefix, true, CAP);
409+
410+
try {
411+
const env = { ...baseEnv, maximumConcurrencyLimit: 20 };
412+
await queue.updateEnvConcurrencyLimits(env);
413+
const shard = testOptions.keys.masterQueueShardForEnvironment(env.id, 2);
414+
415+
// Every iteration uses a concurrency key never seen before, drains it, and never
416+
// brings it back: the worst case for a set that remembers drained variants.
417+
for (let i = 0; i < DRAINS; i++) {
418+
await queue.enqueueMessage({
419+
env,
420+
message: makeMessage({ runId: `f-${i}`, concurrencyKey: `fresh-${i}` }),
421+
workerQueue: env.id,
422+
skipDequeueProcessing: true,
423+
});
424+
const msgs = await queue.testDequeueFromMasterQueue(shard, env.id, 1);
425+
for (const m of msgs) {
426+
await queue.acknowledgeMessage(env.organization.id, m.messageId, {
427+
skipDequeueProcessing: true,
428+
});
429+
}
430+
}
431+
432+
const idleKey = testOptions.keys.ckVtimeIdleKeyFromQueue(variantName("fresh-0"));
433+
const idleSize = await queue.redis.zcard(idleKey);
434+
const floor = await queue.redis.get(
435+
testOptions.keys.ckVtimeFloorKeyFromQueue(variantName("fresh-0"))
436+
);
437+
438+
// The floor really is pinned, so the score reap cannot be what bounded this.
439+
expect(Number(floor ?? 0)).toBe(0);
440+
// One call can park past the cap before the next trim, hence the small margin.
441+
expect(idleSize).toBeLessThanOrEqual(CAP + 10);
442+
// And it is the cap doing the work, not an empty set.
443+
expect(idleSize).toBeGreaterThan(0);
444+
} finally {
445+
await queue.quit();
446+
}
447+
}
448+
);
389449
});

0 commit comments

Comments
 (0)