Repository navigation
Payload-dimension pre-filter test no longer asserts a cluster-wide WAL delta (#4354) - #4356
Merged
Merged
Conversation
…pre-filter test (#4354) pg_current_wal_lsn() is cluster-wide, not scoped to the rows the test writes. The ~39 own-store classes that mint scratch databases on the same Postgres cluster run in parallel with this test's live-postgres collection and can push the WAL delta past the 1 KB bar with traffic that never touches this test's rows. The xmax and last_seen checks already pin "no lock taken, no row touched" directly against the rows this test wrote, so the WAL assertion was redundant as well as flaky. Also notes the own-store exemption's isolation is scoped to rows/relations, not cluster-wide state.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4354.
Why
PayloadDimensionLiveTests.PreFilter_SkipsLockingFreshRows_ButStillRefreshesStaleOnes_AndInsertsNewDigests(added by #4288) asserted a near-zero
pg_wal_lsn_diff(pg_current_wal_lsn(), before)for anall-fresh batch.
pg_current_wal_lsn()is cluster-wide, not scoped to the relation the testwrites. The class is
[Collection("live-postgres")], which only serializes against otherlive-postgresclasses — it does not serialize against the ~39 "own-store" classes (the#1776 own-storeexemption) that mint scratch databases on the SAME cluster and run in parallel. Any ofthat traffic (plus autovacuum/checkpoints) landing inside the test's measurement window pushes the
delta past the 1 KB bar even when the test's own transaction touched nothing. Seen on PR #4347
(run 36186310095, attempt 2) and PR #4353 (run 36198090347), both unrelated to this test.
What changes
Test-only. No production code changes.
pg_wal_lsn_diffassertion and its two now-unused helper locals(
CurrentWalLsnAsync,WalBytesSinceAsync) fromPreFilter_SkipsLockingFreshRows_ButStillRefreshesStaleOnes_AndInsertsNewDigests. The test'sexisting
xmax <> 0andlast_seenchecks already pin "no lock taken, no row touched" directlyagainst the rows this test wrote — they have no exposure to unrelated cluster WAL traffic, so
the WAL assertion was redundant as well as flaky. The summary doc comment on the test now
explains why the WAL check was there and why it was removed.
LivePostgresCollectionHygieneTests.cs's doc comment on the own-store exemption now saysexplicitly that the exemption's isolation is scoped to rows/relations, not cluster-wide state
such as WAL position, which every session on the same server moves.
Test plan
dotnet build Darling/Darling.Tests/Darling.Tests.csproj -p:EnableWindowsTargeting=true: 0 errors, 0 warnings.timescale/timescaledb:2.30.1-pg18container and a throwawaynet10.0 console harness (outside the repo) that replayed the test's arrange/act steps via
PgMigrations.MigrateAsync+PayloadDimensionWriter.FlushAsync, with concurrent WAL noise fromanother session standing in for parallel own-store classes:
locked=0, changedLastSeen=0— PASS.PayloadDimensions.UpsertSql(dropped theWHERE NOT EXISTSanti-join, restoring the pre-Payload-dim upsert still writes on every sighting: ON CONFLICT DO UPDATE locks each presented row before its one-hour guard skips the UPDATE #4249 shape):locked=50, changedLastSeen=0—FAIL on the
xmaxcheck, confirming the remaining assertions still catch the regression Payload-dim upsert still writes on every sighting: ON CONFLICT DO UPDATE locks each presented row before its one-hour guard skips the UPDATE #4249fixed without the WAL bar. The mutation was reverted;
git statusshows only the two fileslisted above changed.
PayloadDimensionLiveTestsandLivePostgresCollectionHygieneTestsagainst the shippedruntime (PG 18.6 + TimescaleDB 2.30.1) — Windows-only (net10.0-windows), left to CI.
Darling.Testssuite — Windows-only, left to CI.For the coordinator
prior lessons on this repo, but is otherwise a throwaway rig (not CI, not the fetch-pg-runtime.ps1
path) — treat the red/green numbers as a code-level proof of the assertion's behavior, not a CI
run.