Add tests for fake LSN assignment on unlogged hash and GiST indexes#77
Draft
NikolayS wants to merge 1 commit into
Draft
Add tests for fake LSN assignment on unlogged hash and GiST indexes#77NikolayS wants to merge 1 commit into
NikolayS wants to merge 1 commit into
Conversation
Commit cea74f4 fixed a missed fake-LSN assignment in the hash AM's log_split_page() for unlogged relations (an oversight in the commit that introduced fake LSN use in the hash AM). The fix was purely defensive: nothing in the tree visibly fails without it, which also means that no existing test would notice if a fake-LSN assignment were lost again. The regression suites exercise almost exclusively logged relations, so the XLogGetFakeLSN() paths in the index AMs -- including the parent-LSN/child-NSN split-detection interlock that unlogged GiST indexes rely on -- had essentially no coverage. Add two layers of coverage: * A pageinspect regression test, unlogged_lsn, deterministically drives an unlogged hash index through a bucket split that migrates a multi-page chain of duplicate tuples, and an unlogged GiST index through many page splits, then asserts that the affected index pages carry nonzero, strictly advancing (fake) LSNs and NSNs. The hash check is written so that precisely the omission fixed by cea74f4 is detected: without log_split_page()'s fake LSN, the last overflow page of the migrated chain keeps the LSN assigned when the page was allocated, which equals the preceding page's LSN, so the test's strictly-advancing assertion fails (verified by temporarily reverting cea74f4). All output is reduced to booleans, keeping it independent of the shared fake-LSN counter's actual values. * A TAP test in src/test/modules/index, 001_unlogged_lsn_stress.pl, runs index scans concurrently with split-heavy insert workloads on unlogged hash and GiST indexes, cross-checking every scan result against WAL-logged twin tables holding identical data. The critical interleaving (a scan paused inside a bucket/page while it is split) cannot be expressed with the isolation tester without adding injection points to the hash/GiST split paths, so this is a bounded stress test with a fixed number of scan iterations; pass/fail is still deterministic, as it fails only if a scan returns incorrect results or a backend fails. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01PtxA6B4d47ietk5dGsseuP
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.
What / why
Commit cea74f4 fixed a missed fake-LSN assignment in the hash AM's
log_split_page()for unlogged relations. The fix was purely defensive — nothing visibly fails without it — which also means no existing test would notice a regression. The regression suites exercise almost exclusively logged relations, so theXLogGetFakeLSN()paths in the index AMs (including the parent-LSN/child-NSN split-detection interlock that unlogged GiST scans rely on) had essentially no coverage.This PR adds the two layers of coverage requested in #71.
Layer 1 — deterministic fake-LSN checks (
contrib/pageinspect/sql/unlogged_lsn.sql)Placement rationale: the check needs pageinspect's
page_header(),hash_metapage_info(),hash_page_stats()andgist_page_opaque_info()functions, and pageinspect already has precedent for using unlogged indexes precisely because their LSNs are predictable (sql/gist.sql). A contrib regression test there is the idiomatic, zero-new-infrastructure home.>= 0/3E8, i.e.FirstNormalUnloggedLSN);log_split_page()'s fake LSN, the last overflow page keeps the LSN stamped when it was allocated, which equals the preceding page's LSN;The workload is self-tuning (it picks a key by hash-code bit pattern at runtime and watches the metapage until the chain migrates), so it does not depend on exact
ffactor/page-geometry constants, and all output is booleans, so it is independent of the cluster-wide fake-LSN counter state.Layer 2 — concurrency interlock stress (
src/test/modules/index/t/001_unlogged_lsn_stress.pl)Placement rationale:
src/test/modules/indexis the existing cross-AM index test module (isolation speckilltuples); this adds its first TAP test (Makefile/meson wired accordingly).Why not the isolation tester: the critical interleaving — a scan positioned inside a bucket/page while another session splits it — happens within single SQL statements. The isolation tester can only interleave at statement granularity unless the split paths gain injection points, and hash/GiST currently have none (only gin/nbtree do). Adding injection points to core split code was judged out of scope for a tests-only change. Per the issue's fallback, this is a bounded stress test that is deterministic in pass/fail: a fixed number of scan iterations (100 per AM) runs while a background writer executes split-heavy bulk inserts; the test fails only if a scan returns incorrect results or a backend fails. Timing verified from the run logs: all 100 hash-phase probes and ~all GiST-phase probes execute while the writer is actively inserting.
Test evidence
meson test --suite pageinspect: 9/9 subtests pass (newunlogged_lsnincluded, ~0.4 s).meson test --suite index: isolationkilltuples+ new TAP test pass (8 subtests, ~6 s total).Revert verification (negative test)
Locally reverted cea74f4's
hashpage.chunk and rebuilt: the pageinspect test fails exactly as designed —With the fix restored, the suite passes again. (The TAP stress test does not fail on the revert, as expected: this tree currently has no scan-side consumer of hash page LSNs, which is why cea74f4 was "defensive"; the stress test guards the behavior the LSNs exist to protect, while the pageinspect test guards the LSN assignment itself.)
Caveats
_hash_splitbucket()/_hash_addovflpage()LSN-stamping order; comments in the test document the reasoning so future refactoring can adjust it.Closes #71
🤖 Generated with Claude Code
https://claude.ai/code/session_01PtxA6B4d47ietk5dGsseuP
Generated by Claude Code