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

On Wed, Apr 1, 2020 at 5:54 PM Andres Freund <andres(at)anarazel(dot)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