Re: snapshot too old issues, first around wraparound and then more.

On Wed, Apr 1, 2020 at 5:54 PM Andres Freund <andres@anarazel.de> wrote:
> As far as I can tell there's not sufficient in-tree explanation of when
> code needs to test for an old snapshot. There's just the following
> comment above TestForOldSnapshot():
>  * Check whether the given snapshot is too old to have safely read the given
>  * page from the given table.  If so, throw a "snapshot too old" error.
>  *
>  * This test generally needs to be performed after every BufferGetPage() call
>  * that is executed as part of a scan.  It is not needed for calls made for
>  * modifying the page (for example, to position to the right place to insert a
>  * new index tuple or for vacuuming).  It may also be omitted where calls to
>  * lower-level functions will have already performed the test.
>
> But I don't find "as part of a scan" very informative.

I also find it strange that _bt_search() calls TestForOldSnapshot() on
every level on the tree (actually, it calls _bt_moveright() which
calls it on every level of the tree). At least with reads (see the
comments at the top of _bt_moveright()).

Why do we need to do the test on internal pages? We only ever call
PredicateLockPage() on a leaf nbtree page. Why the inconsistency
between the two similar-seeming cases?

-- 
Peter Geoghegan