Skip to content

Commit c9f427e

Browse files
authored
fix(replication): key logical replication leader lock on slot name (#4151)
## Problem `LogicalReplicationClient` uses a Redlock leader lock to guarantee a single active consumer per Postgres logical replication slot. The lock resource was keyed on the client `name`: ``` logical-replication-client:${this.options.name} ``` A slot permits exactly one consumer, so the lock's job is to serialize consumers **of a given slot**. Keying it on `name` breaks that whenever two clients target the same slot with different names — most notably across a rolling deploy where the client `name` changes but `slotName` does not. Both acquire *distinct* locks, both consider themselves leader, and the second to reach `START_REPLICATION` hits `replication slot "<slot>" is active for PID <n>`. Because that query was fire-and-forget and its failure was only logged (no retry), the consumer stopped and replication stalled until the process was restarted. ## Fix **1. Key the leader lock on `slotName`** — the actual single-consumer resource: ``` logical-replication-client:${this.options.slotName} ``` Consumers of the same slot now contend on the same lock and hand off cleanly across restarts/deploys; different slots stay independent. `name` is kept for logging and the pg `application_name`. **2. Self-healing resubscribe** (`resubscribeOnFailure`, opt-in) — instead of logging-and-dying, a client re-subscribes with exponential backoff after a lost election or a failed `START_REPLICATION`, so a rolling deploy self-heals: the incoming pod retries until the draining pod releases the slot, then takes over. Safety: - `#cleanupAttempt()` unconditionally ends the pg client (freeing the walsender) and releases the leader lock before rescheduling — retries never leak connections/locks. - `shutdown()` sets an intentional-stop latch re-checked after every `await` in `subscribe()` (and aborts the lock-acquire spin), so a resubscribe can never race or outlive an intentional shutdown. - Backoff resets only on genuine stream start, so a permanently stuck slot backs off to the ceiling and logs loudly rather than tight-looping; an epoch guard neutralises stale `START_REPLICATION` catches. Runs- and sessions-replication opt in and use `shutdown()` for all intentional stops. **3. Observability** — the admin runs-replication status route probed the old name-keyed Redis key (would report `leader:false` for every source after fix #1); now probes the slot-keyed key. ## Tests `internal-packages/replication/src/client.test.ts` (real Postgres + Redis containers): - same-slot/different-name → second client must not double-lead or race into "slot is active" (the regression) - a failing `START_REPLICATION` retry loop must not leak connections or locks - `shutdown()` during an in-flight `subscribe()` must not leave a zombie leader - `subscribe()` after `shutdown()` re-arms `resubscribeOnFailure` - self-heals once the leader releases the slot Plus the multi-source wiring test updated to the slot-keyed lock keys. ## Rollout With the self-healing resubscribe, this ships as a **plain rolling deploy** — the incoming pods retry across the one-time lock-key transition and take over once the old pods drain (a brief replication stall that the durable slot replays on reconnect — no data loss). No stop-before-start required.
1 parent 70bca82 commit c9f427e

7 files changed

Lines changed: 635 additions & 146 deletions

File tree

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+
Key the logical-replication leader lock on the slot name (not the client name) so consumers of the same replication slot serialize correctly across restarts and rolling deploys

apps/webapp/app/routes/admin.api.v1.runs-replication.status.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,15 @@ import { getRunsReplicationConfiguredSources } from "~/services/runsReplicationG
77
/**
88
* Probes per-source replication leadership via the redlock leader-lock key, which
99
* is DOUBLE-PREFIXED with `logical-replication-client:` — once from the connection's
10-
* keyPrefix and once from redlock's resource string. So we prefix this connection
11-
* with `runs-replication:logical-replication-client:` and EXISTS on the resource
12-
* `logical-replication-client:runs-replication:<id>`, resolving to:
13-
* runs-replication:logical-replication-client:logical-replication-client:runs-replication:<id>
10+
* keyPrefix and once from redlock's resource string. The lock is keyed on the
11+
* replication slot, so we prefix this connection with
12+
* `runs-replication:logical-replication-client:` and EXISTS on the resource
13+
* `logical-replication-client:<slotName>`, resolving to:
14+
* runs-replication:logical-replication-client:logical-replication-client:<slotName>
1415
*/
15-
async function probeLeadership(sourceIds: string[]): Promise<Map<string, boolean>> {
16+
async function probeLeadership(
17+
sources: { id: string; slotName: string }[]
18+
): Promise<Map<string, boolean>> {
1619
const leaders = new Map<string, boolean>();
1720

1821
const redis = new Redis({
@@ -26,9 +29,9 @@ async function probeLeadership(sourceIds: string[]): Promise<Map<string, boolean
2629
});
2730

2831
try {
29-
for (const id of sourceIds) {
30-
const exists = await redis.exists(`logical-replication-client:runs-replication:${id}`);
31-
leaders.set(id, exists === 1);
32+
for (const source of sources) {
33+
const exists = await redis.exists(`logical-replication-client:${source.slotName}`);
34+
leaders.set(source.id, exists === 1);
3235
}
3336
} finally {
3437
await redis.quit();
@@ -46,7 +49,7 @@ export async function loader({ request }: LoaderFunctionArgs) {
4649
return json({ enabled: false, sources: [] });
4750
}
4851

49-
const leaders = await probeLeadership(sources.map((s) => s.id));
52+
const leaders = await probeLeadership(sources);
5053

5154
return json({
5255
enabled: env.RUN_REPLICATION_ENABLED === "1" && sources.length > 0,

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@ export class RunsReplicationService {
286286
table: "TaskRun",
287287
redisOptions: options.redisOptions,
288288
autoAcknowledge: false,
289+
resubscribeOnFailure: true,
289290
publicationActions: ["insert", "update", "delete"],
290291
logger:
291292
options.logger ?? new Logger("LogicalReplicationClient", options.logLevel ?? "info"),
@@ -428,7 +429,9 @@ export class RunsReplicationService {
428429

429430
if (!hasCurrentTransaction) {
430431
this.logger.info("No transaction to commit, shutting down immediately");
431-
await Promise.all(Array.from(this._sources.values()).map((runtime) => runtime.client.stop()));
432+
await Promise.all(
433+
Array.from(this._sources.values()).map((runtime) => runtime.client.shutdown())
434+
);
432435
this._isShutDownComplete = true;
433436
return;
434437
}
@@ -458,7 +461,7 @@ export class RunsReplicationService {
458461
for (const runtime of this._sources.values()) {
459462
this.logger.info("Stopping replication client", { sourceId: runtime.source.id });
460463

461-
await runtime.client.stop();
464+
await runtime.client.shutdown();
462465

463466
if (runtime.acknowledgeInterval) {
464467
clearInterval(runtime.acknowledgeInterval);
@@ -636,7 +639,7 @@ export class RunsReplicationService {
636639
// swallow client.stop() rejections so they don't surface as unhandled.
637640
if (!this._shutdownStopInFlight) {
638641
this._shutdownStopInFlight = true;
639-
Promise.all(Array.from(this._sources.values()).map((r) => r.client.stop()))
642+
Promise.all(Array.from(this._sources.values()).map((r) => r.client.shutdown()))
640643
.catch((error) => {
641644
this.logger.error("Error stopping replication clients during shutdown", { error });
642645
})

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ export class SessionsReplicationService {
187187
table: "Session",
188188
redisOptions: options.redisOptions,
189189
autoAcknowledge: false,
190+
resubscribeOnFailure: true,
190191
publicationActions: ["insert", "update", "delete"],
191192
logger: options.logger ?? new Logger("LogicalReplicationClient", options.logLevel ?? "info"),
192193
leaderLockTimeoutMs: options.leaderLockTimeoutMs ?? 30_000,
@@ -265,7 +266,7 @@ export class SessionsReplicationService {
265266

266267
if (!this._currentTransaction) {
267268
this.logger.info("No transaction to commit, shutting down immediately");
268-
await this._replicationClient.stop();
269+
await this._replicationClient.shutdown();
269270
this._isSubscribed = false;
270271
this._isShutDownComplete = true;
271272
return;
@@ -294,7 +295,7 @@ export class SessionsReplicationService {
294295
async stop() {
295296
this.logger.info("Stopping replication client");
296297

297-
await this._replicationClient.stop();
298+
await this._replicationClient.shutdown();
298299

299300
if (this._acknowledgeInterval) {
300301
clearInterval(this._acknowledgeInterval);
@@ -430,10 +431,15 @@ export class SessionsReplicationService {
430431
if (this._isShutDownComplete) return;
431432

432433
if (this._isShuttingDown) {
433-
this._replicationClient.stop().finally(() => {
434-
this._isSubscribed = false;
435-
this._isShutDownComplete = true;
436-
});
434+
this._replicationClient
435+
.shutdown()
436+
.catch((error) => {
437+
this.logger.error("Error stopping replication client during shutdown", { error });
438+
})
439+
.finally(() => {
440+
this._isSubscribed = false;
441+
this._isShutDownComplete = true;
442+
});
437443
}
438444

439445
// If there are no events, do nothing

apps/webapp/test/runsReplicationInstance.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -408,10 +408,12 @@ describe("RunsReplication multi-source wiring (integration)", () => {
408408

409409
probe = new Redis(redisOptions);
410410

411+
// Leader lock is keyed on the slot, so each source holds a distinct
412+
// slot-keyed lock (double-prefixed: connection keyPrefix + redlock resource).
411413
const legacyKey =
412-
"runs-replication:logical-replication-client:logical-replication-client:runs-replication:legacy";
414+
"runs-replication:logical-replication-client:logical-replication-client:tr_legacy_wiring";
413415
const newKey =
414-
"runs-replication:logical-replication-client:logical-replication-client:runs-replication:new";
416+
"runs-replication:logical-replication-client:logical-replication-client:tr_new_wiring";
415417

416418
expect(await probe.exists(legacyKey)).toBe(1);
417419
expect(await probe.exists(newKey)).toBe(1);

0 commit comments

Comments
 (0)