Skip to content

Commit 5389588

Browse files
committed
fix(webapp,run-engine): stop batchTriggerAndWait hanging when item streaming never completes
Phase 1 of the 2-phase batch API blocks the parent run on the batch waitpoint, but only phase 2 can seal the batch. If the item stream never completes, nothing seals the batch, nothing completes the waitpoint, and the parent waits forever. Phase 1 now mints a bounded grant that lets phase 2 through the general API rate limiter, so a batch already admitted by the batch limiter can finish streaming instead of being rejected by a second limiter. A seal-timeout reaper then aborts any batch left unsealed past the timeout and completes the parent waitpoint with an error, so batchTriggerAndWait rejects rather than hanging. The batches page also stops reporting success when asked to resume a batch that can never complete.
1 parent e8a2dbd commit 5389588

14 files changed

Lines changed: 765 additions & 10 deletions
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+
Batch triggers no longer fail to start their runs when an environment is under heavy API load. If a batch still can't finish being created, `batchTriggerAndWait` now fails with an error instead of leaving the parent run waiting forever, and the batches page says so rather than reporting that it resumed.

apps/webapp/app/env.server.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -879,6 +879,20 @@ const EnvironmentSchema = z
879879
BATCH_RATE_LIMIT_MAX: z.coerce.number().int().default(1200),
880880
BATCH_RATE_LIMIT_REFILL_INTERVAL: z.string().default("10s"),
881881
BATCH_CONCURRENCY_LIMIT_DEFAULT: z.coerce.number().int().default(5),
882+
/**
883+
* How long a created batch may remain unsealed before the seal-timeout reaper
884+
* aborts it and resumes any blocked parent with an error. Must exceed the SDK's
885+
* worst-case stream-retry budget (maxAttempts x server request timeout).
886+
* Doubles as the TTL of the phase 2 streaming grant, so the grant and the reaper
887+
* always agree on how long a batch is allowed to be sealing.
888+
*/
889+
BATCH_SEAL_TIMEOUT_MS: z.coerce.number().int().positive().default(1_800_000),
890+
/**
891+
* Number of phase 2 (`POST /api/v3/batches/:id/items`) requests a created batch is
892+
* granted, exempt from the general API rate limit. Sized above the SDK's stream
893+
* maxAttempts so a batch admitted by the batch limiter can always finish streaming.
894+
*/
895+
BATCH_STREAM_GRANT_ATTEMPTS: z.coerce.number().int().positive().default(10),
882896

883897
REALTIME_STREAM_VERSION: z.enum(["v1", "v2"]).default("v1"),
884898
REALTIME_STREAM_MAX_LENGTH: z.coerce.number().int().default(1000),

apps/webapp/app/routes/resources.batches.$batchId.check-completion.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,20 @@ export const action: ActionFunction = async ({ request, params }) => {
4242
return redirectWithErrorMessage(safeRedirectUrl, request, "Batch not found");
4343
}
4444

45+
const batch = await runStore.findBatchTaskRunById(ownedBatchRunId);
46+
47+
if (!batch) {
48+
return redirectWithErrorMessage(safeRedirectUrl, request, "Batch not found");
49+
}
50+
51+
if (!batch.sealed) {
52+
return redirectWithErrorMessage(
53+
safeRedirectUrl,
54+
request,
55+
"This batch was never finished being created, so it can't be resumed. Please get in touch and we'll recover it for you."
56+
);
57+
}
58+
4559
try {
4660
// v3 (engine V1) is retired; finalize the batch through the v2 completion path (no-op if not ready).
4761
await tryCompleteBatchV3(ownedBatchRunId, prisma, true);
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import { createRedisClient, type RedisClient, type RedisWithClusterOptions } from "~/redis.server";
2+
import { logger } from "~/services/logger.server";
3+
4+
export type BatchStreamGrantsOptions = {
5+
redis: RedisWithClusterOptions;
6+
/** How many phase 2 requests a created batch is allowed. */
7+
attempts: number;
8+
/** How long the grant survives, matching how long a batch may legitimately be sealing. */
9+
ttlMs: number;
10+
};
11+
12+
const KEY_PREFIX = "batch-stream-grant:";
13+
14+
/**
15+
* Admission for phase 2 of the 2-phase batch API.
16+
*
17+
* Phase 1 (`POST /api/v3/batches`) already passes its own batch rate limiter, which fixes
18+
* the batch's `expectedCount` and blocks the parent run on the batch's waitpoint. Phase 2
19+
* (`POST /api/v3/batches/:id/items`) is the only thing that can seal that batch, so having
20+
* the general API limiter reject it strands the batch and the parent with it.
21+
*
22+
* Phase 1 therefore mints a bounded grant, and phase 2 spends it to bypass the general
23+
* limiter. Admission stays a single decision made in phase 1, but the bypass is capped at
24+
* `attempts` requests per batch rather than being unconditional.
25+
*/
26+
export class BatchStreamGrants {
27+
private readonly redis: RedisClient;
28+
29+
constructor(private readonly options: BatchStreamGrantsOptions) {
30+
this.redis = createRedisClient("batchStreamGrants", options.redis);
31+
this.#registerCommands();
32+
}
33+
34+
/**
35+
* Grant a newly created batch its phase 2 budget. Never throws: a batch that fails to get
36+
* a grant still works, it just falls back to the general rate limiter for streaming.
37+
*/
38+
async mint(batchId: string): Promise<void> {
39+
try {
40+
await this.redis.set(this.#key(batchId), this.options.attempts, "PX", this.options.ttlMs);
41+
} catch (error) {
42+
logger.warn("BatchStreamGrants: failed to mint grant", {
43+
batchId,
44+
error: error instanceof Error ? error.message : String(error),
45+
});
46+
}
47+
}
48+
49+
/**
50+
* Consume one phase 2 request from the batch's grant.
51+
*
52+
* Returns false when there is no grant, when the budget is spent, or when Redis is
53+
* unreachable, so the caller falls back to the general rate limiter rather than opening
54+
* an unbounded bypass.
55+
*/
56+
async spend(batchId: string): Promise<boolean> {
57+
try {
58+
// @ts-expect-error - Custom command defined via defineCommand
59+
const remaining = (await this.redis.spendBatchStreamGrant(this.#key(batchId))) as number;
60+
61+
return remaining >= 0;
62+
} catch (error) {
63+
logger.warn("BatchStreamGrants: failed to spend grant", {
64+
batchId,
65+
error: error instanceof Error ? error.message : String(error),
66+
});
67+
68+
return false;
69+
}
70+
}
71+
72+
async quit(): Promise<void> {
73+
await this.redis.quit();
74+
}
75+
76+
#key(batchId: string): string {
77+
return `${KEY_PREFIX}${batchId}`;
78+
}
79+
80+
#registerCommands(): void {
81+
this.redis.defineCommand("spendBatchStreamGrant", {
82+
numberOfKeys: 1,
83+
lua: `
84+
local remaining = tonumber(redis.call('GET', KEYS[1]))
85+
86+
if not remaining or remaining <= 0 then
87+
return -1
88+
end
89+
90+
return redis.call('DECR', KEYS[1])
91+
`,
92+
});
93+
}
94+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { env } from "~/env.server";
2+
import { singleton } from "~/utils/singleton";
3+
import { BatchStreamGrants } from "./batchStreamGrants.server";
4+
5+
export const batchStreamGrants = singleton(
6+
"batchStreamGrants",
7+
() =>
8+
new BatchStreamGrants({
9+
redis: {
10+
port: env.RATE_LIMIT_REDIS_PORT,
11+
host: env.RATE_LIMIT_REDIS_HOST,
12+
username: env.RATE_LIMIT_REDIS_USERNAME,
13+
password: env.RATE_LIMIT_REDIS_PASSWORD,
14+
tlsDisabled: env.RATE_LIMIT_REDIS_TLS_DISABLED === "true",
15+
clusterMode: env.RATE_LIMIT_REDIS_CLUSTER_MODE_ENABLED === "1",
16+
},
17+
attempts: env.BATCH_STREAM_GRANT_ATTEMPTS,
18+
ttlMs: env.BATCH_SEAL_TIMEOUT_MS,
19+
})
20+
);

apps/webapp/app/runEngine/services/createBatch.server.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import { RunId } from "@trigger.dev/core/v3/isomorphic";
44
import { type BatchTaskRun, Prisma } from "@trigger.dev/database";
55
import { Evt } from "evt";
66
import { prisma, type PrismaClientOrTransaction } from "~/db.server";
7+
import { env } from "~/env.server";
8+
import { batchStreamGrants } from "../concerns/batchStreamGrantsInstance.server";
79
import type { AuthenticatedEnvironment } from "~/services/apiAuth.server";
810
import { logger } from "~/services/logger.server";
911
import { controlPlaneResolver } from "~/v3/runOpsMigration/controlPlaneResolver.server";
@@ -120,6 +122,8 @@ export class CreateBatchService extends WithRunEngine {
120122

121123
this.onBatchTaskRunCreated.post(batch);
122124

125+
await batchStreamGrants.mint(friendlyId);
126+
123127
// Block parent run if this is a batchTriggerAndWait
124128
if (body.parentRunId && body.resumeParentOnCompletion) {
125129
await this._engine.blockRunWithCreatedBatch({
@@ -129,6 +133,11 @@ export class CreateBatchService extends WithRunEngine {
129133
projectId: environment.projectId,
130134
organizationId: environment.organizationId,
131135
});
136+
137+
await this._engine.scheduleExpireBatch({
138+
batchId: batch.id,
139+
availableAt: new Date(Date.now() + env.BATCH_SEAL_TIMEOUT_MS),
140+
});
132141
}
133142

134143
// Initialize batch metadata in Redis (without items)

apps/webapp/app/services/apiRateLimit.server.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
import { env } from "~/env.server";
2+
import { batchStreamGrants } from "~/runEngine/concerns/batchStreamGrantsInstance.server";
23
import { authenticateAuthorizationHeader } from "./apiAuth.server";
34
import { authorizationRateLimitMiddleware } from "./authorizationRateLimitMiddleware.server";
45
import type { Duration } from "./rateLimiter.server";
56

7+
const BATCH_STREAM_ITEMS_PATH = /^\/api\/v3\/batches\/([^/]+)\/items$/;
8+
69
export const apiRateLimiter = authorizationRateLimitMiddleware({
710
redis: {
811
port: env.RATE_LIMIT_REDIS_PORT,
@@ -75,6 +78,21 @@ export const apiRateLimiter = authorizationRateLimitMiddleware({
7578
/^\/api\/v2\/packets\//,
7679
/^\/api\/v1\/sessions\/[^/]+\/snapshot-url$/,
7780
],
81+
bypass: async (req) => {
82+
const match = BATCH_STREAM_ITEMS_PATH.exec(req.path);
83+
84+
if (!match) {
85+
return false;
86+
}
87+
88+
const batchFriendlyId = match[1];
89+
90+
if (!batchFriendlyId) {
91+
return false;
92+
}
93+
94+
return batchStreamGrants.spend(batchFriendlyId);
95+
},
7896
log: {
7997
rejections: env.API_RATE_LIMIT_REJECTION_LOGS_ENABLED === "1",
8098
requests: env.API_RATE_LIMIT_REQUEST_LOGS_ENABLED === "1",

apps/webapp/app/services/authorizationRateLimitMiddleware.server.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@ type Options = {
6060
keyPrefix: string;
6161
pathMatchers: (RegExp | string)[];
6262
pathWhiteList?: (RegExp | string)[];
63+
/**
64+
* Escape hatch for requests that can only be admitted by consulting state, rather than by
65+
* matching a path. Runs after the authorization header check, so an unauthenticated
66+
* request is still rejected, and only skips the rate limit itself. Must not throw: a
67+
* bypass that cannot decide should return false and let the limiter apply.
68+
*/
69+
bypass?: (req: ExpressRequest) => Promise<boolean>;
6370
defaultLimiter: RateLimiterConfig;
6471
limiterConfigOverride?: LimitConfigOverrideFunction;
6572
limiterCache?: {
@@ -151,6 +158,7 @@ export function authorizationRateLimitMiddleware({
151158
defaultLimiter,
152159
pathMatchers,
153160
pathWhiteList = [],
161+
bypass,
154162
log = {
155163
rejections: true,
156164
requests: true,
@@ -247,6 +255,13 @@ export function authorizationRateLimitMiddleware({
247255
);
248256
}
249257

258+
if (bypass && (await bypass(req))) {
259+
if (log.requests) {
260+
logger.info(`RateLimiter (${keyPrefix}): bypassed ${req.path}`);
261+
}
262+
return next();
263+
}
264+
250265
const hash = createHash("sha256");
251266
hash.update(authorizationValue);
252267
const hashedAuthorizationValue = hash.digest("hex");

0 commit comments

Comments
 (0)