Skip to content

Commit 45b2ebc

Browse files
committed
fix(run-engine,webapp,sdk): converge override failures and unpin blocked keys
A variant blocked at its per-key limit or by a gate now backs off in the candidate index instead of pinning the bounded window, so zero-limit keys can never starve runnable keys behind them; acks and nacks rebalance the score back the moment capacity frees. Override writes persist before enforcing and resets enforce before clearing, so a failure on either side leaves a state a retry converges from, with the cap-rejected row compensated away. Deploys restore per-key overrides from their durable rows into the engine, and an empty concurrencyKey in the SDK no longer falls through to the queue-wide endpoint.
1 parent dbe8498 commit 45b2ebc

5 files changed

Lines changed: 209 additions & 30 deletions

File tree

‎apps/webapp/app/v3/services/concurrencySystem.server.ts‎

Lines changed: 96 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ export class ConcurrencySystem {
104104
},
105105
resetTotalConcurrencyLimit: (environment: AuthenticatedEnvironment, queue: QueueInput) => {
106106
return findQueueFromInput(this.db, environment, queue)
107+
.andThen((queue) => syncQueueTotalConcurrencyResetToEngine(environment, queue))
107108
.andThen((queue) => resetQueueTotalConcurrencyLimit(this.db, queue))
108109
.andThen((queue) => syncQueueTotalConcurrencyToEngine(environment, queue))
109110
.andThen((queue) => getQueueStats(environment, queue));
@@ -134,7 +135,9 @@ export class ConcurrencySystem {
134135
concurrencyKey: string
135136
) => {
136137
return findQueueFromInput(this.db, environment, queue)
137-
.andThen((queue) => resetQueueConcurrencyKeyLimit(this.db, environment, queue, concurrencyKey))
138+
.andThen((queue) =>
139+
resetQueueConcurrencyKeyLimit(this.db, environment, queue, concurrencyKey)
140+
)
138141
.andThen((queue) => getQueueStats(environment, queue));
139142
},
140143
/**
@@ -420,6 +423,35 @@ function overrideQueueTotalConcurrencyLimit(
420423
);
421424
}
422425

426+
/**
427+
* Enforce first, then persist: syncs the engine to the declared base BEFORE clearing
428+
* the override marker, so an engine failure leaves the marker set and a retry
429+
* converges instead of being rejected while the overridden limit stays enforced.
430+
*/
431+
function syncQueueTotalConcurrencyResetToEngine(
432+
environment: AuthenticatedEnvironment,
433+
queue: TaskQueue
434+
) {
435+
if (queue.totalConcurrencyLimitOverriddenAt === null) {
436+
return errAsync({ type: "queue_not_overridden" as const });
437+
}
438+
439+
if (typeof queue.totalConcurrencyLimitBase === "number") {
440+
return fromPromise(
441+
updateQueueTotalConcurrencyLimits(environment, queue.name, queue.totalConcurrencyLimitBase),
442+
(error) => ({
443+
type: "sync_queue_concurrency_to_engine_failed" as const,
444+
cause: error,
445+
})
446+
).andThen(() => okAsync(queue));
447+
}
448+
449+
return fromPromise(removeQueueTotalConcurrencyLimits(environment, queue.name), (error) => ({
450+
type: "sync_queue_concurrency_to_engine_failed" as const,
451+
cause: error,
452+
})).andThen(() => okAsync(queue));
453+
}
454+
423455
function resetQueueTotalConcurrencyLimit(db: PrismaClientOrTransaction, queue: TaskQueue) {
424456
if (queue.totalConcurrencyLimitOverriddenAt === null) {
425457
return errAsync({ type: "queue_not_overridden" as const });
@@ -466,6 +498,13 @@ function syncQueueTotalConcurrencyToEngine(
466498
})).andThen(() => okAsync(queue));
467499
}
468500

501+
/**
502+
* Persist first, then enforce: a database failure leaves nothing enforced and the
503+
* request errors cleanly, while an engine failure after persistence leaves a durable
504+
* record and a retry converges (the upsert is idempotent). When the engine rejects a
505+
* NEW key for exceeding the per-queue override cap, the just-created row is removed
506+
* again so the record never claims an override the engine refused.
507+
*/
469508
function overrideQueueConcurrencyKeyLimit(
470509
db: PrismaClientOrTransaction,
471510
environment: AuthenticatedEnvironment,
@@ -491,20 +530,13 @@ function overrideQueueConcurrencyKeyLimit(
491530
}
492531

493532
return fromPromise(
494-
engine.runQueue.updateQueueConcurrencyKeyLimit(
495-
environment,
496-
queue.name,
497-
concurrencyKey,
498-
concurrencyLimit
499-
),
500-
(error) => {
501-
if (error instanceof RunQueueConcurrencyKeyLimitExceededError) {
502-
return { type: "too_many_key_overrides" as const, message: error.message };
503-
}
504-
return { type: "sync_queue_concurrency_to_engine_failed" as const, cause: error };
505-
}
533+
db.taskQueueConcurrencyKeyOverride.findFirst({
534+
where: { taskQueueId: queue.id, concurrencyKey },
535+
select: { id: true },
536+
}),
537+
(error) => ({ type: "other" as const, cause: error })
506538
)
507-
.andThen(() =>
539+
.andThen((existing) =>
508540
fromPromise(
509541
db.taskQueueConcurrencyKeyOverride.upsert({
510542
where: { taskQueueId_concurrencyKey: { taskQueueId: queue.id, concurrencyKey } },
@@ -524,27 +556,56 @@ function overrideQueueConcurrencyKeyLimit(
524556
type: "queue_update_failed" as const,
525557
cause: error,
526558
})
527-
)
559+
).map(() => existing)
560+
)
561+
.andThen((existing) =>
562+
fromPromise(
563+
engine.runQueue.updateQueueConcurrencyKeyLimit(
564+
environment,
565+
queue.name,
566+
concurrencyKey,
567+
concurrencyLimit
568+
),
569+
(error) => {
570+
if (error instanceof RunQueueConcurrencyKeyLimitExceededError) {
571+
return { type: "too_many_key_overrides" as const, message: error.message };
572+
}
573+
return { type: "sync_queue_concurrency_to_engine_failed" as const, cause: error };
574+
}
575+
).orElse((error) => {
576+
if (!existing && error.type === "too_many_key_overrides") {
577+
return fromPromise(
578+
db.taskQueueConcurrencyKeyOverride.deleteMany({
579+
where: { taskQueueId: queue.id, concurrencyKey },
580+
}),
581+
() => error
582+
).andThen(() => errAsync(error));
583+
}
584+
return errAsync(error);
585+
})
528586
)
529587
.andThen(() => okAsync(queue));
530588
}
531589

590+
/**
591+
* Enforce first, then persist: removing the engine limit is idempotent, so an engine
592+
* failure leaves the override row in place and a retry converges instead of being
593+
* rejected as not overridden while the old limit is still enforced.
594+
*/
532595
function resetQueueConcurrencyKeyLimit(
533596
db: PrismaClientOrTransaction,
534597
environment: AuthenticatedEnvironment,
535598
queue: TaskQueue,
536599
concurrencyKey: string
537600
) {
538601
return fromPromise(
539-
db.taskQueueConcurrencyKeyOverride.deleteMany({
602+
db.taskQueueConcurrencyKeyOverride.findFirst({
540603
where: { taskQueueId: queue.id, concurrencyKey },
604+
select: { id: true },
541605
}),
542-
(error) => ({
543-
type: "queue_update_failed" as const,
544-
cause: error,
545-
})
546-
).andThen((deleted) => {
547-
if (deleted.count === 0) {
606+
(error) => ({ type: "other" as const, cause: error })
607+
).andThen((existing) => {
608+
if (!existing) {
548609
return errAsync({ type: "queue_not_overridden" as const });
549610
}
550611

@@ -554,7 +615,19 @@ function resetQueueConcurrencyKeyLimit(
554615
type: "sync_queue_concurrency_to_engine_failed" as const,
555616
cause: error,
556617
})
557-
).andThen(() => okAsync(queue));
618+
)
619+
.andThen(() =>
620+
fromPromise(
621+
db.taskQueueConcurrencyKeyOverride.deleteMany({
622+
where: { taskQueueId: queue.id, concurrencyKey },
623+
}),
624+
(error) => ({
625+
type: "queue_update_failed" as const,
626+
cause: error,
627+
})
628+
)
629+
)
630+
.andThen(() => okAsync(queue));
558631
});
559632
}
560633

‎apps/webapp/app/v3/services/createBackgroundWorker.server.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,31 @@ async function createWorkerQueue(
582582
await removeQueueTotalConcurrencyLimits(environment, taskQueue.name);
583583
}
584584

585+
/**
586+
* Restore per-key limit overrides into the engine so a fresh or flushed Redis
587+
* converges back to the durable records on the next deploy. Row-level failures
588+
* are logged rather than failing the deploy; the next deploy retries them.
589+
*/
590+
const keyOverrides = await prisma.taskQueueConcurrencyKeyOverride.findMany({
591+
where: { taskQueueId: taskQueue.id },
592+
});
593+
for (const keyOverride of keyOverrides) {
594+
try {
595+
await engine.runQueue.updateQueueConcurrencyKeyLimit(
596+
environment,
597+
taskQueue.name,
598+
keyOverride.concurrencyKey,
599+
keyOverride.concurrencyLimit
600+
);
601+
} catch (error) {
602+
logger.error("createWorkerQueue: failed to restore concurrency key override", {
603+
taskQueueId: taskQueue.id,
604+
concurrencyKey: keyOverride.concurrencyKey,
605+
error,
606+
});
607+
}
608+
}
609+
585610
if (!taskQueue.paused) {
586611
if (typeof newConcurrencyLimit === "number") {
587612
logger.debug("createWorkerQueue: updating concurrency limit", {

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

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,10 @@ export type RunQueueOptions = {
312312
* that dead-lettered or suspended through a mirror-less path. Enabling only after
313313
* every instance runs this build avoids the noise but is no longer load-bearing
314314
* for correctness.
315+
*
316+
* Per-concurrency-key limit overrides are part of the same concurrency-limits
317+
* feature and are deliberately enforced behind this flag too: writes are always
318+
* accepted and durable, and enforcement of both arrives together.
315319
*/
316320
totalConcurrencyEnabled?: boolean;
317321
/**
@@ -5061,6 +5065,7 @@ for _, ckQueueName in ipairs(ckQueues) do
50615065
end
50625066
50635067
local fullQueueKey = keyPrefix .. ckQueueName
5068+
local blockedByGates = false
50645069
50655070
local ckConcurrencyKey = fullQueueKey .. ':currentConcurrency'
50665071
local ckCurrentConcurrency = tonumber(redis.call('SCARD', ckConcurrencyKey) or '0')
@@ -5073,6 +5078,14 @@ for _, ckQueueName in ipairs(ckQueues) do
50735078
end
50745079
end
50755080
5081+
if ckCurrentConcurrency >= perKeyLimit then
5082+
-- Back a blocked variant off so it cannot pin the bounded candidate window
5083+
-- and starve later keys (acute with a zero per-key override, which never
5084+
-- self-clears). Acks and nacks rebalance the score back to the oldest
5085+
-- message, so the key is eligible again the moment capacity frees.
5086+
redis.call('ZADD', ckIndexKey, currentTime + 1000, ckQueueName)
5087+
end
5088+
50765089
if ckCurrentConcurrency < perKeyLimit then
50775090
local messages = redis.call('ZRANGEBYSCORE', fullQueueKey, '-inf', tostring(currentTime), 'WITHSCORES', 'LIMIT', 0, 1)
50785091
@@ -5100,6 +5113,9 @@ for _, ckQueueName in ipairs(ckQueues) do
51005113
if gatesEnabled then
51015114
gatesAllow = __gatesHaveCapacity(keyPrefix, messageData, messageId, envConcurrencyLimit, messageKeyPrefix)
51025115
end
5116+
if not gatesAllow then
5117+
blockedByGates = true
5118+
end
51035119
51045120
local alreadyInGroup = false
51055121
local totalAllows = true
@@ -5142,11 +5158,15 @@ for _, ckQueueName in ipairs(ckQueues) do
51425158
decrLengthCounter()
51435159
end
51445160
5145-
local earliest = redis.call('ZRANGE', fullQueueKey, 0, 0, 'WITHSCORES')
5146-
if #earliest == 0 then
5147-
redis.call('ZREM', ckIndexKey, ckQueueName)
5161+
if blockedByGates then
5162+
redis.call('ZADD', ckIndexKey, currentTime + 1000, ckQueueName)
51485163
else
5149-
redis.call('ZADD', ckIndexKey, earliest[2], ckQueueName)
5164+
local earliest = redis.call('ZRANGE', fullQueueKey, 0, 0, 'WITHSCORES')
5165+
if #earliest == 0 then
5166+
redis.call('ZREM', ckIndexKey, ckQueueName)
5167+
else
5168+
redis.call('ZADD', ckIndexKey, earliest[2], ckQueueName)
5169+
end
51505170
end
51515171
else
51525172
local any = redis.call('ZRANGE', fullQueueKey, 0, 0, 'WITHSCORES')

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

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,17 @@ const authenticatedEnvDev = {
3232
organization: { id: "o1234" },
3333
};
3434

35-
function createQueue(redisContainer: any, totalConcurrencyEnabled: boolean, maxOverrides?: number) {
35+
function createQueue(
36+
redisContainer: any,
37+
totalConcurrencyEnabled: boolean,
38+
maxOverrides?: number,
39+
dequeueCount?: number
40+
) {
3641
return new RunQueue({
3742
...testOptions,
3843
totalConcurrencyEnabled,
3944
maxConcurrencyKeyOverridesPerQueue: maxOverrides,
45+
masterQueueConsumerDequeueCount: dequeueCount,
4046
queueSelectionStrategy: new FairQueueSelectionStrategy({
4147
redis: {
4248
keyPrefix: "runqueue:test:",
@@ -239,6 +245,61 @@ describe("RunQueue per-concurrency-key limit overrides", () => {
239245
}
240246
});
241247

248+
redisTest(
249+
"blocked keys cannot pin the candidate window and starve later keys",
250+
async ({ redisContainer }) => {
251+
/** dequeueCount 2 makes the candidate window 6 variants wide. */
252+
const queue = createQueue(redisContainer, true, undefined, 2);
253+
try {
254+
await queue.updateQueueConcurrencyLimits(authenticatedEnvDev, "task/my-task", 5);
255+
256+
/**
257+
* Ten zero-limit keys with OLDER messages fill the window many times over;
258+
* without the blocked-key backoff the runnable key behind them would never
259+
* be examined.
260+
*/
261+
const now = Date.now();
262+
for (let i = 0; i < 10; i++) {
263+
const ck = `blocked-${i}`;
264+
await queue.updateQueueConcurrencyKeyLimit(authenticatedEnvDev, "task/my-task", ck, 0);
265+
await queue.enqueueMessage({
266+
env: authenticatedEnvDev,
267+
message: makeMessage({
268+
runId: `b${i}`,
269+
concurrencyKey: ck,
270+
timestamp: now - 10_000 + i,
271+
}),
272+
workerQueue: "main",
273+
});
274+
}
275+
276+
await queue.enqueueMessage({
277+
env: authenticatedEnvDev,
278+
message: makeMessage({
279+
runId: "good-0",
280+
concurrencyKey: "ck-good",
281+
timestamp: now - 500,
282+
}),
283+
workerQueue: "main",
284+
});
285+
286+
const goodAdmitted = await waitFor(
287+
async () =>
288+
(await queue.currentConcurrencyOfQueue(
289+
authenticatedEnvDev,
290+
"task/my-task",
291+
"ck-good"
292+
)) === 1,
293+
30_000
294+
);
295+
expect(goodAdmitted).toBe(true);
296+
expect(await queue.lengthOfQueue(authenticatedEnvDev, "task/my-task")).toBe(10);
297+
} finally {
298+
await queue.quit();
299+
}
300+
}
301+
);
302+
242303
redisTest("overrides are ignored when disabled", async ({ redisContainer }) => {
243304
const queue = createQueue(redisContainer, false);
244305
try {

‎packages/trigger-sdk/src/v3/queues.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ export function overrideConcurrencyLimit(
170170
requestOptions
171171
);
172172

173-
if (concurrencyKey) {
173+
if (concurrencyKey !== undefined) {
174174
return apiClient.overrideQueueConcurrencyKeyLimit(
175175
queue,
176176
concurrencyKey,
@@ -291,7 +291,7 @@ export function resetConcurrencyLimit(
291291
requestOptions
292292
);
293293

294-
if (concurrencyKey) {
294+
if (concurrencyKey !== undefined) {
295295
return apiClient.resetQueueConcurrencyKeyLimit(queue, concurrencyKey, $requestOptions);
296296
}
297297

0 commit comments

Comments
 (0)