Skip to content

Commit 6e31ab0

Browse files
d-csTrigger.dev RepoOps
authored andcommitted
fix(webapp): stop replication services re-acking an older commit LSN
Stops the runs, sessions and webhook-deliveries replication services from reporting an older replication position after a keepalive. On an idle database this made `pg_stat_replication` lag appear to grow without bound, even though the slot itself was current. The acknowledge loop compared the keepalive LSN (unpadded) with the last commit LSN (zero-padded) as strings, so they never matched and the older commit LSN was re-sent. LSNs are now compared numerically, and an ack is skipped when the last acknowledged LSN is already at or past the latest commit: ```ts if ( runtime.lastAcknowledgedLsn && lsnToUInt64(runtime.lastAcknowledgedLsn) >= lsnToUInt64(runtime.latestCommitEndLsn) ) { return; } ``` Each service has an integration test against real Postgres that fails before the fix (flush LSN moves backwards) and passes after. Mono-RevId: 3cbe8f510438e51171aa6577edafb525b30bef1a
1 parent ddf9160 commit 6e31ab0

7 files changed

Lines changed: 239 additions & 4 deletions

‎apps/webapp/app/services/runsReplicationService.server.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -832,7 +832,11 @@ export class RunsReplicationService {
832832
return;
833833
}
834834

835-
if (runtime.lastAcknowledgedLsn === runtime.latestCommitEndLsn) {
835+
// Keepalive acks can be ahead of the last commit; re-acking the commit would move flush back.
836+
if (
837+
runtime.lastAcknowledgedLsn &&
838+
lsnToUInt64(runtime.lastAcknowledgedLsn) >= lsnToUInt64(runtime.latestCommitEndLsn)
839+
) {
836840
return;
837841
}
838842

‎apps/webapp/app/services/sessionsReplicationService.server.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,11 @@ export class SessionsReplicationService {
512512
return;
513513
}
514514

515-
if (this._lastAcknowledgedLsn === this._latestCommitEndLsn) {
515+
// Keepalive acks can be ahead of the last commit; re-acking the commit would move flush back.
516+
if (
517+
this._lastAcknowledgedLsn &&
518+
lsnToUInt64(this._lastAcknowledgedLsn) >= lsnToUInt64(this._latestCommitEndLsn)
519+
) {
516520
return;
517521
}
518522

‎apps/webapp/app/services/webhookDeliveriesReplicationService.server.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,11 @@ export class WebhookDeliveriesReplicationService {
527527
return;
528528
}
529529

530-
if (this._lastAcknowledgedLsn === this._latestCommitEndLsn) {
530+
// Keepalive acks can be ahead of the last commit; re-acking the commit would move flush back.
531+
if (
532+
this._lastAcknowledgedLsn &&
533+
lsnToUInt64(this._lastAcknowledgedLsn) >= lsnToUInt64(this._latestCommitEndLsn)
534+
) {
531535
return;
532536
}
533537

‎apps/webapp/test/runsReplicationService.part5.test.ts‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ import { ClickHouse } from "@internal/clickhouse";
22
import { replicationContainerTest } from "@internal/testcontainers";
33
import { z } from "zod";
44
import { RunsReplicationService } from "~/services/runsReplicationService.server";
5+
import {
6+
expectFlushLsnHoldsPastIdleWal,
7+
withShortWalSenderTimeout,
8+
} from "./utils/replicationUtils";
59
import { TestReplicationClickhouseFactory } from "./utils/testReplicationClickhouseFactory";
610

711
vi.setConfig({ testTimeout: 60_000 });
@@ -144,4 +148,79 @@ describe("RunsReplicationService (part 5/7)", () => {
144148
await runsReplicationService.stop();
145149
}
146150
);
151+
// Catches: the 1s ack loop re-acking an older commit LSN after a keepalive ack, which moves
152+
// pg_stat_replication flush_lsn backwards and inflates reported lag on an idle database.
153+
replicationContainerTest(
154+
"never acknowledges an older commit LSN after a keepalive acknowledgement",
155+
async ({ clickhouseContainer, redisOptions, postgresContainer, prisma }) => {
156+
const clickhouse = new ClickHouse({
157+
url: clickhouseContainer.getConnectionUrl(),
158+
name: "runs-replication-keepalive-ack",
159+
logLevel: "warn",
160+
});
161+
162+
const slotName = "task_runs_keepalive_ack_v1";
163+
const runsReplicationService = new RunsReplicationService({
164+
clickhouseFactory: new TestReplicationClickhouseFactory(clickhouse),
165+
pgConnectionUrl: withShortWalSenderTimeout(postgresContainer.getConnectionUri()),
166+
serviceName: "runs-replication-keepalive-ack",
167+
slotName,
168+
publicationName: "task_runs_keepalive_ack_v1_publication",
169+
redisOptions,
170+
maxFlushConcurrency: 1,
171+
flushIntervalMs: 100,
172+
flushBatchSize: 1,
173+
leaderLockTimeoutMs: 5000,
174+
leaderLockExtendIntervalMs: 1000,
175+
logLevel: "warn",
176+
});
177+
178+
await runsReplicationService.start();
179+
try {
180+
const organization = await prisma.organization.create({
181+
data: { title: "keepalive-ack", slug: "keepalive-ack" },
182+
});
183+
const project = await prisma.project.create({
184+
data: {
185+
name: "keepalive-ack",
186+
slug: "keepalive-ack",
187+
organizationId: organization.id,
188+
externalRef: "keepalive-ack",
189+
},
190+
});
191+
const runtimeEnvironment = await prisma.runtimeEnvironment.create({
192+
data: {
193+
slug: "keepalive-ack",
194+
type: "DEVELOPMENT",
195+
projectId: project.id,
196+
organizationId: organization.id,
197+
apiKey: "keepalive-ack",
198+
pkApiKey: "keepalive-ack",
199+
shortcode: "keepalive-ack",
200+
},
201+
});
202+
await prisma.taskRun.create({
203+
data: {
204+
friendlyId: "run_keepalive_ack",
205+
taskIdentifier: "my-task",
206+
payload: "{}",
207+
payloadType: "application/json",
208+
traceId: "keepalive-ack",
209+
spanId: "keepalive-ack",
210+
queue: "keepalive-ack",
211+
runtimeEnvironmentId: runtimeEnvironment.id,
212+
projectId: project.id,
213+
organizationId: organization.id,
214+
environmentType: "DEVELOPMENT",
215+
engine: "V2",
216+
status: "PENDING",
217+
},
218+
});
219+
220+
await expectFlushLsnHoldsPastIdleWal(prisma, slotName);
221+
} finally {
222+
await runsReplicationService.stop();
223+
}
224+
}
225+
);
147226
});

‎apps/webapp/test/sessionsReplicationService.test.ts‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ import { replicationContainerTest } from "@internal/testcontainers";
33
import { setTimeout } from "node:timers/promises";
44
import { z } from "zod";
55
import { SessionsReplicationService } from "~/services/sessionsReplicationService.server";
6+
import {
7+
expectFlushLsnHoldsPastIdleWal,
8+
withShortWalSenderTimeout,
9+
} from "./utils/replicationUtils";
610
import { TestReplicationClickhouseFactory } from "./utils/testReplicationClickhouseFactory";
711

812
vi.setConfig({ testTimeout: 60_000 });
@@ -212,4 +216,75 @@ describe("SessionsReplicationService", () => {
212216
await service.stop();
213217
}
214218
);
219+
220+
// Catches: the 1s ack loop re-acking an older commit LSN after a keepalive ack (flush_lsn regresses).
221+
replicationContainerTest(
222+
"never acknowledges an older commit LSN after a keepalive acknowledgement",
223+
async ({ clickhouseContainer, redisOptions, postgresContainer, prisma }) => {
224+
const clickhouse = new ClickHouse({
225+
url: clickhouseContainer.getConnectionUrl(),
226+
name: "sessions-replication",
227+
logLevel: "warn",
228+
});
229+
230+
const slotName = "sessions_keepalive_ack_v1";
231+
const service = new SessionsReplicationService({
232+
clickhouseFactory: new TestReplicationClickhouseFactory(clickhouse),
233+
pgConnectionUrl: withShortWalSenderTimeout(postgresContainer.getConnectionUri()),
234+
serviceName: "sessions-replication",
235+
slotName,
236+
publicationName: "sessions_keepalive_ack_v1_publication",
237+
redisOptions,
238+
maxFlushConcurrency: 1,
239+
flushIntervalMs: 100,
240+
flushBatchSize: 1,
241+
leaderLockTimeoutMs: 5000,
242+
leaderLockExtendIntervalMs: 1000,
243+
logLevel: "warn",
244+
});
245+
246+
await service.start();
247+
try {
248+
const organization = await prisma.organization.create({
249+
data: { title: "test", slug: "test" },
250+
});
251+
const project = await prisma.project.create({
252+
data: {
253+
name: "test",
254+
slug: "test",
255+
organizationId: organization.id,
256+
externalRef: "test",
257+
},
258+
});
259+
const environment = await prisma.runtimeEnvironment.create({
260+
data: {
261+
slug: "test",
262+
type: "DEVELOPMENT",
263+
projectId: project.id,
264+
organizationId: organization.id,
265+
apiKey: "test",
266+
pkApiKey: "test",
267+
shortcode: "test",
268+
},
269+
});
270+
await prisma.session.create({
271+
data: {
272+
id: "session_test_keepalive_ack",
273+
friendlyId: "session_keepalive_ack",
274+
type: "chat.agent",
275+
projectId: project.id,
276+
runtimeEnvironmentId: environment.id,
277+
environmentType: "DEVELOPMENT",
278+
organizationId: organization.id,
279+
taskIdentifier: "my-agent",
280+
triggerConfig: { basePayload: { messages: [], trigger: "preload" } },
281+
},
282+
});
283+
284+
await expectFlushLsnHoldsPastIdleWal(prisma, slotName);
285+
} finally {
286+
await service.stop();
287+
}
288+
}
289+
);
215290
});

‎apps/webapp/test/utils/replicationUtils.ts‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,47 @@ import type { RedisOptions } from "@internal/redis";
33
import type { PrismaClient } from "~/db.server";
44
import { RunsReplicationService } from "~/services/runsReplicationService.server";
55
import { TestReplicationClickhouseFactory } from "./testReplicationClickhouseFactory";
6-
import { afterEach } from "vitest";
6+
import { afterEach, expect } from "vitest";
7+
import { setTimeout } from "node:timers/promises";
8+
9+
// Keepalives with shouldRespond every 1s instead of 30s, on this connection's walsender only.
10+
export function withShortWalSenderTimeout(pgConnectionUrl: string) {
11+
const url = new URL(pgConnectionUrl);
12+
url.searchParams.set("options", "-c wal_sender_timeout=2s");
13+
return url.toString();
14+
}
15+
16+
// Writes WAL that only keepalives report, then asserts flush_lsn reaches it and never falls back.
17+
export async function expectFlushLsnHoldsPastIdleWal(prisma: PrismaClient, slotName: string) {
18+
const [{ lsn: target }] = await prisma.$queryRaw<{ lsn: string }[]>`
19+
SELECT pg_logical_emit_message(false, 'replication-ack-test', 'idle')::text AS lsn`;
20+
21+
const flushAhead = async () => {
22+
const rows = await prisma.$queryRaw<{ ahead: number }[]>`
23+
SELECT (r.flush_lsn - ${target}::pg_lsn)::float8 AS ahead
24+
FROM pg_stat_replication r
25+
JOIN pg_replication_slots s ON s.active_pid = r.pid
26+
WHERE s.slot_name = ${slotName}`;
27+
return rows[0]?.ahead ?? null;
28+
};
29+
30+
const deadline = performance.now() + 15_000;
31+
let ahead = await flushAhead();
32+
while ((ahead === null || ahead < 0) && performance.now() < deadline) {
33+
await setTimeout(50);
34+
ahead = await flushAhead();
35+
}
36+
expect(ahead, `flush_lsn never reached keepalive position ${target}`).toBeGreaterThanOrEqual(0);
37+
38+
const samples: Array<number | null> = [];
39+
const observeUntil = performance.now() + 3_000;
40+
while (performance.now() < observeUntil) {
41+
samples.push(await flushAhead());
42+
await setTimeout(50);
43+
}
44+
const regressed = samples.filter((s) => s === null || s < 0);
45+
expect(regressed, `flush_lsn moved back below ${target}`).toEqual([]);
46+
}
747

848
export async function setupClickhouseReplication({
949
prisma,

‎apps/webapp/test/webhookDeliveriesReplicationService.test.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ import type { PrismaClient } from "@trigger.dev/database";
44
import { setTimeout } from "node:timers/promises";
55
import { z } from "zod";
66
import { WebhookDeliveriesReplicationService } from "~/services/webhookDeliveriesReplicationService.server";
7+
import {
8+
expectFlushLsnHoldsPastIdleWal,
9+
withShortWalSenderTimeout,
10+
} from "./utils/replicationUtils";
711
import { TestReplicationClickhouseFactory } from "./utils/testReplicationClickhouseFactory";
812

913
vi.setConfig({ testTimeout: 60_000 });
@@ -220,4 +224,29 @@ describe("WebhookDeliveriesReplicationService", () => {
220224
}
221225
}
222226
);
227+
228+
// Catches: the 1s ack loop re-acking an older commit LSN after a keepalive ack (flush_lsn regresses).
229+
replicationContainerTest(
230+
"never acknowledges an older commit LSN after a keepalive acknowledgement",
231+
async ({ clickhouseContainer, redisOptions, postgresContainer, prisma }) => {
232+
const clickhouse = new ClickHouse({
233+
url: clickhouseContainer.getConnectionUrl(),
234+
name: "webhook-deliveries-replication",
235+
logLevel: "warn",
236+
});
237+
const service = buildService(
238+
clickhouse,
239+
withShortWalSenderTimeout(postgresContainer.getConnectionUri()),
240+
redisOptions
241+
);
242+
await service.start();
243+
244+
try {
245+
await createEndpoint(prisma, await seedEnvironment(prisma));
246+
await expectFlushLsnHoldsPastIdleWal(prisma, "webhook_deliveries_to_clickhouse_v1");
247+
} finally {
248+
await service.stop();
249+
}
250+
}
251+
);
223252
});

0 commit comments

Comments
 (0)