Add read/write splitting to the pg module #2

2026-08-31T22:44:45+10:00

unkin-agent commented

2026-08-31 22:44:45 +10:00

Stacked on #1 — based on benvin/initial-pg, so the diff stays to the read/write splitting. Gitea retargets it to main when #1 merges.

Why

Services in the estate read far more than they write, and CloudNativePG already publishes a <cluster>-ro service pointing at the replicas that nothing uses. Every service that wants to use it has to grow its own second pool, its own fallback when the replica is down, and its own rule about which queries are safe to send there. That is the same copy-paste golib exists to absorb.

Routing has to be explicit rather than inferred from the SQL. Statement inspection gets it wrong in both directions: a CTE with an INSERT in it reads as a SELECT and would be sent to a read-only replica, and SELECT ... FOR UPDATE reads as a plain query but takes row locks a replica cannot grant. The caller knows which it wants, so the caller picks.

How

Add pg.Cluster, wrapping a primary and an optional replica *pgxpool.Pool. Write() always returns the primary; Read() returns the replica when one is configured and healthy, the primary otherwise; Primary() is the same pool as Write(), named for the read-your-writes case.
Run single-pool when ReplicaDSN is empty, or equal to PrimaryDSN, so a service can use Cluster unconditionally and the deployment decides whether reads split.
Track replica health with a ping-based circuit. A failed startup ping degrades rather than failing startup; ReportReplicaError(err) trips it; while tripped no probe is issued until the backoff window expires, and the next read then pays for one probe that either promotes the replica or doubles the window, from ReplicaRetryMin to ReplicaRetryMax. A healthy replica is never probed, so the split costs nothing on the read path.
Add pg.ClusterDSNsFromEnv(prefix), resolving the replica from <PREFIX>DATABASE_RO_URL, DATABASE_RO_URL, or <PREFIX>DB_RO_HOST/DB_RO_HOST substituted into the primary's other fields. Nothing is derived — the read-only host is never rewritten out of the primary's, and a host-only replica variable alongside a whole-URL primary is an error rather than a guess.
Give Cluster a Migrate that runs through Write(), so migrations cannot reach a replica.
Refactor DSNFromEnv's field resolution into connPartsFromEnv so both endpoints share it; the rendered primary DSN is unchanged.
Document replica lag and the Primary() escape hatch, the no-SQL-parsing rationale, the single-pool fallback, and the CNPG -rw/-ro env shape in the README.
Cover the routing table, the backoff and recovery behaviour under a fake clock, the ClusterDSNsFromEnv table, and a migration that must reach the primary while the replica is the pool Read() would hand out. Extend the container-backed integration test with one server standing in for both endpoints.

make cover reports 93.1% (gate 90%); gofmt, go vet, golangci-lint and pre-commit are clean.

Stacked on #1 — based on `benvin/initial-pg`, so the diff stays to the read/write splitting. Gitea retargets it to `main` when #1 merges. ## Why Services in the estate read far more than they write, and CloudNativePG already publishes a `<cluster>-ro` service pointing at the replicas that nothing uses. Every service that wants to use it has to grow its own second pool, its own fallback when the replica is down, and its own rule about which queries are safe to send there. That is the same copy-paste golib exists to absorb. Routing has to be explicit rather than inferred from the SQL. Statement inspection gets it wrong in both directions: a CTE with an `INSERT` in it reads as a `SELECT` and would be sent to a read-only replica, and `SELECT ... FOR UPDATE` reads as a plain query but takes row locks a replica cannot grant. The caller knows which it wants, so the caller picks. ## How - Add `pg.Cluster`, wrapping a primary and an optional replica `*pgxpool.Pool`. `Write()` always returns the primary; `Read()` returns the replica when one is configured and healthy, the primary otherwise; `Primary()` is the same pool as `Write()`, named for the read-your-writes case. - Run single-pool when `ReplicaDSN` is empty, or equal to `PrimaryDSN`, so a service can use `Cluster` unconditionally and the deployment decides whether reads split. - Track replica health with a ping-based circuit. A failed startup ping degrades rather than failing startup; `ReportReplicaError(err)` trips it; while tripped no probe is issued until the backoff window expires, and the next read then pays for one probe that either promotes the replica or doubles the window, from `ReplicaRetryMin` to `ReplicaRetryMax`. A healthy replica is never probed, so the split costs nothing on the read path. - Add `pg.ClusterDSNsFromEnv(prefix)`, resolving the replica from `<PREFIX>DATABASE_RO_URL`, `DATABASE_RO_URL`, or `<PREFIX>DB_RO_HOST`/`DB_RO_HOST` substituted into the primary's other fields. Nothing is derived — the read-only host is never rewritten out of the primary's, and a host-only replica variable alongside a whole-URL primary is an error rather than a guess. - Give `Cluster` a `Migrate` that runs through `Write()`, so migrations cannot reach a replica. - Refactor `DSNFromEnv`'s field resolution into `connPartsFromEnv` so both endpoints share it; the rendered primary DSN is unchanged. - Document replica lag and the `Primary()` escape hatch, the no-SQL-parsing rationale, the single-pool fallback, and the CNPG `-rw`/`-ro` env shape in the README. - Cover the routing table, the backoff and recovery behaviour under a fake clock, the `ClusterDSNsFromEnv` table, and a migration that must reach the primary while the replica is the pool `Read()` would hand out. Extend the container-backed integration test with one server standing in for both endpoints. `make cover` reports 93.1% (gate 90%); `gofmt`, `go vet`, `golangci-lint` and `pre-commit` are clean.

unkin-agent commented

2026-08-31 23:00:44 +10:00

Both review findings addressed in 0223391.

ClusterDSNsFromEnv now names the replica host variable that actually resolved. lookup falls back from the prefixed name to the bare one, but the ambiguous-replica error printed the prefixed form unconditionally, so APP_DATABASE_URL + bare DB_RO_HOST told the operator to look at APP_DB_RO_HOST, which was not in their environment. New lookupNamed returns the variable the value came from; TestClusterDSNsFromEnv_Errors gains the bare-var case and asserts the prefixed name does not appear.
NewCluster now builds the replica pool before the primary. pgxpool connects lazily so nothing is dialled, but the replica DSN is parsed, which means a bad one fails before any primary pool exists — the untestable primary.Close() cleanup branch is gone rather than merely asserted, and newCluster is infallible. TestNewCluster_RejectsAnUnparseableReplicaDSNBeforeOpeningThePrimary goes through the public constructor with an unreachable primary and asserts the replica error, not ping postgres, comes back. The mirror path (primary unreachable, replica pool already open) closes the replica and is exercised by TestNewCluster_PropagatesPrimaryFailure.

make cover 95.0% (min 90); gofmt, go vet, golangci-lint and pre-commit all clean.

Both review findings addressed in 0223391. 1. `ClusterDSNsFromEnv` now names the replica host variable that actually resolved. `lookup` falls back from the prefixed name to the bare one, but the ambiguous-replica error printed the prefixed form unconditionally, so `APP_DATABASE_URL` + bare `DB_RO_HOST` told the operator to look at `APP_DB_RO_HOST`, which was not in their environment. New `lookupNamed` returns the variable the value came from; `TestClusterDSNsFromEnv_Errors` gains the bare-var case and asserts the prefixed name does not appear. 2. `NewCluster` now builds the replica pool before the primary. pgxpool connects lazily so nothing is dialled, but the replica DSN is parsed, which means a bad one fails before any primary pool exists — the untestable `primary.Close()` cleanup branch is gone rather than merely asserted, and `newCluster` is infallible. `TestNewCluster_RejectsAnUnparseableReplicaDSNBeforeOpeningThePrimary` goes through the public constructor with an unreachable primary and asserts the replica error, not `ping postgres`, comes back. The mirror path (primary unreachable, replica pool already open) closes the replica and is exercised by `TestNewCluster_PropagatesPrimaryFailure`. `make cover` 95.0% (min 90); gofmt, go vet, golangci-lint and pre-commit all clean.

benvin changed target branch from benvin/initial-pg to main

2026-08-31 23:08:38 +10:00

benvin added 2 commits 2026-08-31 23:08:38 +10:00

Add read/write splitting to the pg module

ci/woodpecker/pr/build Pipeline was successful

Details

ci/woodpecker/pr/test Pipeline was successful

Details

ci/woodpecker/pr/pre-commit Pipeline was successful

Details

5c23db8885

pg.Cluster wraps a primary pool and an optional read-replica pool.
Routing is explicit — Read(), Write(), Primary() — with no SQL
inspection: statement text misroutes CTE writes and SELECT ... FOR
UPDATE in both directions.

An unhealthy replica falls back to the primary behind a ping-based
circuit that retries on a doubling backoff window, and migrations always
run through Write().

pg.ClusterDSNsFromEnv resolves both endpoints from the environment,
naming the read-only host explicitly rather than deriving it from the
primary's.

Address review findings on the pg read/write split

ci/woodpecker/pr/build Pipeline was successful

Details

ci/woodpecker/pr/test Pipeline was successful

Details

ci/woodpecker/pr/pre-commit Pipeline was successful

Details

02233919d7

Name the replica host variable that was actually resolved. lookup falls
back from the prefixed name to the bare one, but the ambiguous-replica
error printed the prefixed form unconditionally, so a deployment setting
APP_DATABASE_URL alongside a bare DB_RO_HOST was told to go and look at
APP_DB_RO_HOST, which is not in its environment. lookupNamed reports the
variable the value came from and the error names that one.

Build the replica pool before the primary in NewCluster. pgxpool
connects lazily, so this dials nothing, but it parses the replica DSN: a
mistyped one now fails before the primary is opened, which removes the
cleanup branch that closed a primary pool nobody could observe and makes
the failure testable through the public constructor. A primary that
cannot be reached closes the lazy replica pool on the way out.

benvin merged commit da23ba9f4a into main

2026-08-31 23:11:19 +10:00

benvin deleted branch benvin/pg-rw-split

2026-08-31 23:11:19 +10:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/golib#2