GiST wal_consistency_checking issue
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253425psql -h localhost -U postgresBuilt from patchset v3 (message #3), August 17, 2026 at 12:44 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253425_3 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253425_3 && git checkout t253425_3Patchset v3 (message #3) is on t253425_3
GiST produces wal_consistency_checking complaints whenever a
XLOG_GIST_DELETE record is replayed. The immediate problem is that
F_TUPLES_DELETED is set (set, *not* unset) within
gistRedoDeleteRecord, even though we do nothing with XLOG_GIST_DELETE
at the corresponding point of original execution, in gistprunepage.
The more fundamental problem is that we're using F_TUPLES_DELETED for
anything at all.
The purpose of XLOG_GIST_DELETE relates in some way to old style
VACUUM FULL. It's obviously useless now: there are 3 places that set
F_TUPLES_DELETED, but nothing ever tests whether it is set in any GiST
page's opaque area's flags field (barring pageinspect).
Attached patch marks F_TUPLES_DELETED deprecated, removes all of its
helper function-style macros, and removes all remaining callers of
those macros.
--
Peter Geoghegan
On Sat, Aug 15, 2026 at 07:31:01PM -0400, Peter Geoghegan wrote:
The purpose of XLOG_GIST_DELETE relates in some way to old style
VACUUM FULL. It's obviously useless now: there are 3 places that set
F_TUPLES_DELETED, but nothing ever tests whether it is set in any GiST
page's opaque area's flags field (barring pageinspect).Attached patch marks F_TUPLES_DELETED deprecated, removes all of its
helper function-style macros, and removes all remaining callers of
those macros.
batta tests wal_consistency_checking (maybe it's the only buildfarm
member doing so), and did not detect anything:
https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=batta&br=master
Your patch is not adding any tests, and I am pretty sure that recovery
test 027 would fail because standbys stop on an inconsistency.
Shouldn't the tests be expanded in some way, then? Your patch does
not do any of that. You could hide an extra expensive test script
under a conditional wal_consistency_checking, for example.
--
Michael
On Sat, Aug 15, 2026 at 7:53 PM Michael Paquier <michael@paquier.xyz> wrote:
batta tests wal_consistency_checking (maybe it's the only buildfarm
member doing so), and did not detect anything:
https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=batta&br=masterYour patch is not adding any tests, and I am pretty sure that recovery
test 027 would fail because standbys stop on an inconsistency.
We already have test coverage for LP_DEAD marking GiST index tuples in
src/test/modules/index/specs/killtuples.spec (plus coverage for all
other index AMs that support this optimization). What we lack is test
coverage for the code that deals with subsequent deletion of those
LP_DEAD-marked tuples by gistprunepage (and so gistRedoDeleteRecord,
its REDO routine, also isn't tested).
Attached v2 adds this missing test coverage.
I verified that hash already has the same coverage for its
corresponding deletion routines.
--
Peter Geoghegan
On Sat, Aug 15, 2026 at 08:15:23PM -0400, Peter Geoghegan wrote:
We already have test coverage for LP_DEAD marking GiST index tuples in
src/test/modules/index/specs/killtuples.spec (plus coverage for all
other index AMs that support this optimization). What we lack is test
coverage for the code that deals with subsequent deletion of those
LP_DEAD-marked tuples by gistprunepage (and so gistRedoDeleteRecord,
its REDO routine, also isn't tested).Attached v2 adds this missing test coverage.
Short runtime, that works. And also confirmed that without your
suggested fix:
2026-08-16 15:24:25.504 JST startup[48936] FATAL: inconsistent page
found, rel 1663/16384/16526, forknum 0, blkno 3
2026-08-16 15:24:25.504 JST startup[48936] CONTEXT: WAL redo at
0/2132BD18 for Gist/DELETE: delete: snapshotConflictHorizon 0, nitems:
162, isCatalogRel F; blkref #0: rel 1663/16384/16526, blk 3 FPW
I have looked as well at the patch, and that seems sensible to remove
the delete markers, I was surprised to see that we do nothing with
them, with traces of these macros removed in commits as old as
68446b2c87a2. Leaving pageinspect as you are suggesting is fine for
existing on-disk data.
So that WFM.
--
Michael