Re: Exceptional md.c paths for recovery and zero_damaged_pages

On Tue, Dec 17, 2024 at 12:57 PM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> Hmm, looking at index_fetch_heap(), I'm surprised it doesn't throw an
> error or even a warning if the heap tuple isn't found. That would seem
> like a useful sanity check. An index tuple should never point to a
> non-existent heap TID I believe.

I think that it is necessary. We need to be prepared to perform a TID
lookup with a wild page offset number to account for concurrent TID
recycling. At least with nbtree plain index scans.

This is also why we can only test the sanity of TIDs in certain
particular contexts. See the comments above index_delete_check_htid().

> > I'm wondering if we should just put an error into the relevant paths in HEAD
> > and see whether it triggers for anybody in the next months. Having all these
> > untested paths in md.c forever doesn't seem great.
>
> +1

Try it with this test case:

https://postgr.es/m/CAH2-Wz=jjiNL9FCh8C1L-GUH15f4WFTWub2x+_NucngcDDcHKw@mail.gmail.com

Just adapt it to nbtree, by removing "USING GIST", and by forcing
plain index scans (concurrent TID recycling is prevented by means of
holding onto a leaf page buffer pin with index-only scans).

My guess is that adapting the test case like this will demonstrate
that you really do need to be prepared for concurrent TID recycling
that leads to accessing out-of-page-bounds TIDs in places like
index_fetch_heap().

--
Peter Geoghegan