Skip to content

Commit fc4c747

Browse files
committed
test(run-engine): pin the ck vtime credit round trip and TTL registration
A second mutation audit covered the paths the first one listed as untouched: the enqueue and nack registration, the ack, dead-letter and TTL-expiry drains, and the TypeScript plumbing between them. 14 of 25 mutations were caught, and the 11 green ones collapse into three real holes. The idle park was checked on the dequeue path and nowhere else, so ack, dead-letter and TTL expiry could all stop parking a drained variant's tag, and nack could stop restoring it, with every suite still green. Each test here gives the variant credit first, drains it by one route, then asserts the tag reaches the idle set and comes back on the next enqueue. The dead-letter test also covers its caller. The old one drove the Lua directly, so forcing that branch to the untracked command was green while the same damage done inside the script was caught. Reaching the drain through the public API needs the rollout shape, where a flag-off instance empties the variant queue without collecting it. The TTL enqueue command carries its own copy of the registration block and no test enqueued a run with a TTL under the flag, so all three of its behaviours were unverified. And no test configured a quantum, so the weight dimension had no coverage at all; this adds one at 0.3.
1 parent 7dd3126 commit fc4c747

2 files changed

Lines changed: 531 additions & 0 deletions

File tree

Lines changed: 331 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,331 @@
1+
import { redisTest } from "@internal/testcontainers";
2+
import { trace } from "@internal/tracing";
3+
import { Logger } from "@trigger.dev/core/logger";
4+
import { Decimal } from "@trigger.dev/database";
5+
import { FairQueueSelectionStrategy } from "../fairQueueSelectionStrategy.js";
6+
import { RunQueue } from "../index.js";
7+
import { RunQueueFullKeyProducer } from "../keyProducer.js";
8+
import type { InputPayload } from "../types.js";
9+
10+
// A draining variant parks its tag in :ckVtimeIdle before it leaves :ckVtime, so its next
11+
// enqueue re-registers with the credit it earned instead of at the floor. A mutation audit
12+
// found that park is checked on the dequeue path and nowhere else: removing it from ack,
13+
// dead-letter and TTL expiry left every vtime suite green, as did removing the restore
14+
// from nack's registration. Those four are what this file pins.
15+
//
16+
// Each test gives the variant credit first. Parking a tag that equals the floor proves
17+
// nothing, because the dequeue reaps everything at or below the floor on its next call.
18+
19+
const testOptions = {
20+
name: "rq",
21+
tracer: trace.getTracer("rq"),
22+
workers: 1,
23+
defaultEnvConcurrency: 25,
24+
logger: new Logger("RunQueue", "error"),
25+
retryOptions: {
26+
maxAttempts: 5,
27+
factor: 1.1,
28+
minTimeoutInMs: 100,
29+
maxTimeoutInMs: 1_000,
30+
randomize: true,
31+
},
32+
keys: new RunQueueFullKeyProducer(),
33+
};
34+
35+
const authenticatedEnvDev = {
36+
id: "e1234",
37+
type: "DEVELOPMENT" as const,
38+
maximumConcurrencyLimit: 10,
39+
concurrencyLimitBurstFactor: new Decimal(2.0),
40+
project: { id: "p1234" },
41+
organization: { id: "o1234" },
42+
};
43+
44+
const QUEUE = "task/my-task";
45+
46+
function createQueue(
47+
redisContainer: any,
48+
opts: { vtime?: boolean; maxAttempts?: number } = {}
49+
): any {
50+
const redis = {
51+
keyPrefix: "runqueue:test:",
52+
host: redisContainer.getHost(),
53+
port: redisContainer.getPort(),
54+
};
55+
return new RunQueue({
56+
...testOptions,
57+
masterQueueConsumersDisabled: true,
58+
workerOptions: { disabled: true },
59+
...(opts.maxAttempts
60+
? { retryOptions: { ...testOptions.retryOptions, maxAttempts: opts.maxAttempts } }
61+
: {}),
62+
...(opts.vtime === false ? {} : { ckVirtualTimeScheduling: { enabled: true } }),
63+
queueSelectionStrategy: new FairQueueSelectionStrategy({ redis, keys: testOptions.keys }),
64+
redis,
65+
} as any) as any;
66+
}
67+
68+
function makeMessage(overrides: Partial<InputPayload> = {}): InputPayload {
69+
return {
70+
runId: "r1",
71+
taskIdentifier: QUEUE,
72+
orgId: "o1234",
73+
projectId: "p1234",
74+
environmentId: "e1234",
75+
environmentType: "DEVELOPMENT",
76+
queue: QUEUE,
77+
timestamp: Date.now(),
78+
attempt: 0,
79+
...overrides,
80+
};
81+
}
82+
83+
const variantName = (ck: string) => testOptions.keys.queueKey(authenticatedEnvDev, QUEUE, ck);
84+
const shardFor = () => testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2);
85+
86+
describe("CK vtime: a drained variant keeps its credit", () => {
87+
redisTest(
88+
"ack parks the credit, and the next enqueue gets it back",
89+
async ({ redisContainer }) => {
90+
const queue = createQueue(redisContainer);
91+
try {
92+
const t0 = Date.now() - 100_000;
93+
94+
// Two on heavy so serving one leaves the variant registered with credit, and one on
95+
// light to hold the floor at zero while heavy's tag climbs.
96+
for (let i = 0; i < 2; i++) {
97+
await queue.enqueueMessage({
98+
env: authenticatedEnvDev,
99+
message: makeMessage({ runId: `h${i}`, concurrencyKey: "heavy", timestamp: t0 + i }),
100+
workerQueue: authenticatedEnvDev.id,
101+
skipDequeueProcessing: true,
102+
});
103+
}
104+
await queue.enqueueMessage({
105+
env: authenticatedEnvDev,
106+
message: makeMessage({ runId: "l0", concurrencyKey: "light", timestamp: t0 + 2 }),
107+
workerQueue: authenticatedEnvDev.id,
108+
skipDequeueProcessing: true,
109+
});
110+
111+
const heavy = variantName("heavy");
112+
const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(heavy);
113+
const ckVtimeIdleKey = testOptions.keys.ckVtimeIdleKeyFromQueue(heavy);
114+
115+
// Both start at the floor, heavy sorts first on the lexical tie-break, so this
116+
// serves heavy and charges it one quantum.
117+
const served = await queue.testDequeueFromMasterQueue(
118+
shardFor(),
119+
authenticatedEnvDev.id,
120+
1
121+
);
122+
expect(served.map((m: any) => m.messageId)).toEqual(["h0"]);
123+
expect(await queue.redis.zscore(ckVtimeKey, heavy)).toBe("1");
124+
125+
// The in-flight one: heavy still has a queued message, so this is not a drain.
126+
await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, "h0", {
127+
skipDequeueProcessing: true,
128+
});
129+
expect(await queue.redis.zscore(ckVtimeKey, heavy)).toBe("1");
130+
131+
// The queued one: heavy empties, so the ack drains it.
132+
await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, "h1", {
133+
skipDequeueProcessing: true,
134+
});
135+
expect(await queue.redis.zscore(ckVtimeKey, heavy)).toBeNull();
136+
expect(await queue.redis.zscore(ckVtimeIdleKey, heavy)).toBe("1");
137+
138+
// Back with the credit it earned. Landing at the floor here would let a variant
139+
// reset its tag by draining and returning, which is the whole point of the park.
140+
await queue.enqueueMessage({
141+
env: authenticatedEnvDev,
142+
message: makeMessage({ runId: "h2", concurrencyKey: "heavy", timestamp: t0 + 3 }),
143+
workerQueue: authenticatedEnvDev.id,
144+
skipDequeueProcessing: true,
145+
});
146+
expect(await queue.redis.zscore(ckVtimeKey, heavy)).toBe("1");
147+
} finally {
148+
await queue.quit();
149+
}
150+
}
151+
);
152+
153+
redisTest(
154+
"nack restores the parked credit rather than re-registering at the floor",
155+
async ({ redisContainer }) => {
156+
const queue = createQueue(redisContainer);
157+
try {
158+
const t0 = Date.now() - 100_000;
159+
160+
await queue.enqueueMessage({
161+
env: authenticatedEnvDev,
162+
message: makeMessage({ runId: "h0", concurrencyKey: "heavy", timestamp: t0 }),
163+
workerQueue: authenticatedEnvDev.id,
164+
skipDequeueProcessing: true,
165+
});
166+
await queue.enqueueMessage({
167+
env: authenticatedEnvDev,
168+
message: makeMessage({ runId: "l0", concurrencyKey: "light", timestamp: t0 + 1 }),
169+
workerQueue: authenticatedEnvDev.id,
170+
skipDequeueProcessing: true,
171+
});
172+
173+
const heavy = variantName("heavy");
174+
const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(heavy);
175+
const ckVtimeIdleKey = testOptions.keys.ckVtimeIdleKeyFromQueue(heavy);
176+
177+
// Serving heavy's only message empties the variant, so the dequeue parks it at 1.
178+
const served = await queue.testDequeueFromMasterQueue(
179+
shardFor(),
180+
authenticatedEnvDev.id,
181+
1
182+
);
183+
expect(served.map((m: any) => m.messageId)).toEqual(["h0"]);
184+
expect(await queue.redis.zscore(ckVtimeKey, heavy)).toBeNull();
185+
expect(await queue.redis.zscore(ckVtimeIdleKey, heavy)).toBe("1");
186+
187+
await queue.nackMessage({
188+
orgId: authenticatedEnvDev.organization.id,
189+
messageId: "h0",
190+
incrementAttemptCount: false,
191+
skipDequeueProcessing: true,
192+
});
193+
194+
// Not the floor (0), and not one quantum behind the idle max (2) either: the exact
195+
// tag it left with.
196+
expect(await queue.redis.zscore(ckVtimeKey, heavy)).toBe("1");
197+
} finally {
198+
await queue.quit();
199+
}
200+
}
201+
);
202+
203+
redisTest(
204+
"the dead-letter path parks the credit of a variant left behind mid-deploy",
205+
async ({ redisContainer }) => {
206+
// The only route the public API has into the dead-letter drain is nacking past
207+
// maxAttempts, which acts on an in-flight message whose variant the vtime dequeue has
208+
// usually already collected. Reaching it with a live ckVtime entry needs the rollout
209+
// case: a flag-off instance empties the variant queue without touching ckVtime, so a
210+
// stranded entry is still there when the dead-letter script runs.
211+
//
212+
// Driving the script directly instead would pass whether or not the caller branches on
213+
// the flag, and whether or not it passes the right keys. Both were unverified until
214+
// this test: a mutation forcing that branch to the untracked command left the suite
215+
// green while the same damage done inside the script was caught.
216+
const on = createQueue(redisContainer, { maxAttempts: 1 });
217+
const off = createQueue(redisContainer, { vtime: false, maxAttempts: 1 });
218+
try {
219+
const t0 = Date.now() - 100_000;
220+
221+
for (let i = 0; i < 2; i++) {
222+
await on.enqueueMessage({
223+
env: authenticatedEnvDev,
224+
message: makeMessage({ runId: `h${i}`, concurrencyKey: "heavy", timestamp: t0 + i }),
225+
workerQueue: authenticatedEnvDev.id,
226+
skipDequeueProcessing: true,
227+
});
228+
}
229+
await on.enqueueMessage({
230+
env: authenticatedEnvDev,
231+
message: makeMessage({ runId: "l0", concurrencyKey: "light", timestamp: t0 + 5_000 }),
232+
workerQueue: authenticatedEnvDev.id,
233+
skipDequeueProcessing: true,
234+
});
235+
236+
const heavy = variantName("heavy");
237+
const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(heavy);
238+
const ckVtimeIdleKey = testOptions.keys.ckVtimeIdleKeyFromQueue(heavy);
239+
240+
await on.testDequeueFromMasterQueue(shardFor(), authenticatedEnvDev.id, 1);
241+
expect(await on.redis.zscore(ckVtimeKey, heavy)).toBe("1");
242+
await on.acknowledgeMessage(authenticatedEnvDev.organization.id, "h0", {
243+
skipDequeueProcessing: true,
244+
});
245+
246+
// The flag-off instance empties heavy without collecting it, which is what leaves the
247+
// entry stranded at its earned tag with an in-flight message to nack.
248+
const strandedServe = await off.testDequeueFromMasterQueue(
249+
shardFor(),
250+
authenticatedEnvDev.id,
251+
1
252+
);
253+
expect(strandedServe.map((m: any) => m.messageId)).toEqual(["h1"]);
254+
expect(await on.redis.zscore(ckVtimeKey, heavy)).toBe("1");
255+
256+
// maxAttempts is 1, so this goes straight to the dead-letter queue.
257+
await on.nackMessage({
258+
orgId: authenticatedEnvDev.organization.id,
259+
messageId: "h1",
260+
skipDequeueProcessing: true,
261+
});
262+
263+
expect(await on.redis.zscore(ckVtimeKey, heavy)).toBeNull();
264+
expect(await on.redis.zscore(ckVtimeIdleKey, heavy)).toBe("1");
265+
} finally {
266+
await off.quit();
267+
await on.quit();
268+
}
269+
}
270+
);
271+
272+
redisTest("TTL expiry parks the credit of the variant it sweeps", async ({ redisContainer }) => {
273+
// The sweep discovers its queues inside the script, so there is no public route and no
274+
// key argument to get wrong; driving the command is the only option. What is worth
275+
// pinning is that it parks before removing, same as the other two drains.
276+
const queue = createQueue(redisContainer);
277+
try {
278+
const t0 = Date.now() - 100_000;
279+
280+
for (let i = 0; i < 2; i++) {
281+
await queue.enqueueMessage({
282+
env: authenticatedEnvDev,
283+
message: makeMessage({ runId: `h${i}`, concurrencyKey: "heavy", timestamp: t0 + i }),
284+
workerQueue: authenticatedEnvDev.id,
285+
skipDequeueProcessing: true,
286+
});
287+
}
288+
await queue.enqueueMessage({
289+
env: authenticatedEnvDev,
290+
message: makeMessage({ runId: "l0", concurrencyKey: "light", timestamp: t0 + 5_000 }),
291+
workerQueue: authenticatedEnvDev.id,
292+
skipDequeueProcessing: true,
293+
});
294+
295+
const heavy = variantName("heavy");
296+
const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(heavy);
297+
const ckVtimeIdleKey = testOptions.keys.ckVtimeIdleKeyFromQueue(heavy);
298+
299+
await queue.testDequeueFromMasterQueue(shardFor(), authenticatedEnvDev.id, 1);
300+
await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, "h0", {
301+
skipDequeueProcessing: true,
302+
});
303+
expect(await queue.redis.zscore(ckVtimeKey, heavy)).toBe("1");
304+
305+
const shard = shardFor();
306+
const ttlQueueKey = testOptions.keys.ttlQueueKeyForShard(shard);
307+
await queue.redis.zadd(
308+
ttlQueueKey,
309+
Date.now() - 1000,
310+
`${heavy}|h1|${authenticatedEnvDev.organization.id}`
311+
);
312+
313+
await queue.redis.expireTtlRunsVtimeTracked(
314+
ttlQueueKey,
315+
"runqueue:test:",
316+
Date.now().toString(),
317+
"10",
318+
"2",
319+
"ttlworker",
320+
"ttlworkeritems",
321+
"30000",
322+
"86400"
323+
);
324+
325+
expect(await queue.redis.zscore(ckVtimeKey, heavy)).toBeNull();
326+
expect(await queue.redis.zscore(ckVtimeIdleKey, heavy)).toBe("1");
327+
} finally {
328+
await queue.quit();
329+
}
330+
});
331+
});

0 commit comments

Comments
 (0)