Skip to content

Commit 352c612

Browse files
committed
fix(run-engine,webapp): return unclaimed runs to the queue when pausing
Pausing a queue or environment set its concurrency limit to 0, which stops the queue admitting new runs but does nothing about runs it had already admitted. Those runs sit in a per-region worker queue that supervisors drain with a plain BLPOP, with no pause or concurrency check on the pop path, so they went on to execute during the pause. An idle environment with headroom can have its whole concurrency limit's worth of runs in that state, since the trigger-time fast path pushes straight to the worker queue. Adds RunQueue.returnUnclaimedMessagesToQueue, which moves runs the queue has admitted but no worker has claimed yet back into the pending queue. Candidates come from the difference between currentConcurrency and currentDequeued, which identifies worker queue residents without scanning the region-wide list. Each move runs in a Lua script whose LREM doubles as the claim: if it removes nothing then a worker already owns that run and every other key is left alone, so a concurrent BLPOP can never produce a duplicate dispatch. Ordering is preserved by re-adding at the message's original timestamp rather than a fresh score the way nack does, so a returned run keeps its place in line. Callers must set the concurrency limit to 0 first: returning a run costs it its position in the worker queue FIFO and it can only re-enter at the back, so a queue that can still admit work could let a newer run overtake one on its way back. All three pause callers already do this ordering. Covers the manual queue pause, the manual environment pause, and the billing-limit environment pause.
1 parent 109e245 commit 352c612

10 files changed

Lines changed: 1019 additions & 6 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Pausing a queue or an environment now also holds back runs that were already waiting to start. Previously those runs would still go ahead, so a pause could take effect a little later than expected.

apps/webapp/app/v3/runQueue.server.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,19 @@ export async function removeQueueConcurrencyLimits(
3030
) {
3131
await engine.runQueue.removeQueueConcurrencyLimits(environment, queueName);
3232
}
33+
34+
/**
35+
* Returns runs waiting to be picked up by a worker back into their queue.
36+
*
37+
* Only safe once the concurrency limit has been set to 0, otherwise a newer run can be
38+
* admitted and overtake one on its way back.
39+
*/
40+
export async function returnUnclaimedMessagesToQueue({
41+
environment,
42+
queue,
43+
}: {
44+
environment: AuthenticatedEnvironment;
45+
queue?: string;
46+
}) {
47+
return engine.returnUnclaimedMessagesToQueue({ environment, queue });
48+
}

apps/webapp/app/v3/services/billingLimit/billingLimitConvergeEnvironments.server.ts

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,16 @@ type UpdateEnvConcurrency = (
2929
maximumConcurrencyLimit?: number
3030
) => Promise<void>;
3131

32+
type ReturnUnclaimedMessages = (environment: EnvironmentWithRelations) => Promise<void>;
33+
3234
export async function convergeBillingLimitEnvironmentsForOrg(
3335
organizationId: string,
3436
targetState: BillingLimitConvergeTargetState,
3537
options?: {
3638
batchSize?: number;
3739
prismaClient?: PrismaClient;
3840
updateConcurrency?: UpdateEnvConcurrency;
41+
returnUnclaimed?: ReturnUnclaimedMessages;
3942
}
4043
): Promise<ConvergeOrgResult> {
4144
const db = options?.prismaClient ?? prisma;
@@ -51,18 +54,32 @@ export async function convergeBillingLimitEnvironmentsForOrg(
5154
return updateEnvConcurrencyLimits(environment, maximumConcurrencyLimit);
5255
});
5356

57+
const returnUnclaimed =
58+
options?.returnUnclaimed ??
59+
(async (environment) => {
60+
const { returnUnclaimedMessagesToQueue } = await import("~/v3/runQueue.server");
61+
await returnUnclaimedMessagesToQueue({ environment });
62+
});
63+
5464
if (targetState === "ok") {
5565
return unpauseBillingLimitEnvironments(organizationId, db, batchSize, updateConcurrency);
5666
}
5767

58-
return pauseBillingLimitEnvironments(organizationId, db, batchSize, updateConcurrency);
68+
return pauseBillingLimitEnvironments(
69+
organizationId,
70+
db,
71+
batchSize,
72+
updateConcurrency,
73+
returnUnclaimed
74+
);
5975
}
6076

6177
async function pauseBillingLimitEnvironments(
6278
organizationId: string,
6379
db: PrismaClient,
6480
batchSize: number,
65-
updateConcurrency: UpdateEnvConcurrency
81+
updateConcurrency: UpdateEnvConcurrency,
82+
returnUnclaimed: ReturnUnclaimedMessages
6683
): Promise<ConvergeOrgResult> {
6784
let paused = 0;
6885
let cursor: string | undefined;
@@ -88,7 +105,7 @@ async function pauseBillingLimitEnvironments(
88105
}
89106

90107
for (const environment of environments) {
91-
await pauseEnvironmentForBillingLimit(environment, db, updateConcurrency);
108+
await pauseEnvironmentForBillingLimit(environment, db, updateConcurrency, returnUnclaimed);
92109
paused++;
93110
}
94111

@@ -156,7 +173,8 @@ async function unpauseBillingLimitEnvironments(
156173
async function pauseEnvironmentForBillingLimit(
157174
environment: EnvironmentWithRelations,
158175
db: PrismaClient,
159-
updateConcurrency: UpdateEnvConcurrency
176+
updateConcurrency: UpdateEnvConcurrency,
177+
returnUnclaimed: ReturnUnclaimedMessages
160178
) {
161179
const updated = await db.runtimeEnvironment.update({
162180
where: { id: environment.id },
@@ -172,6 +190,7 @@ async function pauseEnvironmentForBillingLimit(
172190

173191
try {
174192
await updateConcurrency(updated, 0);
193+
await returnUnclaimed(updated);
175194
} catch (error) {
176195
await db.runtimeEnvironment.update({
177196
where: { id: environment.id },

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { EnvironmentPauseSource, type PrismaClientOrTransaction } from "@trigger
22
import { prisma } from "~/db.server";
33
import { logger } from "~/services/logger.server";
44
import { getManualPauseEnvironmentResult } from "~/v3/services/billingLimit/manualPauseEnvironmentGuard.server";
5-
import { updateEnvConcurrencyLimits } from "../runQueue.server";
5+
import { returnUnclaimedMessagesToQueue, updateEnvConcurrencyLimits } from "../runQueue.server";
66
import { WithRunEngine } from "./baseService.server";
77
import type { AuthenticatedEnvironment } from "~/services/apiAuth.server";
88
import { controlPlaneResolver } from "~/v3/runOpsMigration/controlPlaneResolver.server";
@@ -114,6 +114,7 @@ export class PauseEnvironmentService extends WithRunEngine {
114114
environmentId: environment.id,
115115
});
116116
await updateEnvConcurrencyLimits(environment, 0);
117+
await returnUnclaimedMessagesToQueue({ environment });
117118
} else {
118119
logger.debug("PauseEnvironmentService: resuming environment", {
119120
environmentId: environment.id,

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@ import { type AuthenticatedEnvironment } from "~/services/apiAuth.server";
44
import { logger } from "~/services/logger.server";
55
import { BaseService } from "./baseService.server";
66
import { determineEngineVersion } from "../engineVersion.server";
7-
import { removeQueueConcurrencyLimits, updateQueueConcurrencyLimits } from "../runQueue.server";
7+
import {
8+
removeQueueConcurrencyLimits,
9+
returnUnclaimedMessagesToQueue,
10+
updateQueueConcurrencyLimits,
11+
} from "../runQueue.server";
812
import { engine } from "../runEngine.server";
913

1014
export type PauseStatus = "paused" | "resumed";
@@ -59,6 +63,7 @@ export class PauseQueueService extends BaseService {
5963

6064
if (action === "paused") {
6165
await updateQueueConcurrencyLimits(environment, queue.name, 0);
66+
await returnUnclaimedMessagesToQueue({ environment, queue: queue.name });
6267
} else {
6368
if (queue.concurrencyLimit) {
6469
await updateQueueConcurrencyLimits(environment, queue.name, queue.concurrencyLimit);

apps/webapp/test/billingLimitConvergeEnvironments.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,78 @@ describe("convergeBillingLimitEnvironmentsForOrg", () => {
4949
expect(envAfter.pauseSource).toBeNull();
5050
});
5151

52+
postgresTest("returns unclaimed runs after pausing for a billing limit", async ({ prisma }) => {
53+
const { organization, project } = await createTestOrgProjectWithMember(prisma);
54+
const environment = await createRuntimeEnvironment(prisma, {
55+
projectId: project.id,
56+
organizationId: organization.id,
57+
type: "PRODUCTION",
58+
slug: uniqueId("prod"),
59+
});
60+
61+
const calls: Array<{ concurrency?: number; returnedFor?: string }> = [];
62+
63+
const result = await convergeBillingLimitEnvironmentsForOrg(organization.id, "grace", {
64+
prismaClient: prisma,
65+
updateConcurrency: async (_env, maximumConcurrencyLimit) => {
66+
calls.push({ concurrency: maximumConcurrencyLimit });
67+
},
68+
returnUnclaimed: async (env) => {
69+
calls.push({ returnedFor: env.id });
70+
},
71+
});
72+
73+
expect(result).toEqual({ paused: 1, unpaused: 0 });
74+
75+
expect(calls).toEqual([{ concurrency: 0 }, { returnedFor: environment.id }]);
76+
77+
const envAfter = await prisma.runtimeEnvironment.findUniqueOrThrow({
78+
where: { id: environment.id },
79+
});
80+
expect(envAfter.paused).toBe(true);
81+
expect(envAfter.pauseSource).toBe(EnvironmentPauseSource.BILLING_LIMIT);
82+
});
83+
84+
postgresTest("does not return unclaimed runs when unpausing", async ({ prisma }) => {
85+
const { organization } = await createBillingPausedProductionEnv(prisma);
86+
87+
const returnUnclaimed = vi.fn(async () => undefined);
88+
89+
await convergeBillingLimitEnvironmentsForOrg(organization.id, "ok", {
90+
prismaClient: prisma,
91+
updateConcurrency: async () => undefined,
92+
returnUnclaimed,
93+
});
94+
95+
expect(returnUnclaimed).not.toHaveBeenCalled();
96+
});
97+
98+
postgresTest("rolls back pause when returning unclaimed runs fails", async ({ prisma }) => {
99+
const { organization, project } = await createTestOrgProjectWithMember(prisma);
100+
const environment = await createRuntimeEnvironment(prisma, {
101+
projectId: project.id,
102+
organizationId: organization.id,
103+
type: "PRODUCTION",
104+
slug: uniqueId("prod"),
105+
});
106+
107+
await expect(
108+
convergeBillingLimitEnvironmentsForOrg(organization.id, "grace", {
109+
prismaClient: prisma,
110+
updateConcurrency: async () => undefined,
111+
returnUnclaimed: async () => {
112+
throw new Error("run queue unavailable");
113+
},
114+
})
115+
).rejects.toThrow("run queue unavailable");
116+
117+
const envAfter = await prisma.runtimeEnvironment.findUniqueOrThrow({
118+
where: { id: environment.id },
119+
});
120+
expect(envAfter.paused).toBe(false);
121+
expect(envAfter.pauseSource).toBeNull();
122+
});
123+
52124
postgresTest("rolls back pause when concurrency update fails", async ({ prisma }) => {
53125
const { organization, project } = await createTestOrgProjectWithMember(prisma);
54126
const environment = await createRuntimeEnvironment(prisma, {

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1634,6 +1634,24 @@ export class RunEngine {
16341634
return this.runQueue.currentConcurrencyOfQueues(environment, queues);
16351635
}
16361636

1637+
/**
1638+
* Returns runs that have been admitted for execution but that no worker has claimed yet
1639+
* back into their queue, at the position they held before.
1640+
*
1641+
* Pausing only stops runs being admitted; anything already handed to a worker queue would
1642+
* otherwise still execute. Call this after the queue has been made ineligible for
1643+
* dequeuing, never before — see `RunQueue.returnUnclaimedMessagesToQueue`.
1644+
*/
1645+
async returnUnclaimedMessagesToQueue({
1646+
environment,
1647+
queue,
1648+
}: {
1649+
environment: MinimalAuthenticatedEnvironment;
1650+
queue?: string;
1651+
}): Promise<{ returned: number; skipped: number }> {
1652+
return this.runQueue.returnUnclaimedMessagesToQueue({ env: environment, queue });
1653+
}
1654+
16371655
async removeEnvironmentQueuesFromMasterQueue({
16381656
runtimeEnvironmentId,
16391657
organizationId,

0 commit comments

Comments
 (0)