diff --git a/Darling/Darling.Tests/LivePostgresCollectionHygieneTests.cs b/Darling/Darling.Tests/LivePostgresCollectionHygieneTests.cs index 46e14c5ac2..e576d2c820 100644 --- a/Darling/Darling.Tests/LivePostgresCollectionHygieneTests.cs +++ b/Darling/Darling.Tests/LivePostgresCollectionHygieneTests.cs @@ -31,7 +31,10 @@ namespace Darling.Tests; /// The exemption is real and must stay available. A class that mints its own database (through /// ScratchPostgres) or stands up its own cluster does not race the shared store, and serializing it would /// cost suite time for no safety at all. So this does not demand the attribute — it demands a DECISION, recorded -/// either as the attribute or as an #1776 own-store comment explaining the exemption. +/// either as the attribute or as an #1776 own-store comment explaining the exemption. That isolation is +/// scoped to the rows and relations the own-store class itself created: it does not extend to cluster-wide state +/// such as WAL position, which every session on the same server — own-store scratch databases included — moves +/// (#4354). A test that needs "nothing else touched" must assert against its own rows, not the WAL LSN. /// /// Why the match is the quoted literal and not a substring. DARLING_TEST_PGRUNTIME and /// DARLING_TEST_PGRUNTIME_OLD are DIFFERENT variables that merely share the prefix, naming an assembled diff --git a/Darling/Darling.Tests/PayloadDimensionLiveTests.cs b/Darling/Darling.Tests/PayloadDimensionLiveTests.cs index b300e2cea0..b33110cb9a 100644 --- a/Darling/Darling.Tests/PayloadDimensionLiveTests.cs +++ b/Darling/Darling.Tests/PayloadDimensionLiveTests.cs @@ -727,12 +727,21 @@ await LiveStoreCleanup.RunAsync(connectionString!, bodySucceeded, async (cleanup /// against a real server, three ways: /// /// (1) A second upsert of the SAME still-fresh batch, 30 minutes later, leaves every - /// row's xmax at 0 (never locked) and advances pg_current_wal_lsn() by under 1 - /// KB for a 50-row batch — effectively a read-only transaction. This is the pin: reverting the - /// WHERE NOT EXISTS pre-filter (restoring the pre-#4249 shape) turns it red. Confirmed - /// once by hand with a raw-SQL rehearsal of both shapes against this rig: the OLD shape left a - /// real transaction id (781) in xmax on a row whose content and last_seen were - /// both unchanged; the NEW shape left xmax at 0. See PR #4288. + /// row's xmax at 0 (never locked) and its last_seen unchanged — effectively a + /// read-only transaction. This is the pin: reverting the WHERE NOT EXISTS pre-filter + /// (restoring the pre-#4249 shape) turns it red. Confirmed once by hand with a raw-SQL + /// rehearsal of both shapes against this rig: the OLD shape left a real transaction id (781) + /// in xmax on a row whose content and last_seen were both unchanged; the NEW + /// shape left xmax at 0. See PR #4288. + /// + /// An earlier revision of this test also asserted a near-zero pg_wal_lsn_diff for + /// the same batch. pg_current_wal_lsn() is cluster-wide, not relation-scoped, so any + /// other session on the same server — the ~39 own-store classes that create scratch databases + /// on this cluster and run in parallel under xunit, plus autovacuum and checkpoints — can push + /// the delta well past the bar even when this test's own transaction touched nothing. The + /// xmax and last_seen checks above already pin "no lock taken, no row touched" + /// directly against the rows this test wrote, with no exposure to unrelated WAL traffic, so + /// the WAL assertion was dropped as redundant and flaky (#4354). /// /// (2) A row stamped two hours ago is still refreshed — the pre-filter's own staleness /// read uses the same one-hour boundary the ON CONFLICT ... WHERE guard always used, so @@ -807,28 +816,17 @@ async Task ChangedLastSeenCountAsync(byte[][] digests, DateTime expected) return (long)(await command.ExecuteScalarAsync(ct))!; } - async Task CurrentWalLsnAsync() - => (string)(await ScalarAsync(connection, "SELECT pg_current_wal_lsn()::text", ct))!; - - /* pg_wal_lsn_diff returns numeric, which Npgsql maps to decimal, not a bigint type. */ - async Task WalBytesSinceAsync(string beforeLsn) - => (long)(decimal)(await ScalarAsync( - connection, "SELECT pg_wal_lsn_diff(pg_current_wal_lsn(), $1::text::pg_lsn)", ct, beforeLsn))!; - // The initial collection cycle: 50 "hot" plans plus one that will go stale. await FlushAsync(freshPairs.Append((staleDigest, stalePayload)), t0); Assert.Equal(0, await LockedRowCountAsync(freshDigests)); // (1) Same batch, same digests, 30 minutes later -- still inside the one-hour // freshness window. The pre-filter excludes every one of them before the statement - // ever reaches INSERT/ON CONFLICT: no lock taken, no last_seen change, near-zero WAL. - var lsnBefore = await CurrentWalLsnAsync(); + // ever reaches INSERT/ON CONFLICT: no lock taken, no last_seen change. await FlushAsync(freshPairs, t0.AddMinutes(30)); - var walBytes = await WalBytesSinceAsync(lsnBefore); Assert.Equal(0, await LockedRowCountAsync(freshDigests)); Assert.Equal(0, await ChangedLastSeenCountAsync(freshDigests, t0)); - Assert.True(walBytes < 1024, $"expected a near-zero WAL delta for an all-fresh batch, saw {walBytes} bytes"); // (2) and (3): two hours after t0, the stale row is refreshed and a brand-new digest // is inserted, in the same flush.