On Wed, Apr 1, 2020 at 5:54 PM Andres Freund <and...@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
