Skip to content

Clean up the replication tests (part of #3302) - #6714

Open
Brar wants to merge 3 commits into
npgsql:mainfrom
Brar:repl/tests-3302
Open

Brar wants to merge 3 commits into
npgsql:mainfrom
Brar:repl/tests-3302

Conversation

@Brar

@Brar Brar commented Oct 25, 2026

Copy link
Copy Markdown
Member

Depends on #6689 and #6710, which aren't merged yet; their commits show up here until they are.

Test code and the devcontainer only.

  • The Stream, TextReader, ValueMetadata, Null and CompositeType pgoutput tests never disposed their replication connections, so every fixture leaked five walsenders until the test process exited, which eats into max_wal_senders.
  • The pgoutput tests that used the same object names in all fixtures now use per-fixture names like the others, so that a test failing in one fixture can't affect the same test in the next one. Names too long for an identifier are now derived from a hash instead of a random GUID, so that the cleanup of the next run still finds what a killed run left behind.
  • Tests that prepare a transaction name it like their slot, so that one a killed test left behind can be found and rolled back: by every logical fixture before its tests, and by the test's own cleanup. Creating a logical slot waits for every transaction in the cluster that holds an xid, so the next run hung at its first logical slot creation, long before the owning test, and the transaction's locks made the cleanup wait forever. TwoPhase is skipped when max_prepared_transactions is 0, like the distributed transaction tests.
  • New TwoPhase_option: no test sent pgoutput's two_phase option. It enables decoding prepared transactions, and the server enables two-phase decoding for the slot itself.
  • New Stream_abort: PostgreSQL 18 skips transactions that are already aborted when it would stream them, so the existing assertions never saw a stream abort there. The new test rolls back after part of the transaction was streamed.
  • Replication_survives_pauses_longer_than_wal_sender_timeout no longer fails with negative intervals when wal_sender_timeout is disabled; it is skipped then, except on the build server.
  • The devcontainer database gets the replication settings CI uses and a pg_hba entry for physical replication.

Issue #3302 stays open: its remaining points come with later replication work.

Tests

  • The leftover prepared transaction, on PostgreSQL 18.6 with CI's settings and a leftover left on purpose: before this change, the next run hung at its first logical slot creation until the 60-s hang timeout; with it, the run passed with the same outcomes as a control run without a leftover (also on 15.19).
  • On this branch, live on 18.6: PgOutputReplicationTests 208/208 and CommonReplicationTests 36/36. Earlier cross-version runs: Stream_abort on 14 (protocol V2) and on 17 (all four streaming fixtures), TwoPhase_option on 17 (V3 and V4).

For the reviewer

  • The sweep covers the current database and only gids that match the tests' slot names (^s_[a-z0-9_]+_pgoutput_l$), and it relies on the logical fixtures not running in parallel, as now.

Dependencies and merge order

Part of #3302

Brar and others added 3 commits October 25, 2026 13:26
PgServerMock can now play a walsender: CopyBothResponse, CopyData,
XLogData and keepalive messages, CopyDone, CopyOutResponse and
NoticeResponse writers, NULL DataRow columns, a configurable
server_version, the parsed startup parameters (e.g. "replication") and
Expect helpers for CopyDone, Terminate and standby status updates.

This also fixes length bugs that were hidden because the client parses
these messages by content (CopyInResponse, ParameterDescription and
ErrorResponse sent wrong lengths), and ExpectMessage skipped
synchronously without ensuring the message was fully received.

ReplicationMock opens replication connections against the mock with the
feedback timers disabled, and ReplicationMockTests smoke-tests startup,
IDENTIFY_SYSTEM, physical streaming, keepalive replies and cancellation,
with timeouts on both the client and the mock side.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A slot with two-phase decoding enabled (PostgreSQL 15+) sends prepared
  transaction messages (b/P/K/r, and p when streaming) whatever the
  requested protocol version, but the message instances were only
  allocated for protocol version 3 and later, so protocol version 1
  and 2 sessions failed with a NullReferenceException. The same happened
  with protocol version 1 for stream aborts, which PostgreSQL 18.0 to at
  least 18.6 can send even without streaming and with any protocol
  version (bug #19616). All message instances are now allocated.
- Protocol version 4 sessions read every stream abort as if it carried
  the abort LSN and timestamp, which the server only sends with
  streaming = parallel. With streaming = on, and with streaming off when
  PostgreSQL 18 sent a stream abort anyway (bug #19616), this misparsed
  the stream. The layout is now chosen by the message length.
- GIDs of prepared transactions were read with synchronous I/O when they
  didn't fit into the buffered data, blocking a thread.
- Unknown message codes, unknown tuple types, unknown column kinds and
  changes for relations without a preceding Relation message now break
  the connection, like PostgreSQL's own apply worker errors out. Before,
  an Insert with an unknown tuple type was read as a new row, and in the
  other cases the exception left the connection in the Replication
  state, since nothing disposed the suspended replication enumerator:
  later commands on it threw NpgsqlOperationInProgressException, and the
  walsender kept the slot active until the connection was disposed. The
  errors for Delete and Truncate messages no longer say that they
  happened in an Update message. ReplicationConnection.DisposeAsync now
  also cleans up a replication enumerator that was left suspended when
  the connection broke.
- Reading a ReplicationValue after it had been consumed (e.g. the last
  column after the column enumeration ended) read whatever came next on
  the wire; it now throws InvalidOperationException, like
  TupleEnumerator.Current before the first MoveNextAsync. Reading a NULL
  as a nullable type no longer fails in text mode.
- The linked CancellationTokenSource created for each pgoutput and
  test_decoding enumeration is now disposed.
- PgOutputReplicationOptions.Equals and GetHashCode compared the list of
  publication names by reference; they now compare the names.

The pgoutput tests get a protocol version 4 fixture with streaming = on,
and the two-phase test now also runs with protocol versions 1 and 2.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The Stream, TextReader, ValueMetadata, Null and CompositeType pgoutput
  tests never disposed their replication connections, so every fixture
  leaked five walsenders until the test process exited, which eats into
  max_wal_senders.
- The pgoutput tests that used the same object names in all fixtures now
  use per-fixture names like the others, so that a test failing in one
  fixture can't affect the same test in the next one. Names too long for
  an identifier are now derived from a hash instead of a random GUID, so
  that the cleanup of the next run still finds what a killed run left
  behind.
- Tests that prepare a transaction name it like their slot, so that one
  a killed test left behind can be found and rolled back: by every
  logical fixture before its tests, and by the test's own cleanup.
  Creating a logical slot waits for every transaction in the cluster
  that holds an xid, so the next run hung at its first logical slot
  creation, long before the owning test, and the transaction's locks
  made the cleanup wait forever. TwoPhase is skipped when
  max_prepared_transactions is 0, like the distributed transaction
  tests.
- New TwoPhase_option test: no test sent pgoutput's two_phase option.
  It enables decoding prepared transactions, and the server enables
  two-phase decoding for the slot itself.
- New Stream_abort test: PostgreSQL 18 skips transactions that are
  already aborted when it would stream them, so the existing assertions
  never saw a stream abort there. The new test rolls back after part of
  the transaction was streamed.
- Replication_survives_pauses_longer_than_wal_sender_timeout no longer
  fails with negative intervals when wal_sender_timeout is disabled; it
  is skipped then, except on the build server.
- The devcontainer database gets the replication settings CI uses and a
  pg_hba entry for physical replication.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Brar
Brar requested review from roji and vonzshik as code owners October 25, 2026 21:39

This branch has not been deployed

No deployments
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