test(cluster): spec-5.58 acceptance — opus review fixes (2 P1 + 4 P2)

Post-ship opus code review (0 P0, suite confirmed a genuine non-fake gate):
false-visible caught by the t/319 differential, boundary degradation by t/321
(code-verified unconditional 53R9G), stale-reuse by the in-range forwarded
t/306.  The findings were coverage-claim overstatements, not correctness holes.

P1-1 (t/319): the header claimed a shared-L2-pool cross-backend HIT, but the
actual L4 assertion is cr_tuple_verdict_count (a plain verdict SELECT does NOT
warm the pool -- the fast path short-circuits the full-block construct).
Rewrote the header/abstract to state the real proof + that the pool HIT is gated
by t/303/t/306 (re-run in nightly 276-323); deleted the dead pool_hit sub.

P1-2 (t/322): L3/L4/L5 are CURRENT-snapshot reads that do not enter the CR
construct/pool path, so they are DDL-correctness smoke checks, not CR-pool
stale-reuse gates.  Softened the docstring + labels accordingly and stated that
the genuine relfilenode-reuse stale-serve gate is t/306 (re-run by 5.58); L6
remains the real held-snapshot historical CR read.

P2-1 (t/320 L5): tightened the clean-construct leg from an or-tautology to
is(eclean,'') so it can fail if the band leaks a masked error.
P2-2 (t/319): added a non-empty guard (>300 rows) so the master differential
cannot pass on empty input.
P2-3 (t/319): assert the pre-churn relation fits the sampled 4-block range so
the cap cannot silently drop authoritative rows.
P2-5: dropped unused Time::HiRes imports (t/319/320/322/323).

Tests-only; no behavior change.  Spec: spec-5.58 (FROZEN v1.0)

Author: SqlRush

src/test/cluster_tap/t/319_cluster_5_58_cr_fullstack_differential.pl

@@ -22,19 +22,25 @@
 #	      (cluster_cr.c:1958) = L1 cache -> L2 pool -> reconstruct, i.e. the LIVE
 #	      optimized band.
 #	    * two backends (R1, R2) take REPEATABLE READ snapshots in the same
-#	      quiescent window so they share one read_scn (5.50 §17.3): R1's read
-#	      publishes the CR image to the shared L2 pool, R2's read HITS it
-#	      cross-backend -> proves the pool really fired (not all-fallback).
+#	      quiescent window so they share one read_scn (5.50 §17.3) -> cross-backend
+#	      equivalence of the live optimized read.  (The tuple fast path serves
+#	      these verdicts WITHOUT warming the L2 pool; the pool cross-backend HIT is
+#	      gated by the dedicated t/303 + t/306, re-run in nightly range 276-323.)
 #
 #	  Assertions (own-instance, single all-on node):
 #	    L1  R1 and R2 share one read_scn (quiescent window).
 #	    L2  the REAL optimized read == the AUTHORITATIVE SRF image, exact visible
 #	        (id,v) set, for every sampled block (the master differential, HG#1).
 #	    L3  cross-backend equivalence: R1's optimized read == R2's optimized read
 #	        (no false-visible / extra / missing row across backends).
-#	    L4  the band REALLY fired: the shared L2 pool recorded a cross-backend HIT
-#	        (R2 served from R1's published image) -- the equivalence is over the
-#	        live optimized path (L250).
+#	    L4  the band REALLY fired: the tuple-level fast path served a DECIDED
+#	        verdict (cr_tuple_verdict_count advanced -- a distinct counter from the
+#	        fallback outcomes), so the differential above is over the LIVE
+#	        optimized path, not an all-fallback no-op (L250).  (A plain verdict
+#	        SELECT does NOT warm the shared L2 pool -- the fast path short-circuits
+#	        the full-block construct -- so the pool cross-backend HIT is gated by
+#	        t/303 + t/306, which spec-5.58 re-runs in nightly range 276-323, not
+#	        here.)
 #	    L5  determinism: re-reading is identical across >=5 runs (no flake).
 #
 # IDENTIFICATION
@@ -53,7 +59,6 @@
 use lib "$FindBin::RealBin/../lib";
 
 use Test::More;
-use Time::HiRes qw(usleep);
 use PgracClusterNode;
 
 # ---- all-on profile: the whole CR read-path band live ---------------------
@@ -83,7 +88,6 @@ sub cr_counter
 		qq{SELECT COALESCE((SELECT value::bigint FROM pg_cluster_state
 		   WHERE category='$cat' AND key='$key'), 0)}) + 0);
 }
-sub pool_hit { return cr_counter('cr_pool', 'hit_count'); }
 sub tuple_verdict { return cr_counter('cr', 'cr_tuple_verdict_count'); }
 
 # Authoritative ground truth: raw full-block reconstruct (no cache) as a sorted
@@ -111,6 +115,12 @@ sub auth_digest
 
 my $maxblk = $node->safe_psql('postgres',
 	q{SELECT GREATEST(0, (pg_relation_size('t_diff') / current_setting('block_size')::int)::int - 1)});
+# P2-3 (review): the authoritative SRF is sampled over blocks 0..maxblk while the
+# optimized read is the whole table; assert the pre-churn relation fits within the
+# sampled range so the 4-block cap below can never silently drop rows from the
+# authoritative side (which would make L2a fail for the wrong reason).
+cmp_ok($maxblk, '<=', 3,
+	"L1 pre-churn t_diff fits in the sampled block range (maxblk=$maxblk <= 3)");
 $maxblk = 3 if $maxblk > 3;	# sample the first few blocks
 
 # Two readers take REPEATABLE READ snapshots in the same quiescent window: their
@@ -154,6 +164,10 @@ sub auth_digest
 # Normalize: both are sorted (id,v) over the same visible set; compare as sets.
 my %rset = map { $_ => 1 } split /\|/, $real1;
 my %aset = map { $_ => 1 } split /\|/, $auth;
+# P2-2 (review): the master differential must not pass on empty input (both sets
+# empty would make the diff asserts trivially green).
+cmp_ok(scalar keys %rset, '>', 300,
+	'L2 sanity: the optimized read returned the expected ~400 pre-churn rows (not empty)');
 my $only_real = join(',', grep { !$aset{$_} } sort keys %rset);
 my $only_auth = join(',', grep { !$rset{$_} } sort keys %aset);
 

src/test/cluster_tap/t/320_cluster_5_58_cr_failclosed_direction.pl

@@ -46,7 +46,6 @@
 use lib "$FindBin::RealBin/../lib";
 
 use Test::More;
-use Time::HiRes qw(usleep);
 use PgracClusterNode;
 
 # all-on band profile
@@ -127,12 +126,15 @@ sub cr_counter
 }
 is($corrupt_hits, 4, 'L4 cr_corruption fails closed data_corrupted for all 4 subkinds (band on)');
 
-# --- L5: clean construct (no injection) is deterministic CR behavior ---
+# --- L5: clean construct (no injection, max read_scn) SUCCEEDS -------------
+# All injections disarmed and read_scn = INT64_MAX (the watermark can never
+# exceed it, so no 53R9F), so the construct must succeed with NO error -- this
+# leg can actually fail if the band leaks a stale/masked error after the armed
+# cases (review P2-1: was an or-tautology that passed on any CR-shaped error).
 $s->query("SELECT cluster_cr_test_construct('t_fc'::regclass,0,0,9223372036854775807)");
 my $eclean = $s->{stderr}; $s->{stderr} = '';
-ok($eclean eq '' || $eclean =~ /(53R9F|53R9G|XX001|cluster CR)/,
-	'L5 a clean construct is deterministic CR behavior, not a leftover/masked error '
-  . '(the band did not enter a broken state)');
+is($eclean, '',
+	'L5 a clean construct succeeds with no error (the band did not enter a broken state)');
 $s->quit;
 
 # --- L6: the fail-closed paths really ran (counters advanced) ---

src/test/cluster_tap/t/322_cluster_5_58_cr_no_stale_reuse.pl

@@ -4,27 +4,34 @@
 # 322_cluster_5_58_cr_no_stale_reuse.pl
 #	  spec-5.58 D2 (HG#2) — no stale reuse under lifecycle churn, full band on.
 #
-#	  With the WHOLE CR read-path band live (shared L2 pool + admission + tuple
-#	  fast path + per-rel generation + resolver + L1 cache), prove that lifecycle
-#	  churn that frees/rewrites a relfilenode (TRUNCATE / VACUUM FULL / CLUSTER /
-#	  DROP+recreate) NEVER lets a cached CR image be served stale: a read after
-#	  the churn reflects the NEW relation state, never the pre-churn image, and
-#	  the pool/cache invalidation (epoch bump on relfilenode unlink) really fires.
-#	  This is the combination-level check (5.53 relfilenode-reuse fence + 5.56
-#	  lifecycle epoch bump live at once).
+#	  With the WHOLE CR read-path band live, prove that lifecycle churn that
+#	  frees/rewrites a relfilenode (TRUNCATE / VACUUM FULL / DROP+recreate) leaves
+#	  the relation correct and the band live -- DDL correctness with the band on,
+#	  plus a held-snapshot historical CR read that still reconstructs correctly
+#	  after the churn.
+#
+#	  Scope (review P1-2, honest): L3/L4/L5 are CURRENT-snapshot reads; a current
+#	  read sees live tuples via native MVCC and does NOT enter the CR construct /
+#	  pool path (only an as-of-snapshot HISTORICAL read does -- L6), so these legs
+#	  cannot serve a cached CR image stale and are DDL-correctness smoke checks,
+#	  not CR-pool stale-reuse gates.  The genuine CR-pool relfilenode-reuse
+#	  stale-serve gate (shared-read_scn cross-backend pool HIT -> epoch bump on
+#	  unlink -> the stale-epoch image MISSES) is spec-5.53 t/306, which spec-5.58
+#	  re-runs in nightly range 276-323; the tuple fast path never reads the pool,
+#	  so its isolation coverage == its combination coverage on this axis.
 #
 #	    L1  band-all-on node up.
-#	    L2  warm the band: a long-snapshot CR read populates the pool/cache with
-#	        the relation's images.
-#	    L3  TRUNCATE + reinsert DIFFERENT data -> a fresh read reflects the new
-#	        data, never the warmed (pre-truncate) image (no stale serve).
-#	    L4  VACUUM FULL (relfilenode rewrite, same data) -> the optimized read
-#	        still equals the authoritative reconstruct (differential preserved
-#	        across a rewrite).
-#	    L5  DROP + recreate + insert -> a read of the recreated relation reflects
-#	        ONLY the new data (no over-hit from the old relfilenode's pool entries).
-#	    L6  the pool epoch really bumped across the churn (relfilenode-unlink
-#	        invalidation fired -- the no-stale guarantee is enforced, not luck).
+#	    L2  a long-snapshot CR read sees the pre-churn relation (band engaged).
+#	    L3  TRUNCATE + reinsert DIFFERENT data -> a current read reflects ONLY the
+#	        new data (DDL correctness, band on).
+#	    L4  VACUUM FULL (relfilenode rewrite) -> a current read reflects ONLY the
+#	        new data exactly (no loss / no duplicate across the rewrite).
+#	    L5  DROP + recreate + insert -> a current read reflects ONLY the reborn
+#	        data (DDL correctness, band on).
+#	    L6  a held-snapshot HISTORICAL CR read after all the churn STILL
+#	        reconstructs the pre-update image exactly, and the tuple fast path
+#	        still fires -- the band is live + correct after lifecycle churn (not a
+#	        broken state).
 #
 # IDENTIFICATION
 #	  src/test/cluster_tap/t/322_cluster_5_58_cr_no_stale_reuse.pl
@@ -42,7 +49,6 @@
 use lib "$FindBin::RealBin/../lib";
 
 use Test::More;
-use Time::HiRes qw(usleep);
 use PgracClusterNode;
 
 my $node = PgracClusterNode->new('cr_nsr');
@@ -90,7 +96,7 @@ sub tuple_verdict
 $r->query_safe('ROLLBACK');
 $r->quit;
 chomp($warm);
-is($warm, '300', 'L2 long-snapshot CR read warmed the band (sees all 300 pre-churn rows)');
+is($warm, '300', 'L2 long-snapshot CR read sees all 300 pre-churn rows (band engaged)');
 
 # ---- L3: TRUNCATE + reinsert different -> no stale serve -------------------
 $node->safe_psql('postgres', 'TRUNCATE t_nsr');
@@ -100,7 +106,7 @@ sub tuple_verdict
 	q{SELECT count(*) || ',' || count(*) FILTER (WHERE v LIKE 'NEW_%') || ',' ||
 	         count(*) FILTER (WHERE v LIKE 'orig_%' OR v LIKE 'upd%') FROM t_nsr});
 is($after_trunc, '50,50,0',
-	'L3 after TRUNCATE+reinsert: read reflects ONLY the new 50 rows, NO stale pre-truncate image');
+	'L3 after TRUNCATE+reinsert: current read reflects ONLY the new 50 rows (DDL correctness, band on)');
 
 # ---- L4: VACUUM FULL (rewrite) -> differential preserved -------------------
 $node->safe_psql('postgres', 'VACUUM FULL t_nsr');
@@ -109,7 +115,7 @@ sub tuple_verdict
 my $expect_vac = $node->safe_psql('postgres',
 	q{SELECT string_agg(g||':'||'NEW_'||g, '|' ORDER BY g) FROM generate_series(1,50) g});
 is($after_vac, $expect_vac,
-	'L4 after VACUUM FULL (relfilenode rewrite): every row exactly the new data (no stale, no loss)');
+	'L4 after VACUUM FULL (relfilenode rewrite): current read is exactly the new data (no loss, no duplicate)');
 
 # ---- L5: DROP + recreate + insert -> no over-hit --------------------------
 $node->safe_psql('postgres', 'DROP TABLE t_nsr');
@@ -119,8 +125,8 @@ sub tuple_verdict
 my $after_drop = $node->safe_psql('postgres',
 	q{SELECT count(*) || ',' || count(*) FILTER (WHERE v LIKE 'REBORN_%') FROM t_nsr});
 is($after_drop, '20,20',
-	'L5 after DROP+recreate+insert: read reflects ONLY the reborn 20 rows '
-  . '(no over-hit from the old relfilenode pool entries)');
+	'L5 after DROP+recreate+insert: current read reflects ONLY the reborn 20 rows '
+  . '(DDL correctness, band on)');
 
 # ---- L6: the band is STILL live + correct after all the churn --------------
 # A fresh long-snapshot CR read on the reborn relation reconstructs correctly

src/test/cluster_tap/t/323_cluster_5_58_cr_real_benefit_path.pl

@@ -47,7 +47,6 @@
 use lib "$FindBin::RealBin/../lib";
 
 use Test::More;
-use Time::HiRes qw(usleep);
 use PgracClusterNode;
 
 my $node = PgracClusterNode->new('cr_rb');