diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c1095d2..d1742a1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,42 @@ true until the next version shipped. ### Added +- The two visibility-map clears that no test held are now held (#877). + + **Three paths retire a row group and give its live rows new row numbers**, and + each must clear the all-visible bits over the OLD numbers, or an index-only + scan answers from the index for a TID whose group no longer exists. Only + `pgcolumnar.expire`'s clear was covered, because only its absence had been + reported as data loss. Deleting the other two -- `rewrite_one_group` + (`src/columnar_vacuum.c:346`, reached through `pgcolumnar.compact_rewrite`) and + `pgcolumnar_recluster_online` (`:765`, through `pgcolumnar.recluster`) -- left + 319 checks across 14 suites green. + + `test/vm_clear_on_renumber.sh` is the two missing arms. Neuter the clear at + `:346` and one arm reddens; neuter the one at `:765` and two redden; neither + mutation reddens the other's arms. + + **The instrument is why this was not covered earlier.** + `pg_class.relallvisible` is a statistic `VACUUM` refreshes, and clearing a bit + does not touch it, so an arm reading it reports the same number with the clear + and without. `pgcolumnar.vm_is_visible(rel, blk)` reads the fork through + `visibilitymap_get_status`, which is the call the index-only-scan executor + makes. + + **And a `DELETE` clears the bits of the blocks holding the deleted rows**, so a + group made compactable is already not-all-visible there and an arm placed on it + cannot fail. These arms `VACUUM` first, delete only the front of the target + group, and assert over the group's later blocks, which are all-visible on the + way in and reachable only by the rewrite. + + **The two controls differ in strength and the suite says so.** The rewrite's + control is a group in the same table, and widening that clear to the whole + relation reddens it. Recluster renumbers every group, so that half has no + in-table control; its control is a second relation, which catches a clear + reaching beyond the relation it was called on but NOT an over-broad range -- + the same widening leaves it green. That limit is measured and recorded in the + file rather than left for a reader to infer. + - `MERGE` is documented as working, which it has been all along. It needs no index on the columnar target and takes every arm, including `WHEN MATCHED ... DELETE`, `WHEN NOT MATCHED BY SOURCE`, and diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 6682bb93..39ecc7df 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -276,6 +276,7 @@ SUITES=( vacuum_stripe_count vector_agg_rescan_memory vector_agg_tlist_shape + vm_clear_on_renumber vm_privilege wal_envelope write_fsst_compressed diff --git a/test/vm_clear_on_renumber.sh b/test/vm_clear_on_renumber.sh new file mode 100755 index 00000000..3389d480 --- /dev/null +++ b/test/vm_clear_on_renumber.sh @@ -0,0 +1,207 @@ +#!/usr/bin/env bash +# +# pgColumnar: the visibility map is cleared wherever LIVE rows are RENUMBERED. +# +# Three paths retire a row group and give its live rows new row numbers, and each +# must clear the all-visible bits over the OLD numbers. Otherwise an index-only +# scan answers from the index for a TID whose group no longer exists: +# +# src/columnar_vacuum.c:2310 pgcolumnar_expire -- held by ttl_expire.sh +# src/columnar_vacuum.c:346 rewrite_one_group -- held by NOTHING before this file +# src/columnar_vacuum.c:765 pgcolumnar_recluster_online -- held by NOTHING before this file +# +# Only expire's clear was covered, because only expire's was reported as data +# loss. Deleting the other two left 319 checks across 14 suites green. This file +# is the two missing arms, and it exists as its own suite because the rule is one +# rule -- the comment at :765 states it as "wherever LIVE rows are renumbered +# rather than only where they expire" -- while the operations that break it live +# in different suites. +# +# THE INSTRUMENT, and why the obvious one does not work. `pg_class.relallvisible` +# is a statistic VACUUM refreshes; clearing a VM bit does not touch it, so an arm +# reading it reports the same number on a tree with the clear and a tree without. +# `pg_visibility` is not built under these prefixes. What does work is already in +# the extension: `pgcolumnar.vm_is_visible(rel, blk)` reads the fork through +# visibilitymap_get_status -- the same call the index-only-scan executor makes. +# +# THE FIXTURE'S ONE SUBTLETY. A DELETE clears the bits of the blocks holding the +# deleted rows, so a group that has been made compactable is ALREADY not +# all-visible over those blocks and an arm there cannot fail. The arms below +# therefore VACUUM first, delete only the FRONT of the target group, and assert +# over the group's LATER blocks -- all-visible on the way in, and reachable only +# by the rewrite's clear. +# +# THE TWO CONTROLS DIFFER IN STRENGTH, and the difference is measured rather than +# assumed. Part 1's control is a group in the SAME table the rewrite does not +# touch, so widening that clear to the whole relation reddens it -- measured, 16 +# passed + 1 failed with the range replaced by (rel, 1, 100000000). Part 2 has NO +# in-table control available, because recluster renumbers every group and leaves +# nothing untouched to compare against. Its control is a second relation, which +# catches a clear reaching beyond the relation it was called on, NOT an +# over-broad range: the same widening at :765 leaves the suite 17 passed + 0 +# failed. That is a real limit of this half, stated here rather than left for a +# reader to infer from an arm that looks like Part 1's. +# +# Usage: test/vm_clear_on_renumber.sh [PG_CONFIG] +# Written fresh for pgColumnar. + +set -uo pipefail +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" + +# premise: the block arithmetic below is written for MaxHeapTuplesPerPage at an +# 8 kB block size. A synthetic block covers COLUMNAR_VALID_ITEMPOINTER_OFFSETS +# row numbers (src/columnar.h), which is MaxHeapTuplesPerPage, 291 at BLCKSZ +# 8192. At another block size every block number below names a different row +# range and the arms would measure nothing. +PGC_BLOCK_SIZE="$(q "SELECT current_setting('block_size');")" +if [ "$PGC_BLOCK_SIZE" != "8192" ]; then + echo "PREMISE FAILED: block_size is [$PGC_BLOCK_SIZE]; these arms are written for 8192" >&2 + exit 1 +fi +SPAN=291 + +# Block holding a given row number, the same division the extension does. +blk_of() { echo $(( $1 / SPAN )); } + +# t/f for one synthetic block. +vis() { q "SELECT pgcolumnar.vm_is_visible('$1', $2);"; } + +# --------------------------------------------------------------------------- +# Part 1: rewrite_one_group, reached through pgcolumnar.compact_rewrite +# --------------------------------------------------------------------------- + +psql_run "CREATE TABLE cw (id int, v int) USING pgcolumnar;" +psql_run "SELECT pgcolumnar.set_options('cw', stripe_row_limit => 2048, chunk_group_row_limit => 1024);" +psql_run "INSERT INTO cw SELECT g, g FROM generate_series(1,20480) g;" +psql_run "CREATE INDEX cw_id ON cw (id);" +psql_run "VACUUM cw;" + +# Take the geometry from the catalog rather than from the INSERT, so a change to +# the group size is a premise failure here instead of an arm that silently moves +# to a different part of the table. +CW_G3_FIRST="$(q "SELECT first_row_number FROM pgcolumnar.row_group + WHERE storage_id = pgcolumnar.get_storage_id('cw') AND group_number = 3;")" +CW_G3_COUNT="$(q "SELECT row_count FROM pgcolumnar.row_group + WHERE storage_id = pgcolumnar.get_storage_id('cw') AND group_number = 3;")" +CW_G5_FIRST="$(q "SELECT first_row_number FROM pgcolumnar.row_group + WHERE storage_id = pgcolumnar.get_storage_id('cw') AND group_number = 5;")" +case "$CW_G3_FIRST$CW_G3_COUNT$CW_G5_FIRST" in + *[!0-9]*|"") echo "PREMISE FAILED: group 3/5 geometry unreadable [$CW_G3_FIRST/$CW_G3_COUNT/$CW_G5_FIRST]" >&2; exit 1 ;; +esac + +CW_LAST=$(( CW_G3_FIRST + CW_G3_COUNT - 1 )) +CW_FRONT_END=$(( CW_G3_FIRST + 600 )) # deleted; ~29% of a 2048 group +CW_TARGET_BLK=$(blk_of $(( CW_LAST - 100 ))) # late in group 3, no deleted row +CW_FRONT_BLK=$(blk_of "$CW_G3_FIRST") # early in group 3, deleted rows +CW_CTRL_BLK=$(blk_of $(( CW_G5_FIRST + 100 ))) # group 5, untouched throughout + +# The arm is only meaningful if the target block holds no deleted row, otherwise +# the DELETE below clears it and the rewrite is not what the arm measures. +if [ "$CW_TARGET_BLK" -le "$(blk_of "$CW_FRONT_END")" ]; then + echo "PREMISE FAILED: target block $CW_TARGET_BLK is inside the deleted range" >&2 + exit 1 +fi + +check "premise: VACUUM set all-visible on group 3's late block" \ + "$(vis cw "$CW_TARGET_BLK")" "t" +check "premise: and on the untouched control group's block" \ + "$(vis cw "$CW_CTRL_BLK")" "t" + +psql_run "DELETE FROM cw WHERE id BETWEEN $CW_G3_FIRST AND $CW_FRONT_END;" + +# These two are what make the arm attributable. The DELETE clears the blocks it +# touches; if it also cleared the target block there would be nothing left for +# the rewrite to do and the arm could not fail. +check "the delete cleared the block holding the deleted rows" \ + "$(vis cw "$CW_FRONT_BLK")" "f" +check "premise: but left group 3's late block all-visible" \ + "$(vis cw "$CW_TARGET_BLK")" "t" +check "premise: and left the control block all-visible" \ + "$(vis cw "$CW_CTRL_BLK")" "t" + +# Measure the work, not the intent: the call must report that it rewrote exactly +# the one qualifying group. A zero here would leave every arm below green for a +# rewrite that never ran. +CW_REWROTE="$(q "SELECT pgcolumnar.compact_rewrite('cw', 0.2);")" +check "compact_rewrite rewrote exactly the one qualifying group" "$CW_REWROTE" "1" +check "premise: group 3 is gone from the catalog, so its rows were renumbered" \ + "$(q "SELECT count(*) FROM pgcolumnar.row_group + WHERE storage_id = pgcolumnar.get_storage_id('cw') AND group_number = 3;")" "0" + +# THE ARM. Revert src/columnar_vacuum.c:346 and this goes green-to-red: the block +# stays all-visible over row numbers whose group no longer exists. +check "rewrite_one_group cleared the retired group's late block (#877)" \ + "$(vis cw "$CW_TARGET_BLK")" "f" + +# THE CONTROL, and it is load-bearing: replace this site's range with the whole +# relation and this arm goes red while the arm above stays green. +check "control: a group the rewrite did not touch keeps its all-visible bit" \ + "$(vis cw "$CW_CTRL_BLK")" "t" + +# --------------------------------------------------------------------------- +# Part 2: pgcolumnar_recluster_online, reached through pgcolumnar.recluster +# --------------------------------------------------------------------------- +# +# Recluster renumbers every live row and takes no group limit, so there is no +# untouched group to use as an in-table control, and this half has none. A second +# columnar table, vacuumed the same way and never reclustered, carries what is +# left: it fails a clear that reached beyond the RELATION it was called on. +# +# It does NOT fail an over-broad RANGE, and the arms below must not be read as if +# it did. PgColumnarVMClearForRowRange takes `rel`, so widening the range cannot +# make a clear on rc touch rcctl -- measured: with this site's range replaced by +# (rel, 1, 100000000) the suite is 17 passed + 0 failed. A helper rewritten to +# clear globally WOULD be caught here; a wrong row range would not. + +for t in rc rcctl; do + psql_run "CREATE TABLE $t (id int, x int, y int) USING pgcolumnar;" + psql_run "SELECT pgcolumnar.set_options('$t', stripe_row_limit => 2048, chunk_group_row_limit => 1024);" + psql_run "INSERT INTO $t SELECT g, ((g::bigint*7919)%200)::int, ((g::bigint*104729)%200)::int + FROM generate_series(1,20480) g;" + psql_run "VACUUM $t;" +done + +RC_FIRST="$(q "SELECT min(first_row_number) FROM pgcolumnar.row_group + WHERE storage_id = pgcolumnar.get_storage_id('rc');")" +RC_LAST="$(q "SELECT max(first_row_number + row_count - 1) FROM pgcolumnar.row_group + WHERE storage_id = pgcolumnar.get_storage_id('rc');")" +case "$RC_FIRST$RC_LAST" in *[!0-9]*|"") echo "PREMISE FAILED: rc geometry unreadable" >&2; exit 1 ;; esac + +# A synthetic block is all-visible only when every row number it spans exists. +# The first and last blocks of a relation are PARTIALLY covered -- block 0 spans +# row 0, which no relation has, and the top block runs past the last row -- so +# both read `f` on a correct tree and an arm placed there cannot fail. Step three +# whole blocks in from each end, and gate that the result is genuinely interior. +RC_EARLY_BLK=$(( $(blk_of "$RC_FIRST") + 3 )) +RC_LATE_BLK=$(( $(blk_of "$RC_LAST") - 3 )) +if [ "$RC_EARLY_BLK" -ge "$RC_LATE_BLK" ] || \ + [ $(( RC_EARLY_BLK * SPAN )) -lt "$RC_FIRST" ] || \ + [ $(( (RC_LATE_BLK + 1) * SPAN - 1 )) -gt "$RC_LAST" ]; then + echo "PREMISE FAILED: blocks $RC_EARLY_BLK/$RC_LATE_BLK are not wholly inside rows $RC_FIRST..$RC_LAST" >&2 + exit 1 +fi + +check "premise: VACUUM set all-visible early in rc" "$(vis rc "$RC_EARLY_BLK")" "t" +check "premise: and late in rc" "$(vis rc "$RC_LATE_BLK")" "t" +check "premise: and on the control relation" "$(vis rcctl "$RC_EARLY_BLK")" "t" + +RC_GROUPS="$(q "SELECT pgcolumnar.recluster('rc','x','y');")" +check "recluster reported reclustering groups" \ + "$([ "${RC_GROUPS:-0}" -gt 0 ] 2>/dev/null && echo yes || echo no)" "yes" + +# THE ARMS. Revert src/columnar_vacuum.c:765 and both go red: recluster gives +# every live row a new number, so every old block must lose its bit. +check "recluster_online cleared the old numbers' early block (#877)" \ + "$(vis rc "$RC_EARLY_BLK")" "f" +check "recluster_online cleared the old numbers' late block (#877)" \ + "$(vis rc "$RC_LATE_BLK")" "f" + +# THE CONTROL for this half: a clear that escaped its relation. See the note +# above for what it does not cover. +check "control: a relation that was not reclustered keeps its all-visible bits" \ + "$(vis rcctl "$RC_EARLY_BLK")" "t" +check "control: and its late block too" \ + "$(vis rcctl "$RC_LATE_BLK")" "t" + +pgc_summary