Skip to content

Commit 22dbbc9

Browse files
committed
feat(redis-worker): mollifier ack marks materialised + grace TTL (Phase B2)
`MollifierBuffer.ack` previously deleted the entry hash. It now sets `materialised=true` and resets the TTL to a 30s grace window via a new atomic `ackMollifierEntry` Lua script. The entry hash persists past materialisation as a read-fallback safety net for the brief PG replica lag window between drainer-side write and reader-side visibility (Q1 D2). `BufferEntrySchema` gains an optional `materialised` boolean (string "true"/"false" in Redis → boolean in JS). Accept still refuses while *any* entry exists for the runId — including materialised ones — as defense-in-depth against runId reuse. The drainer's "drains one queued entry … and acks" test now asserts `materialised=true` instead of entry deletion. The "re-accept after ack works" test is inverted to "accept refused while a previously-acked entry is still inside its grace TTL".
1 parent c193f53 commit 22dbbc9

5 files changed

Lines changed: 106 additions & 12 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@trigger.dev/redis-worker": patch
3+
---
4+
5+
Mollifier drainer ack no longer deletes the entry hash. Instead, `MollifierBuffer.ack` sets `materialised=true` on the entry and resets its TTL to a 30s grace window. Entry hashes persist past materialisation as a read-fallback safety net for the brief PG replica-lag window between drainer-side write and reader-side visibility. `BufferEntrySchema` gains an optional `materialised` boolean.

packages/redis-worker/src/mollifier/buffer.test.ts

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,41 @@ describe("MollifierBuffer.pop", () => {
172172
});
173173

174174
describe("MollifierBuffer.ack", () => {
175-
redisTest("ack deletes the entry", { timeout: 20_000 }, async ({ redisContainer }) => {
175+
redisTest(
176+
"ack marks entry materialised and applies the grace TTL — entry persists as a read-fallback safety net",
177+
{ timeout: 20_000 },
178+
async ({ redisContainer }) => {
179+
const buffer = new MollifierBuffer({
180+
redisOptions: {
181+
host: redisContainer.getHost(),
182+
port: redisContainer.getPort(),
183+
password: redisContainer.getPassword(),
184+
},
185+
entryTtlSeconds: 600,
186+
logger: new Logger("test", "log"),
187+
});
188+
189+
try {
190+
await buffer.accept({ runId: "run_x", envId: "env_a", orgId: "org_1", payload: "{}" });
191+
await buffer.pop("env_a");
192+
await buffer.ack("run_x");
193+
194+
const after = await buffer.getEntry("run_x");
195+
expect(after).not.toBeNull();
196+
expect(after!.materialised).toBe(true);
197+
198+
// TTL was reset to the grace window — should be at most 30s, well
199+
// under the original 600s entryTtlSeconds.
200+
const ttl = await buffer.getEntryTtlSeconds("run_x");
201+
expect(ttl).toBeGreaterThan(0);
202+
expect(ttl).toBeLessThanOrEqual(30);
203+
} finally {
204+
await buffer.close();
205+
}
206+
},
207+
);
208+
209+
redisTest("ack on missing entry is a no-op", { timeout: 20_000 }, async ({ redisContainer }) => {
176210
const buffer = new MollifierBuffer({
177211
redisOptions: {
178212
host: redisContainer.getHost(),
@@ -184,12 +218,12 @@ describe("MollifierBuffer.ack", () => {
184218
});
185219

186220
try {
187-
await buffer.accept({ runId: "run_x", envId: "env_a", orgId: "org_1", payload: "{}" });
188-
await buffer.pop("env_a");
189-
await buffer.ack("run_x");
190-
191-
const after = await buffer.getEntry("run_x");
192-
expect(after).toBeNull();
221+
await buffer.ack("run_ghost");
222+
const stored = await buffer.getEntry("run_ghost");
223+
expect(stored).toBeNull();
224+
// Critical: no partial hash created.
225+
const raw = await buffer["redis"].hgetall("mollifier:entries:run_ghost");
226+
expect(Object.keys(raw)).toHaveLength(0);
193227
} finally {
194228
await buffer.close();
195229
}
@@ -909,9 +943,15 @@ describe("MollifierBuffer.accept idempotency", () => {
909943
);
910944

911945
redisTest(
912-
"re-accept after ack works (terminal entry can be re-accepted)",
946+
"accept refused while a previously-acked (materialised) entry is still inside its grace TTL",
913947
{ timeout: 20_000 },
914948
async ({ redisContainer }) => {
949+
// After ack, the entry hash persists for the grace window as a
950+
// read-fallback safety net (Q1 D2). RunIds are server-generated and
951+
// never collide in practice, but defense-in-depth: accept refuses
952+
// while *any* entry exists for the runId, including materialised
953+
// ones. The entry hash's TTL is now ~30s instead of the original
954+
// entryTtlSeconds.
915955
const buffer = new MollifierBuffer({
916956
redisOptions: {
917957
host: redisContainer.getHost(),
@@ -932,7 +972,6 @@ describe("MollifierBuffer.accept idempotency", () => {
932972
await buffer.pop("env_a");
933973
await buffer.ack("run_x");
934974

935-
// Entry is gone — re-accept should succeed.
936975
const reAccept = await buffer.accept({
937976
runId: "run_x",
938977
envId: "env_a",
@@ -941,7 +980,10 @@ describe("MollifierBuffer.accept idempotency", () => {
941980
});
942981

943982
expect(first).toBe(true);
944-
expect(reAccept).toBe(true);
983+
expect(reAccept).toBe(false);
984+
985+
const stored = await buffer.getEntry("run_x");
986+
expect(stored!.materialised).toBe(true);
945987
} finally {
946988
await buffer.close();
947989
}

packages/redis-worker/src/mollifier/buffer.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ export type MollifierBufferOptions = {
1414
logger?: Logger;
1515
};
1616

17+
// Grace TTL applied to the entry hash on drainer ack. The entry survives
18+
// this long after materialisation so direct reads (retrieve, trace, etc.)
19+
// have a safety net while PG replica lag settles. Q1 D2.
20+
const ACK_GRACE_TTL_SECONDS = 30;
21+
1722
export class MollifierBuffer {
1823
private readonly redis: Redis;
1924
private readonly entryTtlSeconds: number;
@@ -158,8 +163,15 @@ export class MollifierBuffer {
158163
return entries;
159164
}
160165

166+
// Marks the entry as materialised (PG row written) and resets its TTL to
167+
// the grace window. Entry hash persists past ack as a read-fallback
168+
// safety net for the brief PG replica-lag window between drainer-side
169+
// write and reader-side visibility (Q1 D2).
161170
async ack(runId: string): Promise<void> {
162-
await this.redis.del(`mollifier:entries:${runId}`);
171+
await this.redis.ackMollifierEntry(
172+
`mollifier:entries:${runId}`,
173+
String(ACK_GRACE_TTL_SECONDS),
174+
);
163175
}
164176

165177
async requeue(runId: string): Promise<void> {
@@ -353,6 +365,24 @@ export class MollifierBuffer {
353365
`,
354366
});
355367

368+
this.redis.defineCommand("ackMollifierEntry", {
369+
numberOfKeys: 1,
370+
lua: `
371+
local entryKey = KEYS[1]
372+
local graceTtlSeconds = tonumber(ARGV[1])
373+
374+
-- Guard: never create a partial entry. If the hash expired between
375+
-- pop and ack, the run is gone — nothing to mark materialised.
376+
if redis.call('EXISTS', entryKey) == 0 then
377+
return 0
378+
end
379+
380+
redis.call('HSET', entryKey, 'materialised', 'true')
381+
redis.call('EXPIRE', entryKey, graceTtlSeconds)
382+
return 1
383+
`,
384+
});
385+
356386
this.redis.defineCommand("failMollifierEntry", {
357387
numberOfKeys: 1,
358388
lua: `
@@ -427,6 +457,11 @@ declare module "@internal/redis" {
427457
orgEnvsPrefix: string,
428458
callback?: Callback<number>,
429459
): Result<number, Context>;
460+
ackMollifierEntry(
461+
entryKey: string,
462+
graceTtlSeconds: string,
463+
callback?: Callback<number>,
464+
): Result<number, Context>;
430465
failMollifierEntry(
431466
entryKey: string,
432467
errorPayload: string,

packages/redis-worker/src/mollifier/drainer.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,11 @@ describe("MollifierDrainer.runOnce", () => {
8787
payload: { foo: 1 },
8888
});
8989

90+
// After ack the entry persists as a read-fallback safety net with
91+
// materialised=true and a fresh grace TTL (Q1 D2 / Phase B2).
9092
const entry = await buffer.getEntry("run_1");
91-
expect(entry).toBeNull();
93+
expect(entry).not.toBeNull();
94+
expect(entry!.materialised).toBe(true);
9295
} finally {
9396
await buffer.close();
9497
}

packages/redis-worker/src/mollifier/schemas.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ const stringToDate = z.string().transform((v, ctx) => {
2727
return d;
2828
});
2929

30+
const stringToBool = z
31+
.union([z.literal("true"), z.literal("false")])
32+
.transform((v) => v === "true");
33+
3034
const stringToError = z.string().transform((v, ctx) => {
3135
try {
3236
return BufferEntryError.parse(JSON.parse(v));
@@ -47,6 +51,11 @@ export const BufferEntrySchema = z.object({
4751
// Microsecond epoch matching the ZSET queue score. Stable across
4852
// requeues — the score never moves once set at accept time.
4953
createdAtMicros: stringToInt,
54+
// Drainer-ack flag: `true` once the drainer has materialised this run
55+
// into PG. The hash persists for a short grace TTL after ack so direct
56+
// reads (retrieve, trace, etc.) still resolve while PG replica lag
57+
// settles. Absent on pre-ack entries.
58+
materialised: stringToBool.default("false"),
5059
lastError: stringToError.optional(),
5160
});
5261

0 commit comments

Comments
 (0)