On Thu, Aug 25, 2016 at 2:56 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> I'm wondering about the TestForOldSnapshot call in scanPendingInsert().
> Why do we apply it to the metapage buffer (line 1717 in master)?
If there is any chance that GinPageGetMeta(page)->head could have
changed from a valid block number to InvalidBlockNumber or a
different pending-list page due to a vacuum freeing pages from the
pending-list, the metapage must be checked -- there is no other way
to detect that data might have disappeared.
> Shouldn't we apply it to the pending-list pages themselves only, if any?
If the pending-list pages are never freed, we could drop the
metapage test.
> (If there are no pending-list pages, surely the age of the snapshot used
> to read the metapage doesn't matter; and if there are, then the age of
> the pending-list pages will fail the test.)
True as long as no pending-list page is ever removed from the
pending-list. If we take out the test, it might be worth a comment
explaining why it's not needed and that it will be needed if
someone modifies the AM to recycle empty pages from the list for
reuse.
> FWIW I like the "revert" commit, because it easily shows me in what
> places you considered a snapshot-too-old test and decided not to add
> one. Bare BufferGetPage calls (the current situation) don't indicate that.
I'm glad there is some value from having done that little dance. :-)
Thanks for looking this over so closely!
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
