Skip to content

SS-431 - PG source: implement a timeout for replication slot creation - #39605

Merged
martykulma merged 1 commit into
MaterializeInc:mainfrom
martykulma:maz-repl-slot-timeout
Oct 19, 2026
Merged

martykulma merged 1 commit into
MaterializeInc:mainfrom
martykulma:maz-repl-slot-timeout

Conversation

@martykulma

@martykulma martykulma commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

PostgreSQL slot creation isn't bound by a timeout, and can be blocked indefinitely. This has been observed for more than one user, generally caused by issues with the upstream DB or upstream clients. This uses PostgreSQL's lock_timeout as statement_timeout doesn't apply to creation of replication slots, and a client-side timeout would leave the CREATE command running on the upstream.

The timeout defaults to 1 minute, and is configurable via dyncfg pg_source_replication_slot_creation_timeout. For PG, 0 is disabled, so there was no need to wrap in Option. This clamps the timeout to i32::MAX, which is the PG's documented max value.

Test coverage added to validate the error and resumption.

Closes SS-431

@martykulma martykulma changed the title PG source: implement a timeout for replication slot creation SS-431 - PG source: implement a timeout for replication slot creation Oct 6, 2026
@martykulma
martykulma marked this pull request as ready for review October 7, 2026 00:29
@martykulma
martykulma requested review from a team as code owners October 7, 2026 00:29
@def-

def- commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review

1. MEDIUM -- Failing the attempt after lock_timeout throws away the wait, so upstreams with overlapping long write transactions never get a slot

src/storage/src/source/postgres.rs:486

Each timeout aborts CREATE_REPLICATION_SLOT. The source then restarts after 5 s, and the new attempt rebuilds its snapshot from a new xl_running_xacts record, so time already spent waiting is lost. If the upstream always has some write transaction with more than 60 s left, every attempt times out and the source stays stalled. Before this change, creation finished once the transactions in those records drained, about twice the length of the longest one.

Details

Reproduced on PostgreSQL 16 at 1/30 scale. Two sessions loop BEGIN; INSERT ...; SELECT pg_sleep(4); COMMIT;, staggered 2 s apart. With SET lock_timeout = 2000 before CREATE_REPLICATION_SLOT ... NOEXPORT_SNAPSHOT, 20 of 20 attempts failed with canceling statement due to lock timeout. The same command without the SET succeeded in about 5 s. Scaled up, this is two 2-minute batch writers offset by a minute.

The temporary snapshot slot (snapshot.rs:481) takes the same path. It runs for every new source and every CREATE TABLE ... FROM SOURCE, and the replication operator waits on all rewinds before streaming (replication.rs:345). So adding one table to an existing source on such an upstream stalls ingestion for all of its tables, and the stall no longer ends by itself. Only a change to the system-level dyncfg clears it, which users cannot make.

lock_timeout applies to each lock wait separately (XactLockTableWait per xid, in two snapshot-building phases). So the dyncfg description "Timeout for creating a replication slot" and the "timed out after 60s" message understate how long creation can actually take.

One way to keep both visibility and liveness: report the stall from a client-side timer while the command keeps waiting, instead of cancelling it. Another option is a default large enough that this is only a backstop for transactions that never end.

2. LOW -- The timeout never fires when the upstream is a physical standby

src/storage/src/source/postgres.rs:480

On a hot standby, the primary's transactions hold no xid locks. Logical slot creation there waits on them in a pg_usleep loop, not on a heavyweight lock, so lock_timeout does not apply. A source pointed at a standby, which is a supported setup with its own CI job, still hangs silently at slot creation.

Details

Checked against a PG 16 primary/standby pair. With a write transaction open on the primary and SET lock_timeout = 2000 on the standby, CREATE_REPLICATION_SLOT was still running after 30 s. pg_stat_activity showed the walsender active with no wait event. A second run returned the slot after 35 s, once the primary transaction committed. The comment at this line ("Slot creation blocks on the transaction locks of in-progress transactions, which lock_timeout bounds") is not true for this configuration.

@martykulma
martykulma force-pushed the maz-repl-slot-timeout branch from 88f863f to 226959b Compare October 7, 2026 10:00
@def-

def- commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review

1. MEDIUM -- When the cancel has no effect, every retry leaves another blocked walsender and slot on the upstream

src/storage/src/source/postgres.rs:535

If pg_cancel_backend does not stop the command, create_replication_slot gives up anyway and the connection is dropped. The walsender keeps waiting and holds its slot, and each source restart (about every 2 minutes with defaults) adds another. This can use up the upstream's max_wal_senders/max_replication_slots (default 10), so other replication clients of that server can't connect until the blocking transaction ends. Before this commit, the same upstream had a single waiting walsender.

Details

The cancel does nothing in at least two supported setups:

  • Standbys on PG 16.0–16.9 and 17.0–17.5. The wait loop in XactLockTableWait only checks for interrupts from 16.10/17.6 on (upstream commit "Make XactLockTableWait() and ConditionalXactLockTableWait() interruptable more"). CI runs 18.4, so it never hits this.
  • Neon, and any other proxy that sends its own BackendKeyData. Neon's proxy hands out a random backend PID, so pg_cancel_backend warns PID … is not a PostgreSQL backend process and returns f. cancel() ignores the boolean and reports success.

Any failure to open the cancel connection (:522) has the same result.

Reproduced on a PG 16.9 primary/standby pair, with a write transaction open on the primary, running Materialize's command sequence by hand. pg_cancel_backend returned t, but the command did not stop. After each dropped client, the walsender stayed, and slots accumulated as main, tmp2, tmp3, tmp4, one per attempt. On a primary, a dropped client's walsender likewise stays in Lock:transactionid with its slot. On 16.10 the cancel failed the command at once, as intended.

Do not drop the connection unless the command actually failed with QUERY_CANCELED. Otherwise keep awaiting it and report the stall while it waits. The protocol-level cancel (client.cancel_token()) also works through such proxies.

@martykulma
martykulma force-pushed the maz-repl-slot-timeout branch from 226959b to 396fdac Compare October 7, 2026 11:16
@martykulma

Copy link
Copy Markdown
Contributor Author
  1. MEDIUM -- Failing the attempt after lock_timeout throws away the wait, so upstreams with overlapping long write transactions never get a slot

If the upstream is suffering from this degenerate case, the ability to disable exists via the dyncfg.

  1. LOW -- The timeout never fires when the upstream is a physical standby

Shame. If this becomes a major headache, we can look at a solution. Attempts at using pg_cancel_backend are also flawed, but affect more configurations. Documented in the comments and dyncfg description.

  1. MEDIUM -- When the cancel has no effect, every retry leaves another blocked walsender and slot on the upstream

The pg_cancel_backend approach is dropped.

@martykulma
martykulma force-pushed the maz-repl-slot-timeout branch from 396fdac to 63427d7 Compare October 15, 2026 02:19
@martykulma
martykulma force-pushed the maz-repl-slot-timeout branch from 63427d7 to 523ee41 Compare October 19, 2026 20:23
@martykulma
martykulma merged commit 4e4ba3c into MaterializeInc:main Oct 19, 2026
83 checks passed
@bosconi bosconi mentioned this pull request Oct 23, 2026
val-materialize added a commit that referenced this pull request Oct 30, 2026
Release notes for the **v26.48.0** train. One commit is added per
snapshot; the `final` snapshot sets the real dates. **Applied: `rc.1`,
`rc.2`, `rc.3`, `final` — the train is complete.**

- **Content**: 3 Feature sections (upsert hydration; Kafka sink headers
GA; MySQL `EXCLUDE ALL NOT NULL CONSTRAINTS`), 9 Improvements, 2 Guides,
4 Bug Fixes. No **Agent Skills** section — every agent-skills PR in all
four windows was omitted.
- **Scope**: 123 PRs across the four snapshots — rc.1 114 (14 included),
rc.2 5 (0 included), rc.3 1 (1 included), final 3 (0 included).
- **Dates — now real, not provisional**: Cloud **2026-10-29**,
Self-Managed **2026-10-30**. The `rc.1`-anchored provisional estimate
was right, so `final` left both files unchanged. The `v26.48.0` tag's
commit date is 2026-10-28T18:35:05Z, a Wednesday; the first Thursday on
or after it is 2026-10-29, with Self-Managed the day after.
- **Operator compatibility**: row for `v26.48`, release date
`2026-10-30`. Unchanged by rc.2, rc.3, and final.
- **Branch base**: `main`, whose newest published section was `##
v26.46.1` when this branch was cut; v26.47.0 and v26.47.1 have since
merged in. `v26.48.0` stays at the top of the list.

### final increment — empty, and verified empty

`final`'s commit is empty. The tracking issue
([release#627](MaterializeInc/release#627))
lists exactly one item, the unassociated `4edd60a8d` "release: bump to
version v26.48.0", which the release-process-automation rule omits. The
three agent-skills PRs in the window
([#118](MaterializeInc/agent-skills#118),
[#119](MaterializeInc/agent-skills#119),
[#120](MaterializeInc/agent-skills#120)) were
all omitted.

This was **verified rather than assumed**, because a zero-PR snapshot
and a failed fetch produce identical output:
`compare/v26.48.0-rc.3...v26.48.0` returns `total_commits: 1`, and that
commit is the version bump. Nothing merged onto the release branch in
the 42 h 49 m between the `rc.3` tag and the ship tag. Across the train,
every agent-skills PR from
[#111](MaterializeInc/agent-skills#111) to
[#120](MaterializeInc/agent-skills#120) falls in
exactly one window, with no gaps or overlaps.

⚠️ **One decision is still open for the release owner — see "Upsert v2"
below.** Every flag this train held open was re-read in source at the
`v26.48.0` ship tag rather than inherited from the RC snapshots:

| Flag | Default at `v26.48.0` | Source | Held since | Disposition |
| -- | -- | -- | -- | -- |
| `enable_upsert_v2` | **`false`** |
`src/storage-types/src/dyncfgs.rs:505-509` | rc.2 | ⚠️ premise
contradicted — see below |
| `enable_upsert_chunked_stash` | **`false`** |
`src/storage-types/src/dyncfgs.rs:469-475` | rc.2 | same |
| `enable_upsert_paged_spill` | **`false`** |
`src/storage-types/src/dyncfgs.rs:450-455` | rc.2 | same |
| `enable_mcp_protocol_2026_07_28` | **`false`** |
`src/adapter-dyncfgs/src/dyncfgs.rs:295-299` | rc.1 | hold stands —
still omitted |
| `cluster-details-page-cns-173` | n/a (LaunchDarkly) | no source
default | rc.1 | hold stands — still omitted |

**The rc.2 hold on
[#40189](#40189) rests
on a premise this branch now contradicts.** rc.2 omitted it at
Borderline on exactly one ground — every measurement is taken on a path
`enable_upsert_v2` keeps off by default — and asked whether any
environment actually runs upsert v2. The branch answers that without
anyone feeding the answer back: the human-added **"Improved hydration
times and memory efficiency for upsert sources"** section states *"This
change has been rolled out to all Materialize Cloud environments and
applies to all existing and new clusters."* The three flags still read
`false` at the ship tag, which is exactly the
source-default-versus-LaunchDarkly divergence the rollout-lever rule
covers: the published docs call wins, and the sole stated reason for the
omit has fallen away.

- **Not already covered.** The published section is about **hydration**
— building a source from scratch, 206 min → 108–113 min, 542 GiB →
183–208 GiB. #40189 is about **catch-up after a backlog**, a different
operation with its own motivating incident
([CPU-320](https://linear.app/materializeinc/issue/CPU-320): a source
that *"could not catch up on any replica size after a producer rate
increase"*). A customer who read the hydration section would learn
nothing about catch-up.
- **Suggested Improvements bullet, to add only if you confirm** — it was
*not* applied in the `final` commit:
> **Faster catch-up for upsert sources with a large backlog**: An upsert
source working through a backlog now drains one timestamp per round
instead of merging and re-pushing the whole backlog each round, so
catch-up time grows linearly rather than quadratically with the backlog;
draining a 100-timestamp tail fell from 6.5 to 0.15 seconds, and a
400-timestamp tail drains in 0.67 seconds.
- **Caveat that must travel with the decision**: the baseline in those
timings is the *paged* flavor on `main`, not the configuration Cloud ran
before this rollout — so they are not a clean before/after for a given
customer. Dropping the two timings and keeping the
linear-versus-quadratic claim holds regardless of baseline.
- **Two weaker links, stated rather than buried**: that the rolled-out
configuration is the one #40189 was measured on is *inferred from the
published section's prose* (its "buffer pool … serve sequential work
from disk" reads as chunked stash plus paged spill), not confirmed by
anyone. And the author declared "Release notes: None", naming
`enable_upsert_v2` — so publishing this would be an **override** of an
author signal, justified by your own later rollout statement.
- **Third consecutive train** to hold this flag: `v26.44.0-rc.1`
([#39041](#39041)),
`v26.45.0-rc.1`
([#39044](#39044)),
`v26.48.0-rc.2` (#40189). If the rollout statement is accurate, the
earlier two are worth revisiting rather than left to age out.

**The other two holds stand, corroborated.**
`enable_mcp_protocol_2026_07_28` still reads `false` at the ship tag and
the five MCP PRs rc.1 held appear nowhere in the section; neither does
the Console cluster-details page stack. In both cases you had the whole
train in view and did not promote them — the confirmation rc.1 asked
for, arriving in the negative.

**No duplicates, re-checked at ship.** The seven PRs rc.1 flagged as
already covered by v26.47.0 are still absent, the `## v26.47.1` section
is untouched, and rc.3's entry
([#40065](#40065))
appears exactly once.

**Two notes on human edits to this branch**, both recorded as fact
rather than objection:

- `524adb8` removed the rc.1 feature section **"Native Google Cloud
Storage for persist"**
([#39983](#39983))
outright. Flagging it only because a published entry disappearing is
easy to miss in a long branch — if that was deliberate, nothing to do.
- Two rc.1 Improvements were promoted to their own feature sections with
docs links and a code sample (Kafka sink headers
[#39908](#39908);
MySQL `EXCLUDE ALL NOT NULL CONSTRAINTS`
[#39873](#39873)), and
a **Guides** section was added that is not in the skill's template. The
lesson for next time: an option whose flag flipped to `true` in the same
release more likely warrants a feature section than the Improvements
bullet the rubric suggests by default.

**Carried forward from rc.3 and still open**: the `K8s recovery: envd on
failing node` nightly — the job that sank
[#40029](#40029), the
first attempt at what #40065 re-lands — **was not checked**; it needs
Buildkite, outside this run. The entry itself was verified correct in
source at both the rc.3 and ship tags, so this is a CI-confidence
question, not a notes-accuracy one.

### rc.3 increment

One Improvement, for #40065: a read-only `environmentd` that reboots to
pick up DDL changes during zero-downtime upgrade catch-up now restarts
in place rather than exiting its container, so Kubernetes no longer
applies crash-restart backoff. Not inherited from rc.1 — rc.1 correctly
omitted the first attempt (#40029) together with its revert (#40061) as
a no-op on `main`; rc.3 re-lands the behavior in narrowed form, verified
as a real change by comparing `entrypoint.sh` at the `v26.47.0` tag (no
retry loop) against `v26.48.0-rc.3` (re-exec on exit 75). The draft's
closing "so zero-downtime upgrades catch up faster" was cut: no cited PR
measures an upgrade duration, so the entry claims only the mechanism.

- **Not a Bug Fix.** 0dt upgrades completed on v26.47.0; the catch-up
reboots merely collected backoff delay. Existing behavior improved, so
Improvements.
- **Residual, deliberately unpublished.** A stale read-only process in a
replaced pod still restarts in place on these halts until the kubelet
kills its container, competing with its replacement for the generation's
replicas. The `MZ_RESTARTED_IN_PLACE` marker check keeps it from booting
writable, so it cannot fence the leader — the failure #40061 reverted.
Worth watching if 0dt reports look odd after this ships.

### rc.2 increment — empty, by design

rc.2's commit is empty: no entry was published and the provisional dates
did not move. It exists so later runs can see from the commit subject
that rc.2 was applied. Its single materialize PR, #40189, is the open
item discussed under the `final` increment above.

### rc.1 — standing warnings

⚠️ **Seven PRs in rc.1's tracking issue were already covered by the
v26.47.0 train, and none is republished here.** v26.47.0 took five RCs
whose later ones were cherry-picks onto the release branch; the same PRs
appear in this issue because they are also what landed on `main` for
v26.48.0. Four of the seven **shipped to Cloud in v26.47.0 on
2026-10-22** — #37653, #39910, #39927, #40036 — so republishing any of
them would put duplicate text in front of customers eight days later.
The other three (#39903, #39906, #40046) were held at Borderline in
v26.47.0 and are held again here, each reason re-checked rather than
inherited. The "snapshots are disjoint by construction" guarantee holds
*within* a train, not across trains, so this was caught mechanically by
grepping every PR number in the issue against the v26.47.0 snapshot
directories. All seven are listed under their own heading in
`rc.1/omitted.md`.

**Open questions for the release owner**

- **Upsert v2** (#40189, rc.2) — see the `final` increment above. The
rollout statement on this branch has changed the picture since rc.2;
this is the one item that needs a decision before merge.
- **MCP protocol revision 2026-07-28** (#39698, #39702, #39703, #39759,
#39761) — the stack is complete and `enable_mcp_protocol_2026_07_28`
still defaults to **`false`** at the ship tag. Omitted, with the absence
of any MCP text on this branch taken as confirmation. Drafted wording
remains in `rc.1/omitted.md` if the flag flips.
- **Redesigned cluster details page in the Console** (#39446, #39507,
#39508, #39510, #39607) — omitted for the same reason. Note that the
Improvements section does carry **"New cluster details page on
Self-Managed"**, added by hand, which links the v26.47 section rather
than re-describing the page.
- **Schema projection on the Console's Freshness page** (#40031) — a
genuine new way to diagnose lag, but the author states the whole
Freshness page "is currently behind a feature flag that is only enabled
for certain customers in impersonation mode". **Is the page generally
reachable yet?** This is the same page v26.47.0 held for the same
reason.
- **Time zone data 2026e timing** (#40045) — the author flags the
Canadian zone changes as effective **2026-11-01**, three days after the
Cloud date. Tight, but it holds.

*Resolved since rc.1: the Kafka sink headers framing question — the GA
section on this branch answers it.*

**Verification notes** — rc.1's Step 5 pass cut one drafted entry and
rewrote four:

- **Cut**: a Bug Fix drafted for #40032 ("halt with exit code 168 when a
live reader loses its lease"). The PR's own Problem section is about
alerting — `mz_persist_lease_timeout_read` cannot distinguish a dead
reader from a stuck one — and the follow-up it names is a Cloud alerting
change. Nothing a customer observes changes.
- **tzdata** (#40045): the draft said the Canadian zones move to
"permanent daylight saving time", the author's prose. The PR's data
table shows a permanent **−05/EST** and **−06/CST** — summer *offsets*
kept year-round, but with another zone's standard-time *abbreviations*,
which is what a user sees from `AT TIME ZONE`. Reworded to what the
table supports.
- **Hydration memory** (#39923): scoped to the consolidation step and
the measured outcome (~628 GiB peak, no restart, against every
pre-change run exiting on the 849 GiB limit). The PR explicitly says the
fix "covers this consolidation path, not the sink's whole footprint";
[CPU-317](https://linear.app/materializeinc/issue/CPU-317) tracks the
rest.
- **Kafka clients** (#40026): "55 to 24" is measured on a 4-worker,
11-source setup. The CPU throttling and degraded freshness are observed
symptoms of the *old* behavior at a few hundred sources, not a measured
post-change result at that scale, so the entry states the count as the
outcome and the throttling as what the count drove.
- **`environmentd` accept backlog** (#40035): the author's hedge is kept
— "less likely to be restarted", not eliminated, because the PR says
"This makes the queue fill later, not never."

**Judgment calls a reviewer may want to revisit** — #39605 is filed as
an **Improvement**, not a Bug Fix: nothing was incorrect, an unbounded
operation gained a bound and a knob. #40101 is filed as a **Bug Fix**
rather than omitted under the internal-logging rule, because credentials
reaching the logs is a security defect with a user-visible remedy; only
credentials a user inlined rather than wrapped in a `SECRET` were
affected.

Snapshot drafts (with PR links) in mz-skills:
- rc.1:
[release_notes.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.48.0/rc.1/release_notes.md)
/
[omitted.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.48.0/rc.1/omitted.md)
- rc.2:
[release_notes.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.48.0/rc.2/release_notes.md)
/
[omitted.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.48.0/rc.2/omitted.md)
- rc.3:
[release_notes.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.48.0/rc.3/release_notes.md)
/
[omitted.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.48.0/rc.3/omitted.md)
- final:
[release_notes.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.48.0/final/release_notes.md)
/
[omitted.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.48.0/final/omitted.md)

**Borderline PRs omitted:** rc.1: 12 —
[omitted.md#borderline](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.48.0/rc.1/omitted.md#borderline),
rc.2: 1 —
[omitted.md#borderline](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.48.0/rc.2/omitted.md#borderline),
rc.3: 0, final: 0

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude <[email protected]>
Co-authored-by: Valerie Chiang <[email protected]>
Co-authored-by: Pranshu Maheshwari <[email protected]>
Co-authored-by: Pranshu Maheshwari <[email protected]>
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.

3 participants