Test physical replication and standby redo (#241)

[jdatcmd]

@ChronicallyJD recovery.sh covers crash redo in place. Nothing has ever stood
up a replica, and wal_envelope.sh only greps the C source for the envelope
around the one direct XLogInsert — so that record had never been executed,
let alone replayed.

test/replication.sh: pg_basebackup to a second cluster, stream a columnar
workload, assert the standby matches after replay. 21 checks, ~40s.

A correction to the issue's premise

#241 says standby replay "requires the module loaded on the standby with the
correct redo routines". It does not. Every record emitted is a core type —
data pages through log_newpage/log_newpage_buffer (RM_XLOG_ID), and one
XLogInsert(RM_SMGR_ID, XLOG_SMGR_TRUNCATE). There is no custom rmgr, so redo is
core's. The module is needed to read a columnar table, which needs the access
method handler, and that is a different thing from replaying the bytes.

Scenario 3 asserts it: a standby with shared_preload_libraries emptied starts,
replays columnar WAL past the primary's LSN, does not PANIC, and once the module
is restored reads the table it replayed while blind to it. That turns the
standing WAL constraint into a test instead of an argument, and it is the check I
would keep if I could keep only one.

The controls are the point

A replication suite that never replicates passes every content comparison. So the
standby is asserted to be in recovery and to be a different data
directory, and there is a positive control that pauses replay, writes on the
primary, requires the standby not to see it, then resumes and requires that it
does.

Proven by removal. Drop -R from pg_basebackup so the copy is not a
replica: 13 of 21 checks fail, including the in-recovery control, every
content hash, and the truncate assertions.

Two things that cost me time, both recorded in the file

The SMGR truncate record is hard to emit and easy to think you have emitted.
End truncation is opt-in (GUC default off), and truncBlock comes from the
highest live row group, so a DELETE alone reclaims nothing. My first
attempt used default limits and 60,000 rows: pgcolumnar.truncate() returned 0
every time, pg_waldump confirmed zero SMGR TRUNCATE records in the entire
run — and three assertions about replaying that record passed anyway. The suite
now asserts the primary actually truncated before claiming anything about the
standby, and that precondition failed loudly until the fixture was right.

I also mis-read pgcolumnar.truncate() as SQL TRUNCATE at first. It is
best-effort physical end-truncation; rows are not meant to disappear. I nearly
filed that as a replication bug.

Each statement is its own psql call, deliberately. psql -c with several
statements wraps them in one implicit transaction, and set_options and
compact must commit before the next step reads their effect. Batching them was
why the first fixture silently reclaimed nothing.

Gate

21/21 on PG17.10, registered in run_all_versions.sh, harness_selftest green.
I have not run the full five-major matrix on it yet — worth doing before merge,
since it adds a suite that starts two clusters and the port arithmetic
(SB_PORT = PGC_PORT + 1) is the kind of thing that only collides under the
matrix's parallelism. Flagging it rather than discovering it there.

[jdatcmd]

@ChronicallyJD ready for review. Full five-major matrix is running on this branch
now, for the reason in the PR body: it adds a suite that starts two clusters
and SB_PORT = PGC_PORT + 1 is exactly the arithmetic that only collides under
the matrix's parallelism. I will post the result here either way.

The part most worth your scepticism is Scenario 3, because it is the one that
tests the project's standing WAL constraint rather than a behaviour: a standby
with shared_preload_libraries emptied replays columnar WAL and does not PANIC.
If that reasoning is wrong -- if some path does need the module at redo time --
the whole "no new WAL semantics" argument from #187 is weaker than we think, and
I would rather you attacked it now.

Second thing worth checking: I assert the primary actually truncated before
asserting anything about the standby, because my first version had three passing
assertions about replaying a record that pg_waldump proves was never written.
The precondition is doing real work, not decoration -- it failed loudly through
three wrong fixtures before the fourth was right.

[ChronicallyJD]

Attacked it where you asked — Scenario 3 and the truncate precondition — and both hold. The one thing I'd block on is the standby port: it's the collision you suspected, confirmed with the exact mechanism.

Scenario 3 (module-less redo) — sound, and it settles the argument

The standby restarts with shared_preload_libraries='' (verified via current_setting), the primary then INSERTs into a pgcolumnar table generating log_newpage WAL, and the standby replays past that LSN, in recovery, before the module is reloaded to read. Real columnar WAL is in the replayed range, so it isn't vacuous. This turns "redo needs no module" from an argument into a test — and it means the #241 premise ("requires the module loaded on the standby") was mine and wrong: every record is a core type, so redo is core's. Good correction.

Truncate precondition — real, hole closed

check "the primary actually truncated" (trunc > 0) runs before any standby assertion. Traced it: columnar_truncate → columnar_do_end_truncation returns oldnblocks - truncBlock only after ColumnarTruncateMainFork emits XLogInsert(RM_SMGR_ID, XLOG_SMGR_TRUNCATE) (columnar_storage.c:368, gated on RelationNeedsWAL). So trunc > 0 genuinely proves the record was written; zero reclaim → precondition FAILs. The "three assertions passed with zero records" hole is closed.

Vacuity controls — sound, couldn't break them

sync_standby polls pg_last_wal_replay_lsn() vs the primary's current LSN before every compare (no stale-but-equal pass); post-basebackup mutations mean a frozen standby can't match; the negative control's resume half proves streaming actually propagates. Registration (SUITES, not in not_a_suite) and teardown (both clusters under PGC_WORKDIR) are correct.

Block on this: the standby port collides by construction in the matrix

SB_PORT=$((PGC_PORT + 1)) (replication.sh:41) is unguarded — no pgc_pick_port, no freeness loop, no retry — and sb_q (:49) connects to it with no cluster-identity guard. In the matrix, ports are consecutive (run_all_versions.sh:220 port=$((BASE_PORT++))) and replication sits immediately before native_backend_crash in SUITES:92. So native_backend_crash's assigned primary port == replication's SB_PORT, and both run in the same 6-wide window. native_backend_crash binds it early (just initdb+start); replication's standby binds it late (after a 20k INSERT + pg_basebackup), loses the race, its pg_ctl start fails silently (>/dev/null, :100), and sb_q then talks to native_backend_crash's primary — so :102 "standby accepts connections" PASSES against the foreign cluster and :109 "is in recovery" gets f → a baffling RED, with SB_LOG showing EADDRINUSE. It's the exact cross-cluster false-red the harness_selftest identity guard exists to prevent, reintroduced on the standby side — and that guard's port-literal grep doesn't model a derived PGC_PORT + 1, so it won't catch it.

Fix: pick SB_PORT with pgc_pick_port / a freeness loop, and have sb_q assert the endpoint is actually in recovery / the expected datadir before trusting it (mirror pgc_cluster_is_ours). Your running five-major matrix should surface this as a replication RED with "is in recovery: got [f]" — that's this, not a redo bug.

Two minor

• hash_primary/hash_standby (:81) return an empty string on query error; lib.sh's pgc_set_hash returns a unique QUERY_ERROR.$seq for exactly this reason. Two simultaneous errors would compare equal → false PASS. It doesn't fire today, but the port bug can land the standby on a table-less foreign cluster returning empty — so the margin is thinnest right where the other bug lives.
• The negative control's pause half (:126) can't distinguish "paused" from "lagging" in its 1s window; the resume half is what proves detectability. Fine, just noting the pause half alone isn't load-bearing.

Fix the port and this is a strong suite — it's the first thing that actually executes and replays the WAL envelope instead of grepping for it.

[jdatcmd]

The matrix caught the thing I flagged, and then a second thing behind it. Both
fixed; full five-major matrix now ALL VERSIONS PASSED, 490 suite runs.

1. The port, which I predicted and still got wrong

SB_PORT = PGC_PORT + 1 is the single value that cannot work. run_all_versions.sh
hands each suite a port by walking upward from a base, one per suite per major,
so +1 is the next suite's primary by construction. Green standalone, red in
the matrix. portlib.sh documents that walk in detail; I wrote the bug and read
the comment afterwards.

2. The one actually worth having found

Fixing the port moved the failure from PG18 to PG19, which is the signature of a
race rather than a collision. The captured log:

-- primary port 49080, standby port 34152
PASS  pg_basebackup produced a data directory
FAIL  the standby accepts connections: got [] want [1]
... eleven more empty-result failures

pg_ctl -w gives up after 60s and I was discarding its exit status, so under
the full matrix -- six suites and their clusters on one box -- the standby
sometimes had not reached consistency in time, and the suite carried on and
reported twelve downstream failures, not one of which said "the standby never
started".

sb_start now waits for the standby to actually answer and prints the standby's
own log when it does not. A suite whose subject fails to start should say that
once, with evidence, rather than let the cascade speak for it.

How it was found

Neither reproduced under four concurrent copies of this suite, nor under a
single-major matrix run. Only the full five-major run triggers it, and the
builddir is deleted per major, so the log had to be snapshotted while the run was
in progress. Recording that because the next person chasing an intermittent
matrix failure will need the same trick.

[ChronicallyJD]

Fix confirmed against the diff:

• pick_sb_port() (:54) walks pgc_port_free instead of PGC_PORT + 1, so the collision is gone at the root, not papered over. sb_start() (:91) now polls until the standby actually answers and dumps SB_LOG on failure, so the second issue -- pg_ctl -w giving up under matrix contention while its exit status was discarded -- reports itself once, with evidence, instead of cascading into twelve empty-result reds. Both the thing I flagged and the race behind it are properly closed, and 490 suite runs across five majors is the proof.

The two minors I raised are now low priority precisely because the port fix removed the condition that made them matter: the sb_q identity guard is moot once the port is uniquely picked, and the empty-string-on-error in hash_* only bit when the standby could land on a foreign cluster, which it no longer can. Worth the QUERY_ERROR.$seq treatment someday for parity with lib.sh, but not blocking.

The race is the good find -- start-wait races that only appear under the full five-major run, snapshotted from a per-major builddir that's deleted after, are exactly the kind that eat a day. Recording the snapshot trick in the file is the right call.