fix(webapp): stop locked-version triggers failing on stale replica reads (#3930)
## Summary `triggerAndWait` (and other locked-version triggers) could intermittently fail with `Task '<id>' not found on locked version '<version>'` for a task that was registered on that version. The failures came in bursts and recovered on their own, so a retry minutes later would succeed. ## Root cause For a locked-version trigger, the queue resolver looks up the task's `BackgroundWorkerTask` metadata from the read replica (behind a Redis cache). On a cache miss it queried the replica, and a `null` result was treated as "task not registered" and turned into a non-retryable 422. A read replica can return an empty result for a row that already exists on the primary, so a momentarily-behind replica produced a false negative even though the locked worker (resolved on the primary in the same request) clearly had the task. ## Fix On a cache miss, when the replica returns no row the resolver now re-checks the primary before concluding the task is missing. If the primary has the row it is used (and the cache is back-filled); the error fires only when the primary genuinely lacks it, which is the only case where the 422 is correct. The extra read happens on the cache-miss-and-replica-empty path only, so the hot path is unchanged. Verified with a unit test (replica stub vs. real primary) and end-to-end against a local streaming replica with replication paused to reproduce the stale read. TRI-10868
This commit is contained in:
@@ -0,0 +1,6 @@
|
||||
---
|
||||
area: webapp
|
||||
type: fix
|
||||
---
|
||||
|
||||
Fix locked-version triggers such as triggerAndWait occasionally failing with "task not found on locked version" for a task that is actually registered, by confirming against the primary database when the read replica returns no row.
|
||||
@@ -268,14 +268,27 @@ export class DefaultQueueManager implements QueueManager {
|
||||
const cached = await this.taskMetaCache.getByWorker(workerId, slug);
|
||||
if (cached) return cached;
|
||||
|
||||
const row = await this.replicaPrisma.backgroundWorkerTask.findFirst({
|
||||
where: { workerId, runtimeEnvironmentId: environmentId, slug },
|
||||
select: {
|
||||
ttl: true,
|
||||
triggerSource: true,
|
||||
queue: { select: { id: true, name: true } },
|
||||
},
|
||||
});
|
||||
// Cache miss. Read the row from the replica first; if the replica comes
|
||||
// back empty, re-check the writer before concluding the task is missing.
|
||||
// The locked worker itself was just resolved on the writer (see
|
||||
// triggerTask.server.ts), so a replica that returns no row here is stale,
|
||||
// not authoritative. Trusting a stale-replica negative throws a
|
||||
// non-retryable "not found on locked version" for a task that is in fact
|
||||
// registered. The writer read only runs on this rare miss-then-empty path,
|
||||
// never on the hot path.
|
||||
let row = await this.findLockedTaskRow(this.replicaPrisma, workerId, environmentId, slug);
|
||||
|
||||
if (!row && this.replicaPrisma !== this.prisma) {
|
||||
row = await this.findLockedTaskRow(this.prisma, workerId, environmentId, slug);
|
||||
|
||||
if (row) {
|
||||
logger.warn("Locked task metadata missing on replica but found on writer", {
|
||||
workerId,
|
||||
environmentId,
|
||||
slug,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
if (!row) return null;
|
||||
|
||||
@@ -294,6 +307,22 @@ export class DefaultQueueManager implements QueueManager {
|
||||
return entry;
|
||||
}
|
||||
|
||||
private findLockedTaskRow(
|
||||
client: PrismaClientOrTransaction,
|
||||
workerId: string,
|
||||
environmentId: string,
|
||||
slug: string
|
||||
) {
|
||||
return client.backgroundWorkerTask.findFirst({
|
||||
where: { workerId, runtimeEnvironmentId: environmentId, slug },
|
||||
select: {
|
||||
ttl: true,
|
||||
triggerSource: true,
|
||||
queue: { select: { id: true, name: true } },
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve task metadata for a non-locked trigger. Reads from the
|
||||
* `task-meta:env:{envId}` Redis hash; falls back to
|
||||
|
||||
@@ -23,7 +23,10 @@ import { TaskRun } from "@trigger.dev/database";
|
||||
import { Redis } from "ioredis";
|
||||
import { IdempotencyKeyConcern } from "~/runEngine/concerns/idempotencyKeys.server";
|
||||
import { DefaultQueueManager } from "~/runEngine/concerns/queues.server";
|
||||
import { RedisTaskMetadataCache } from "~/services/taskMetadataCache.server";
|
||||
import {
|
||||
NoopTaskMetadataCache,
|
||||
RedisTaskMetadataCache,
|
||||
} from "~/services/taskMetadataCache.server";
|
||||
import {
|
||||
EntitlementValidationParams,
|
||||
MaxAttemptsValidationParams,
|
||||
@@ -949,6 +952,129 @@ describe("RunEngineTriggerTaskService", () => {
|
||||
}
|
||||
);
|
||||
|
||||
containerTest(
|
||||
"should fall back to the writer when a stale replica returns no row for a locked task",
|
||||
async ({ prisma, redisOptions }) => {
|
||||
const engine = new RunEngine({
|
||||
prisma,
|
||||
worker: {
|
||||
redis: redisOptions,
|
||||
workers: 1,
|
||||
tasksPerWorker: 10,
|
||||
pollIntervalMs: 100,
|
||||
},
|
||||
queue: {
|
||||
redis: redisOptions,
|
||||
},
|
||||
runLock: {
|
||||
redis: redisOptions,
|
||||
},
|
||||
machines: {
|
||||
defaultMachine: "small-1x",
|
||||
machines: {
|
||||
"small-1x": {
|
||||
name: "small-1x" as const,
|
||||
cpu: 0.5,
|
||||
memory: 0.5,
|
||||
centsPerMs: 0.0001,
|
||||
},
|
||||
},
|
||||
baseCostInCents: 0.0005,
|
||||
},
|
||||
tracer: trace.getTracer("test", "0.0.0"),
|
||||
});
|
||||
|
||||
const authenticatedEnvironment = await setupAuthenticatedEnvironment(prisma, "PRODUCTION");
|
||||
const taskIdentifier = "test-task";
|
||||
|
||||
const worker = await setupBackgroundWorker(engine, authenticatedEnvironment, taskIdentifier);
|
||||
|
||||
// A read replica that has not yet caught up to the BackgroundWorkerTask
|
||||
// row: it is the real database for every query except the locked-task
|
||||
// lookup, which comes back empty (the TRI-10868 false-negative window).
|
||||
const staleReplica = new Proxy(prisma, {
|
||||
get(target, prop, receiver) {
|
||||
if (prop === "backgroundWorkerTask") {
|
||||
const delegate = Reflect.get(target, prop, receiver);
|
||||
return new Proxy(delegate, {
|
||||
get(taskTarget, taskProp, taskReceiver) {
|
||||
if (taskProp === "findFirst") {
|
||||
return async () => null;
|
||||
}
|
||||
const value = Reflect.get(taskTarget, taskProp, taskReceiver);
|
||||
return typeof value === "function" ? value.bind(taskTarget) : value;
|
||||
},
|
||||
});
|
||||
}
|
||||
const value = Reflect.get(target, prop, receiver);
|
||||
return typeof value === "function" ? value.bind(target) : value;
|
||||
},
|
||||
}) as typeof prisma;
|
||||
|
||||
// Noop cache so every resolve misses the cache and exercises the
|
||||
// replica -> writer fallback. The writer is the real `prisma`.
|
||||
const queuesManager = new DefaultQueueManager(
|
||||
prisma,
|
||||
engine,
|
||||
staleReplica,
|
||||
new NoopTaskMetadataCache()
|
||||
);
|
||||
|
||||
const triggerTaskService = new RunEngineTriggerTaskService({
|
||||
engine,
|
||||
prisma,
|
||||
payloadProcessor: new MockPayloadProcessor(),
|
||||
queueConcern: queuesManager,
|
||||
idempotencyKeyConcern: new IdempotencyKeyConcern(
|
||||
prisma,
|
||||
engine,
|
||||
new MockTraceEventConcern()
|
||||
),
|
||||
validator: new MockTriggerTaskValidator(),
|
||||
traceEventConcern: new MockTraceEventConcern(),
|
||||
tracer: trace.getTracer("test", "0.0.0"),
|
||||
metadataMaximumSize: 1024 * 1024 * 1,
|
||||
});
|
||||
|
||||
// The task IS registered on the locked worker, but the replica returns
|
||||
// nothing. Before the fix this threw "not found on locked version"; now
|
||||
// the writer fallback resolves the registered row.
|
||||
const result = await triggerTaskService.call({
|
||||
taskId: taskIdentifier,
|
||||
environment: authenticatedEnvironment,
|
||||
body: {
|
||||
payload: { test: "test" },
|
||||
options: {
|
||||
lockToVersion: worker.worker.version,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
expect(result).toBeDefined();
|
||||
expect(result?.run.status).toBe("PENDING");
|
||||
expect(result?.run.queue).toBe(`task/${taskIdentifier}`);
|
||||
|
||||
// A genuinely unregistered task must still throw, even with the writer
|
||||
// fallback — the writer has no row either, so the 422 is correct.
|
||||
await expect(
|
||||
triggerTaskService.call({
|
||||
taskId: "not-a-registered-task",
|
||||
environment: authenticatedEnvironment,
|
||||
body: {
|
||||
payload: { test: "test" },
|
||||
options: {
|
||||
lockToVersion: worker.worker.version,
|
||||
},
|
||||
},
|
||||
})
|
||||
).rejects.toThrow(
|
||||
`Task 'not-a-registered-task' not found on locked version '${worker.worker.version}'`
|
||||
);
|
||||
|
||||
await engine.quit();
|
||||
}
|
||||
);
|
||||
|
||||
containerTest(
|
||||
"should preserve runFriendlyId across retries when RunDuplicateIdempotencyKeyError is thrown",
|
||||
async ({ prisma, redisOptions }) => {
|
||||
|
||||
Reference in New Issue
Block a user