Re: Issues with hash and GiST LP_DEAD setting for kill_prior_tuple

On Mon, Jul 21, 2025 at 9:29 PM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>
> On Thu, Jul 17, 2025 at 7:27 PM Mihail Nikalayeu
> <mihailnikalayeu(at)gmail(dot)com> wrote:
> > > FWIW _hash_readpage has a comment about a stashed LSN, so it seems as
> > > if this was barely missed by the work on hash indexes around 2017:
> >
> > I think commit 22c5e735 [0] (Remove lsn from HashScanPosData) is the
> > thing you are looking for in relation to hash.
>
> Oh yeah, good catch.
>
> I wonder if this is already correct. It definitely *looks* wrong,
> because _hash_kill_items() is prepared to re-pin and then LP_DEAD-mark
> a page, without the LSN check that would make that safe. But it seems
> possible that that never actually happens -- maybe the
> HashScanPosIsPinned() check will invariably find that the page is
> already pinned (in which case commit 22c5e735 merely neglected to
> remove some dead code from _hash_kill_items, but was otherwise
> correct).
>

As per my understanding, during scan, we retain the pin on the primary
bucket page and release the lock whereas for overflow pages we neither
retain pin nor lock. However, IIUC, this is still safe because of the
interlocking provided by vacuum scan. See comments atop
_hash_kill_items() [There are never any scans active in a bucket at
the time VACUUM begins ...]. I think we only need to worry about
vacuum processing the page after we have collected the killed_items
which shouldn't happen as explained in comments atop
_hash_kill_items().

--
With Regards,
Amit Kapila.