Skip to content

Add regression test for FSM vacuum of bulk-extended relations#68

Draft
NikolayS wants to merge 1 commit into
masterfrom
claude/testplan-fsm-vm-boundary
Draft

Add regression test for FSM vacuum of bulk-extended relations#68
NikolayS wants to merge 1 commit into
masterfrom
claude/testplan-fsm-vm-boundary

Conversation

@NikolayS

@NikolayS NikolayS commented Jul 18, 2026

Copy link
Copy Markdown
Owner

What

Adds a TAP regression test (contrib/pg_freespacemap/t/001_bulk_extend_fsm.pl, 151 lines incl. comments) covering the off-by-one that commit a2fd8d6 ("Include last block in FSM vacuum of bulk extended relation") fixed. That fix shipped with no test.

Why

RelationAddBlocks() bulk-extends a relation, records the not-immediately-used new blocks in the FSM, and calls FreeSpaceMapVacuumRange(rel, first, last) to propagate that free space up the FSM tree. The end argument is exclusive, but the code passed the last added block number instead of last + 1. The bug only bites when that last block is the first slot of a new FSM leaf page (heap block SlotsPerFSMPage = 4069 for the default 8 kB BLCKSZ, 2*SlotsPerFSMPage, …): its free space lands in the leaf FSM page but never reaches the upper levels, so it stays invisible to FSM searches (GetPageWithFreeSpace) until the next full FSM vacuum. pg_freespace() reads the leaf value directly, so it is unaffected by the bug and cannot detect it.

How the test works

  • Sizes the relation from block_size and the measured tuples-per-page. SlotsPerFSMPage is used only to size the relation so a bulk extension lands on the boundary — the detection needs no knowledge of the FSM page node layout.
  • Bulk-extends a committed table with COPY (which uses a BulkInsertState, taking the bulk-extension path). The extension grid is 64-block-periodic once saturated; the test learns the grid offset from a small probe COPY and shifts it with a committed prefill so the final extension's last block lands exactly on the FSM leaf-page boundary.
  • Detection uses the invariant a2fd8d6 restores, not hard-coded layout arithmetic: because the bulk extension has already propagated the new free space up the tree, an immediately-following redundant VACUUM must not raise any FSM node value (it may only lower stale entries for partially-filled pages). The test snapshots every non-zero FSM node (pageinspect.fsm_page_contents), runs VACUUM, and asserts no node value increased. With the bug, the boundary block's free space is missing from the upper levels, so the redundant VACUUM raises the parent node from 0 and the test fails.

Changes from the first revision (reviewer feedback)

  • Removed the visibility-map round-trip entirely — it was unrelated to the FSM bug and duplicated existing contrib/pg_visibility coverage (test_vac_unmodified_heap, copyfreeze already exercise pg_visibility_map_summary / pg_check_visible / pg_check_frozen after VACUUM; a real VM page boundary is 32672 blocks ≈ 255 MB, infeasible to test). pg_visibility is no longer an EXTRA_INSTALL dependency.
  • Removed hard-coded FSM node/physical-block arithmetic (previously inspected parent physical block 1, node 4096). Detection is now the layout-agnostic "redundant VACUUM raises no FSM node" invariant.
  • Dropped auxiliary/control assertions. Kept only: geometry guard (nblocks == boundary+1), the leaf-has-free-space check (isolates the bug to propagation, since the leaf is unaffected), and the core propagation invariant.

Provenance / test evidence

  • Base commit: d374280e1837adc1f8218bb0d69ffbae5d808fc6 (master, v19 devel), which already contains a2fd8d6.
  • Build flags: meson setup build --buildtype=debug -Dcassert=true -Dtap_tests=enabled
  • Runtime: ~2.2 s (table is ~33 MB — block 4069 is inherent to the default block size).

Fix present — all pass:

1/2 postgresql:pg_freespacemap / pg_freespacemap/regress             OK   0.87s   1 subtest passed
2/2 postgresql:pg_freespacemap / pg_freespacemap/001_bulk_extend_fsm OK   2.21s   4 subtests passed
Ok: 2   Fail: 0

ok 1 - measured 226 tuples per heap block
ok 2 - relation bulk-extended to the FSM leaf-page boundary (4070 blocks)
ok 3 - boundary block 4069 has free space in the leaf FSM page
ok 4 - bulk extension propagated boundary free space: redundant VACUUM raises no FSM node

Gold-standard revert verification. Procedure: in the same worktree, revert a2fd8d6's one-line change (FreeSpaceMapVacuumRange(relation, first_fsm_block, last_block + 1)... last_block)), rebuild src/backend/postgres, refresh it into the meson tmp_install, and re-run the test. Result — the core assertion fails:

ok 1 - measured 226 tuples per heap block
ok 2 - relation bulk-extended to the FSM leaf-page boundary (4070 blocks)
ok 3 - boundary block 4069 has free space in the leaf FSM page
not ok 4 - bulk extension propagated boundary free space: redundant VACUUM raises no FSM node
#   at contrib/pg_freespacemap/t/001_bulk_extend_fsm.pl line 145.
#          got: '1'
#     expected: '0'
Ok: 0   Fail: 1

The single raised node is the parent-page slot for the boundary block's FSM leaf page (block 1, node 4096: 0 → 255). Restored the fix afterward; hio.c is unmodified in this branch.

Closes #59

🤖 Generated with Claude Code

https://claude.ai/code/session_01PtxA6B4d47ietk5dGsseuP

Commit a2fd8d6 ("Include last block in FSM vacuum of bulk extended
relation") fixed an off-by-one in RelationAddBlocks(): the exclusive end
block passed to FreeSpaceMapVacuumRange() was the number of the last
added block instead of one past it.  The bug only manifested when the
last bulk-extended block was the first slot of a new FSM leaf page (heap
block SlotsPerFSMPage, 2*SlotsPerFSMPage, ...), in which case that
block's free space was recorded in the leaf FSM page but never
propagated to the upper FSM levels, leaving it invisible to FSM searches
until the next full FSM vacuum.  The fix shipped without test coverage.

Add a TAP test to contrib/pg_freespacemap that closes that gap.  It
bulk-extends a relation with COPY (which uses a BulkInsertState and thus
takes the bulk-extension path) so that the final extension's last block
lands exactly on an FSM leaf-page boundary.  The relation is sized from
the block size and the measured tuples-per-page, and the extension grid
is shifted with a committed prefill so the boundary is reached
deterministically.

Detection uses the invariant a2fd8d6 restores rather than any hard-coded
FSM page layout: because the bulk extension has already propagated the
new free space up the tree, an immediately following VACUUM must not
raise any FSM node value (it may only lower stale entries for partially
filled pages).  With the bug present the boundary block's free space is
missing from the upper levels, so the redundant VACUUM raises the parent
node from zero and the test fails.  pg_freespace() reads the leaf value
directly and is unaffected by the bug, so it cannot be used to detect it.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01PtxA6B4d47ietk5dGsseuP
@NikolayS
NikolayS force-pushed the claude/testplan-fsm-vm-boundary branch from 911caab to ce6ae56 Compare July 18, 2026 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add FSM/VM boundary-geometry regression tests

2 participants