Skip to content

Commit 4ec7e01

Browse files
committed
fix(write-queue): outbox pattern closes partial-commit window
submitWrite now inserts the ledger row + outbox envelope inside one DB transaction. The inline XADD stays as a happy-path latency optimization: on success we mark the outbox row sent, on failure the upcoming drain worker will pick it up. Process death between the commit and the XADD no longer loses the write — the outbox row stays unsent and the drain catches it. Also adds a skip-if-completed guard to processNext. Duplicate stream delivery (PEL redelivery, outbox re-XADD) previously ran the handler a second time and wrote the row twice. Now it short-circuits to ack. - repo: insertOutbox, markOutboxSent, getLedgerStatus helpers - service: submitWrite wrapped in transaction returning outboxId - service: processNext checks ledger status before running handler
1 parent d2758fb commit 4ec7e01

2 files changed

Lines changed: 147 additions & 17 deletions

File tree

slices/write-queue/src/repo.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,72 @@ export async function updateLedgerStatus({
6262
.where(eq(schema.writeLedger.writeId, writeId));
6363
}
6464

65+
/**
66+
* Read-only status lookup used by processNext to skip duplicate stream
67+
* deliveries (PEL redelivery or outbox drain re-XADD). Returns null if the
68+
* ledger row doesn't exist, which is itself a signal that the write was
69+
* never accepted — caller should treat as "safe to ack and move on."
70+
*/
71+
export async function getLedgerStatus({
72+
db,
73+
writeId,
74+
}: {
75+
db: Db;
76+
writeId: string;
77+
}): Promise<WriteLedgerStatus | null> {
78+
const rows = await db
79+
.select({ status: schema.writeLedger.status })
80+
.from(schema.writeLedger)
81+
.where(eq(schema.writeLedger.writeId, writeId))
82+
.limit(1);
83+
return (rows[0]?.status ?? null) as WriteLedgerStatus | null;
84+
}
85+
86+
/**
87+
* Outbox insert — runs inside submitWrite's transaction so the ledger row
88+
* and the queue envelope commit atomically. Returns the outbox row id so
89+
* the caller can mark it sent after a successful inline XADD.
90+
*/
91+
export async function insertOutbox({
92+
db,
93+
writeId,
94+
sheetId,
95+
streamKey,
96+
envelope,
97+
}: {
98+
db: Db;
99+
writeId: string;
100+
sheetId: string;
101+
streamKey: string;
102+
envelope: unknown;
103+
}): Promise<string> {
104+
const [row] = await db
105+
.insert(schema.writeOutbox)
106+
.values({ writeId, sheetId, streamKey, envelope })
107+
.returning({ id: schema.writeOutbox.id });
108+
if (!row) throw new Error('insert into write_outbox did not return a row');
109+
return row.id;
110+
}
111+
112+
/**
113+
* Mark an outbox row as sent so the drain worker skips it. Called best-
114+
* effort after an inline XADD succeeds; if this fails the drain worker
115+
* may re-XADD, but processNext's skip-if-completed guard prevents the
116+
* handler from running twice.
117+
*/
118+
export async function markOutboxSent({
119+
db,
120+
id,
121+
}: {
122+
db: Db;
123+
id: string;
124+
}): Promise<void> {
125+
await db
126+
.update(schema.writeOutbox)
127+
.set({ sentAt: new Date() })
128+
.where(eq(schema.writeOutbox.id, id));
129+
}
130+
65131
/**
66132
* Postgres advisory lock scoped to a single transaction. Returns true if the
67133
* lock was acquired, false if another transaction currently holds it. Released

slices/write-queue/src/service.ts

Lines changed: 81 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { randomUUID } from 'node:crypto';
22
import {
33
type ClaimedMessage,
4+
type EnqueueMessage,
45
type QueueRedisClient,
56
ackMessage,
67
claimNext,
@@ -14,7 +15,10 @@ import {
1415
countLedgerByStatus,
1516
findLedgerByIdempotencyKey,
1617
findRecentLedger,
18+
getLedgerStatus,
1719
insertLedger,
20+
insertOutbox,
21+
markOutboxSent,
1822
tryAdvisoryXactLock,
1923
updateLedgerStatus,
2024
} from './repo.js';
@@ -66,17 +70,41 @@ export async function submitWrite({
6670
}
6771

6872
const writeId = randomUUID();
73+
const streamKey = streamKeyForSheet(sheetId);
74+
const envelope: EnqueueMessage<WritePayload> = { writeId, idempotencyKey, payload };
75+
76+
// Atomic commit of the ledger row + outbox envelope. The outbox closes
77+
// the gap where a process death between the INSERT and the Redis XADD
78+
// would leave a "pending" ledger row with no stream entry — permanently
79+
// lost. By staging the envelope in Postgres within the same transaction,
80+
// a background drain worker can later re-XADD anything the inline path
81+
// didn't manage to deliver.
82+
let outboxId: string;
6983
try {
70-
await insertLedger({
71-
db,
72-
sheetId,
73-
idempotencyKey: idempotencyKey ?? null,
74-
writeId,
84+
outboxId = await (
85+
db as unknown as {
86+
transaction: <T>(cb: (tx: Db) => Promise<T>) => Promise<T>;
87+
}
88+
).transaction(async (tx) => {
89+
await insertLedger({
90+
db: tx,
91+
sheetId,
92+
idempotencyKey: idempotencyKey ?? null,
93+
writeId,
94+
});
95+
return await insertOutbox({
96+
db: tx,
97+
writeId,
98+
sheetId,
99+
streamKey,
100+
envelope,
101+
});
75102
});
76103
} catch (err) {
77-
// Concurrent submitWrite() raced us between the find above and the insert
78-
// here. The partial unique index on (sheet_id, idempotency_key) catches
79-
// it; treat as a replay so the caller still gets ONE writeId per key.
104+
// Concurrent submitWrite() raced us between the find above and the
105+
// transaction here. The partial unique index on
106+
// (sheet_id, idempotency_key) catches it; treat as a replay so the
107+
// caller still gets ONE writeId per key.
80108
if (idempotencyKey !== undefined && isUniqueViolation(err)) {
81109
const prior = await findLedgerByIdempotencyKey({
82110
db,
@@ -90,21 +118,46 @@ export async function submitWrite({
90118
);
91119
return { writeId: prior.writeId, status: 'replayed' };
92120
}
93-
// Race-loser found nothing — winner must have rolled back. Don't leak
94-
// the raw constraint name to clients via the 500 message.
121+
// Race-loser found nothing — winner must have rolled back. Don't
122+
// leak the raw constraint name to clients via the 500 message.
95123
throw new InternalError('write submission failed; please retry');
96124
}
97125
throw err;
98126
}
99127

100-
const streamKey = streamKeyForSheet(sheetId);
101-
const { messageId } = await enqueue({
102-
redis,
103-
streamKey,
104-
message: { writeId, idempotencyKey, payload },
105-
});
128+
// Best-effort inline XADD — keeps happy-path latency low. On success
129+
// we mark the outbox row sent so the drain skips it. On failure the
130+
// drain worker will pick it up within ~1s; the extra stream entry it
131+
// might later produce is absorbed by processNext's skip-if-completed
132+
// guard.
133+
let messageId: string | undefined;
134+
try {
135+
const { messageId: mid } = await enqueue({ redis, streamKey, message: envelope });
136+
messageId = mid;
137+
try {
138+
await markOutboxSent({ db, id: outboxId });
139+
} catch (markErr) {
140+
log.warn(
141+
{
142+
err: markErr instanceof Error ? markErr.message : String(markErr),
143+
outboxId,
144+
writeId,
145+
},
146+
'outbox-mark-sent-failed',
147+
);
148+
}
149+
} catch (enqueueErr) {
150+
log.warn(
151+
{
152+
err: enqueueErr instanceof Error ? enqueueErr.message : String(enqueueErr),
153+
writeId,
154+
outboxId,
155+
},
156+
'inline-xadd-failed-drain-will-retry',
157+
);
158+
}
106159

107-
log.debug({ sheetId, writeId, messageId }, 'enqueued');
160+
log.debug({ sheetId, writeId, messageId, outboxId }, 'enqueued');
108161
return { writeId, status: 'enqueued', messageId };
109162
}
110163

@@ -178,6 +231,17 @@ export async function processNext<P extends WritePayload>({
178231
return; // transaction commits without touching the ledger
179232
}
180233

234+
// Guard against duplicate stream delivery: Redis PEL redelivery or
235+
// the outbox drain re-XADDing the same writeId can land a message
236+
// for work already completed. Running the handler again would
237+
// write the same row twice. Short-circuit to "safe to ack" instead.
238+
const currentStatus = await getLedgerStatus({ db: tx, writeId: msg.writeId });
239+
if (currentStatus === 'completed' || currentStatus === 'dead_lettered') {
240+
log.debug({ streamKey, writeId: msg.writeId, currentStatus }, 'skip-already-terminal');
241+
processedInsideTx = true;
242+
return;
243+
}
244+
181245
await updateLedgerStatus({
182246
db: tx,
183247
writeId: msg.writeId,

0 commit comments

Comments
 (0)