On Thu, Aug 25, 2016 at 2:56 PM, Alvaro Herrera <alvhe...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
