Skip to content

dbconn: have the replication connection ask for warnings itself - #154

Draft
Kiran01bm wants to merge 1 commit into
mainfrom
kiran01bm/cs7-replication-warnings
Draft

Kiran01bm wants to merge 1 commit into
mainfrom
kiran01bm/cs7-replication-warnings

Conversation

@Kiran01bm

Copy link
Copy Markdown
Collaborator

Why

Follow-up to #151. The stream stops fail-closed on a WARNING from the walsender — from PostgreSQL 18 that is how a publication skipped at load time is reported (55000), and without the stop a keepalive would move Delivered, and the caller's Confirm the slot, past a change the slot will never resend. That stop only works if the warning reaches the connection. A walsender filters its messages through client_min_messages like any backend, and ConnectReplication inherited whatever the role, the database, or the server set. A database set to error to quiet noisy clients silenced the warning, and on 18 the silent skip came back: the change never arrived, Delivered moved past its commit, and Confirm moved the slot past it too. Before 18 the server sends an ERROR for the missing publication, which no setting filters, so 14 through 17 already failed closed.

Before / after

before                                              after
┌──────────────┐ SET client_min_messages = error    ┌──────────────┐ startup: client_min_messages = warning
│ role / db /  │──────────────┐                     │ replication  │ (outranks role, database, server)
│ server       │              ▼                     │ connection   │──────────────┐
└──────────────┘      ┌───────────────┐             └──────────────┘              ▼
                      │ walsender     │ drops the                        ┌───────────────┐
  DROP PUBLICATION ──▶│ skips loading │ WARNING          DROP PUBLICATION│ walsender     │ WARNING 55000
  + INSERT            │ publication   │ ──────▶ nothing  + INSERT     ──▶│ skips loading │ ──────▶ stream
                      └───────────────┘                                  └───────────────┘
                              │ keepalive                                        │
                              ▼                                                  ▼
             Delivered → past the change                           ErrInvariantViolation (ST-4)
             Confirm   → slot past the change                      Confirm refuses

What

  • pkg/dbconn/replication.go — ConnectReplication sets client_min_messages = warning as a startup parameter alongside replication = database. A startup parameter outranks the role's, the database's, and the server's setting, so none of them can keep the walsender's warning from the connection.
  • pkg/dbconn/replication_integration_test.go — TestConnectReplicationAsksForWarningsOverTheDatabaseSetting: with the database set to error, SHOW client_min_messages on the replication connection reads warning.
  • pkg/decode/stream_refusal_integration_test.go — TestStreamStopsOnTheWarningWhenTheDatabaseSendsOnlyErrors: ALTER DATABASE … SET client_min_messages = error before the stream opens, then the dropped-publication scenario; the stream must still end with the server's SQLSTATE on every major. The dropped-publication steps and the per-major expectation move into two fixture helpers shared with TestStreamReturnsTheDecodersError.
  • pkg/decode/stream_test.go — TestStreamStopsOnALocalizedWarning: a notice with Severity: "WARNUNG" and SeverityUnlocalized: "WARNING" stops the stream, pinning the check to the unlocalized field.
  • Docs — SAFETY.md decode row, design decode row, and the ST-4 Enforced line record that the replication connection asks for warnings itself; the ST-4 test list names the two new tests.

Decisions to veto

  • A startup parameter rather than a SET after connect: a walsender accepts only the simple query protocol and ConnectReplication runs no session preparation, and the startup parameter is also what outranks the role and database settings. It overrides a value in the connection URL as well, since it is assigned after parsing.
  • warning, not notice or lower: the walsender says what it will withhold at WARNING; everything below stays informational and the stream ignores it, as before.

Verification

  • Mutants on PostgreSQL 18: without the startup parameter, TestStreamStopsOnTheWarningWhenTheDatabaseSendsOnlyErrors times out at the stream deadline (17.1 s, the stream did not fail before the stream deadline) and TestConnectReplicationAsksForWarningsOverTheDatabaseSetting reads error; with it both pass (5.5 s / 1.8 s). Reading the localized Severity instead of SeverityUnlocalized fails TestStreamStopsOnALocalizedWarning.
  • go test -race -count=1 ./pkg/decode/ ./pkg/dbconn/ green on PostgreSQL 16 (decode 152 s, dbconn 95 s) and with PG_VERSION=18 on 18 (decode 138 s, dbconn 80 s).
  • make lint 0 issues; SKIP_INTEGRATION=1 go test ./pkg/decode/ ./pkg/dbconn/ ./internal/safety/ ./pkg/schemachange/ green (docs guards included).

🤖 Created by Kiran's coding agent (Amp, Claude Opus 4.6).

A walsender reports what it will withhold from the stream as a warning,
and filters it through client_min_messages like any backend. A role,
database, or server set to send only errors kept that warning from the
stream, so from PostgreSQL 18 a publication dropped under the stream
was skipped silently and a keepalive confirmed past the change. The
replication connection now sets client_min_messages = warning as a
startup parameter, which outranks every one of those settings.

Also pins the unlocalized severity in the stream's warning check with a
localized warning.
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