Repository navigation
Add a throwaway primary/standby cluster for replication tests - #6690
Conversation
PgTestCluster creates PostgreSQL clusters in temporary directories with the binaries that NPGSQL_TEST_PG_BIN points to (initdb, pg_ctl, pg_basebackup): a primary that allows physical and logical replication, streaming standbys of it, promotion and shutdown in any mode. This covers tests that can't run against the shared test server, like timeline switches and server shutdowns. Tests using it are ignored when the variable isn't set, so CI is unaffected. The clusters listen on 127.0.0.1 only (no Unix sockets, whose default directory isn't writable for unprivileged users on Debian-based builds), pick a free port right before each start, retry when another process took it, and limit every step to well below CI's 30 seconds per test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
roji
left a comment
There was a problem hiding this comment.
Recommendation
The direction is justified, but fix the primary restart behavior and strengthen the smoke test before merging. Keep this as a small test helper rather than a general cluster-management framework.
Problem and expected approach
The existing TimelineHistory test is explicit because a freshly initialized shared server has no timeline history. Promotion and shutdown tests genuinely need an isolated server. I would expect one internal C# helper, roughly a couple hundred test-only lines, using PostgreSQL's own tools, with primary-to-standby replay, promotion, and cleanup coverage. No shipped API or public documentation changes are needed; documenting the opt-in binary directory is sufficient.
Fit with that scope
The helper largely matches that approach: temporary data directories, native tools, non-pooled connections, bounded process execution, and opt-in execution. Its basic use is straightforward:
await using var primary = await PgTestCluster.StartPrimary();
await using var standby = await primary.StartStandby();
await standby.Promote();Gaps
The inline comments cover a reproduced correctness issue: Stop()/Start() changes the primary port without updating existing standbys, so replication does not resume. The smoke test also never checks replay; a disconnected standby can still report recovery and promote successfully.
This PR supplies infrastructure, not actual Npgsql replication coverage yet. A small follow-up should use it for the existing timeline test. My direct probe of IdentifySystem() followed by TimelineHistory(identity.Timeline) on the promoted standby threw InvalidCastException (String to Byte[]) at ReplicationConnection.cs:341. That cast already exists in the base commit: it demonstrates the coverage gap, not a product defect introduced by this PR, and should be fixed separately.
Manual validation
I compiled a separate console client against the unmodified helper and this checkout's Npgsql, using PostgreSQL 18.4. I created a primary and standby, inserted a row on the primary and observed it on the standby, reproduced the restart failure, promoted the standby to timeline 2, exercised the timeline API, restarted after immediate shutdown, and verified disposal removed the temporary cluster root. The added NUnit smoke test also passed when enabled and was skipped when the environment variable was absent. Local SDK constraints required an installed .NET 11 preview SDK; no repository files were changed.
Posted by copilot
roji
left a comment
There was a problem hiding this comment.
See agentic review comments, approving without deeper review as this is a test infra only change.
There was a problem hiding this comment.
🟡 Changes recommended
Restarting an upstream can change its port without updating existing standbys, preventing replication from resuming.
1 open issue
What changed in this PR
Adds an opt-in PostgreSQL cluster helper for replication tests that require isolated servers rather than the shared test database.
Changes:
- Creates temporary primaries and streaming standbys with startup, promotion, shutdown, and cleanup support.
- Adds process timeouts, loopback-only connections, and port-conflict retries.
- Adds a standby promotion smoke test, skipped when
NPGSQL_TEST_PG_BINis unset.
| File | Description |
|---|---|
| test/Npgsql.Tests/Support/PgTestCluster.cs | Implements temporary PostgreSQL cluster lifecycle management. |
| test/Npgsql.Tests/Replication/PgTestClusterTests.cs | Tests primary and standby recovery states, promotion, and shutdown. |
Review effort: Balanced
- A cluster keeps its port when it restarts. pg_basebackup writes the primary's port into the standby's primary_conninfo, so a primary that came back on another port lost its standbys. - The smoke test checks that the standby replays a row from the primary before it gets promoted, and a new test does the same after the primary restarted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PgTestCluster creates PostgreSQL clusters in temporary directories with the binaries that NPGSQL_TEST_PG_BIN points to (initdb, pg_ctl, pg_basebackup): a primary that allows physical and logical replication, streaming standbys of it, promotion and shutdown in any mode. This covers tests that can't run against the shared test server, like timeline switches and server shutdowns. Tests using it are ignored when the variable isn't set, so CI is unaffected.
The clusters listen on 127.0.0.1 only (no Unix sockets, whose default directory isn't writable for unprivileged users on Debian-based builds), pick a free port right before their first start, retry when another process took it, keep the port when they restart, since their standbys connect to it, and limit every step to well below CI's 30 seconds per test. The smoke tests check that a standby replays what the primary writes, also after the primary restarted.