Skip to content

Commit 11e6fb8

Browse files
committed
fix(run-engine): stop counting the wait for a deployment as queue scheduling delay
The scheduling-delay anchor was inferred from includeTtl, the flag that decides whether to arm a run's TTL. That conflated two independent questions, and they disagree on one path: when a run held in PENDING_VERSION is promoted, the promotion is its first real entry into the queue, so arming TTL there is right, but the run was not runnable while it waited for a worker version and its queueTimestamp still holds the original trigger time. Anchoring there billed the entire wait-for-deployment period, minutes to hours, as queue scheduling delay, which then fed the delay charts and the health report's flow verdict. Eligibility anchoring is now its own opt-in, set by the two paths where a run genuinely was runnable from its queue position: the trigger and a delayed run coming due. The pending-version promotion anchors at the promotion instead. TTL behaviour is unchanged. Anchoring is kept separate from queueTimestamp on purpose: that field is also the queue ordering score, so moving it would cost a run its place in line to fix a metric.
1 parent 0183707 commit 11e6fb8

4 files changed

Lines changed: 39 additions & 9 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,6 +1169,7 @@ export class RunEngine {
11691169
tx: prisma,
11701170
skipRunLock: true,
11711171
includeTtl: true,
1172+
anchorEligibilityAtQueuePosition: true,
11721173
enableFastPath,
11731174
});
11741175
} catch (enqueueError) {

internal-packages/run-engine/src/engine/systems/delayedRunSystem.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ export class DelayedRunSystem {
164164
batchId: run.batchId ?? undefined,
165165
skipRunLock: true,
166166
includeTtl: true,
167+
anchorEligibilityAtQueuePosition: true,
167168
});
168169

169170
const queuedAt = new Date();

internal-packages/run-engine/src/engine/systems/enqueueSystem.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export class EnqueueSystem {
3737
runnerId,
3838
skipRunLock,
3939
includeTtl = false,
40+
anchorEligibilityAtQueuePosition = false,
4041
enableFastPath = false,
4142
store,
4243
}: {
@@ -58,8 +59,25 @@ export class EnqueueSystem {
5859
workerId?: string;
5960
runnerId?: string;
6061
skipRunLock?: boolean;
61-
/** When true, include TTL in the queued message (only for first enqueue from trigger). Default false. */
62+
/**
63+
* When true, arm the run's TTL on the queued message. Set by every path that is the run's
64+
* first real entry into the queue: trigger, a delayed run coming due, and the pending-version
65+
* promotion. Waitpoint and checkpoint re-enqueues must not re-arm it. Default false.
66+
*/
6267
includeTtl?: boolean;
68+
/**
69+
* When true, the scheduling-delay clock starts at the run's queue position (its
70+
* `queueTimestamp`, which for a delayed run is `delayUntil`) rather than now. Set only where
71+
* the run was genuinely eligible to execute from that moment: trigger and a delayed run
72+
* coming due.
73+
*
74+
* Deliberately separate from `includeTtl`, because the two disagree on the pending-version
75+
* promotion: that promotion is the run's first entry into the queue (so TTL arms there), but
76+
* the run was NOT runnable while it waited for a worker version, and its `queueTimestamp`
77+
* still holds the original trigger time. Anchoring there would bill the whole wait-for-deploy
78+
* period as queue scheduling delay. Default false, so a new call site has to opt in.
79+
*/
80+
anchorEligibilityAtQueuePosition?: boolean;
6381
/** When true, allow the queue to push directly to worker queue if concurrency is available. */
6482
enableFastPath?: boolean;
6583
/**
@@ -98,16 +116,10 @@ export class EnqueueSystem {
98116
// Force development runs to use the environment id as the worker queue.
99117
const workerQueue = env.type === "DEVELOPMENT" ? env.id : run.workerQueue;
100118

101-
// Ordering keeps the run's original position; the scheduling-delay anchor is the
102-
// trigger/delay time only on first enqueue (includeTtl). Re-enqueues anchor to now,
103-
// else the wait metric absorbs the whole waitpoint/checkpoint duration.
104119
const queuePositionMs = (run.queueTimestamp ?? run.createdAt).getTime();
105120
const timestamp = queuePositionMs - run.priorityMs;
106-
const eligibleAtMs = includeTtl ? queuePositionMs : Date.now();
121+
const eligibleAtMs = anchorEligibilityAtQueuePosition ? queuePositionMs : Date.now();
107122

108-
// Include TTL only when explicitly requested: the first enqueue from trigger, the
109-
// delayed-run system, and the pending-version promotion (each is a run's first real
110-
// entry into the queue). Waitpoint and checkpoint re-enqueues must not add TTL.
111123
let ttlExpiresAt: number | undefined;
112124
if (includeTtl && run.ttl) {
113125
const expireAt = parseNaturalLanguageDuration(run.ttl);

internal-packages/run-engine/src/engine/tests/pendingVersion.test.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ describe("RunEngine pending version", () => {
314314
);
315315

316316
containerTest(
317-
"PENDING_VERSION re-enqueue arms TTL on the queued message",
317+
"PENDING_VERSION re-enqueue arms TTL but anchors the scheduling-delay clock at the promotion",
318318
async ({ prisma, redisOptions }) => {
319319
// When a run enters PENDING_VERSION (background worker doesn't yet have
320320
// the task), the first enqueue happens but the message is dequeued and
@@ -407,6 +407,7 @@ describe("RunEngine pending version", () => {
407407

408408
// Now a worker arrives WITH the task — pendingVersionSystem
409409
// re-enqueues the run.
410+
const beforePromotion = Date.now();
410411
await setupBackgroundWorker(engine, authenticatedEnvironment, taskIdentifier);
411412

412413
await setTimeout(1000);
@@ -424,6 +425,21 @@ describe("RunEngine pending version", () => {
424425
assertNonNullable(message);
425426
expect(message.ttlExpiresAt).toBeDefined();
426427
expect(typeof message.ttlExpiresAt).toBe("number");
428+
429+
const queuePositionMs = (run.queueTimestamp ?? run.createdAt).getTime();
430+
431+
expect(
432+
message.eligibleAtMs,
433+
"scheduling-delay clock must start at the promotion, since the run was not runnable while it waited for a worker version"
434+
).toBeGreaterThanOrEqual(beforePromotion);
435+
expect(
436+
message.eligibleAtMs,
437+
"anchoring at the original queue position would bill the whole wait-for-deploy period as queue scheduling delay"
438+
).toBeGreaterThan(queuePositionMs);
439+
expect(
440+
message.timestamp,
441+
"ordering keeps the original queue position, so waiting for a deployment must not cost the run its place in line"
442+
).toBe(queuePositionMs - run.priorityMs);
427443
} finally {
428444
await engine.quit();
429445
}

0 commit comments

Comments
 (0)