Skip to content

Fix/medium level improvements#1

Open
deepshekhardas wants to merge 6 commits into
mainfrom
fix/medium-level-improvements
Open

Fix/medium level improvements#1
deepshekhardas wants to merge 6 commits into
mainfrom
fix/medium-level-improvements

Conversation

@deepshekhardas

Copy link
Copy Markdown
Owner

Closes #

✅ Checklist

  • I have followed every step in the contributing guide
  • The PR title follows the convention.
  • I ran and tested the code works

Testing

[Describe the steps you took to test this change]


Changelog

[Short description of what has changed]


Screenshots

[Screenshots]

💯

deepshekhardas added 6 commits February 14, 2026 07:14
triggerdotdev#2796)

When a user removes the machine configuration from a task and redeploys, task.machine becomes undefined. Prisma's create() silently skips undefined fields for Json columns rather than setting them to NULL. This change uses the nullish coalescing operator to explicitly pass null, ensuring the machineConfig column is cleared in the database.
…undWorkerTask create

Applied the fix pattern to ensure retryConfig, queueConfig, and payloadSchema are also explicitly cleared when removed from task definition, as suggested in PR feedback.
Efficiently select and map batch relation in AttemptForExecutionGetPayload and _executionFromAttempt to restore batch context during dequeue. Part of Legend Rank mission.
Refactor enhanceExecutionSnapshotWithWaitpoints to use an Index Map for O(N+M) complexity, replacing a quadratic nested loop. Improves performance for runs with large numbers of waitpoints. Part of Mythic Rank mission.
- Feat: Expose more trigger options in MCP trigger-task tool
- Refactor: Remove deprecated 'id' field from SCHEDULE_ATTEMPT message
- Test: Add ResourceMonitor unit tests for memory scaling
- Fix: Handle processKeepAlive in runTimelineMetrics to prevent stale fork metrics
deepshekhardas pushed a commit that referenced this pull request Jul 11, 2026
…riggerdotdev#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant