| From: | Andrey Borodin <x4mmm(at)yandex-team(dot)ru> |
|---|---|
| To: | Peter Geoghegan <pg(at)bowt(dot)ie> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
| Subject: | Re: GIN VACUUM can corrupt internal posting tree pages |
| Date: | 2026-08-16 11:26:18 |
| Message-ID: | 66B7BC2D-A1CB-4971-B57F-E772A6598909@yandex-team.ru |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> index-all-keys-match can't detect LP_UNUSED references
index-all-keys-match does handle this partially.
bt_index_parent_check() version holds ShareLock and, when the MVCC fetch
finds nothing, inspects the heap line pointer and reports out-of-range
offsets and LP_UNUSED. bt_index_check() skips that because concurrent
VACUUM can remove the copied index entry and then reclaim its heap slot.
ShareLock still permits opportunistic pruning by SELECT. As far as I can
see, that can only make heap-only HOT chain members LP_UNUSED, while a valid
index TID points to the chain root. Do you see another pruning path that
would make this check unsafe under ShareLock?
It's not "watertight" of course. The inspection is reached only after a
Bloom filter miss, so a false positive can still hide a dangling TID.
> without verifying agreement with the heap
More generally, I think many of the most valuable corruption checks are at
the boundaries between independently maintained representations: adjacent
or parent and child pages, an index and its heap, a relation and its
metadata, or heap visibility information and pg_xact. Local checks cannot
detect contradictions across such boundaries.
That is the direction in which I would like to extend amcheck.
> consistently call ginInsertCleanup within ginbulkdelete
The pending-list fix also seems to be in the right place. Each
ginbulkdelete() pass has a different set of dead TIDs, so ginInsertCleanup()
has to run before every pass, not only when stats is initially allocated.
Doing it in ginvacuumcleanup() would be too late, after the relevant deletion
callback is gone.
> Attached V2 has a second new patch
Are you planning to post fixes with these tests, or are the test patches
only intended as reproducers? I can prepare fixes along the lines you
outlined, but do not want to duplicate your work.
Best regards, Andrey Borodin.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Mats Kindahl | 2026-08-16 11:37:32 | Re: pg_rewind does not rewind diverging timelines |
| Previous Message | Alexandre Felipe | 2026-08-16 09:54:47 | Re: [PATCH] bufmgr: tighten LWLock:BufferMapping on InvalidateBuffer |