On Tue, Aug 27, 2019 at 10:59 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > At first glance it seems
> > like we need to capture PageGetLSN(page) while we have the lock, and
> > then later pass that into TestForOldSnapshot() instead of the page.
> > I'll look into that and write a patch, probably in a day or two.
>
> Hm, but surely we need to do other things to the page besides
> TestForOldSnapshot? I was imagining that we'd collect the
> RelationHasUnloggedIndex flag (or perhaps better, the
> RelationAllowsEarlyPruning result) before attempting to lock
> the page, and then pass it to TestForOldSnapshot.
OK I started writing a patch and realised there were a few ugly
problems that I was about to report here... but now I wonder if, based
on the comment for RelationHasUnloggedIndex(), we shouldn't just nuke
all this code. We don't actually support unlogged indexes on
permanent tables (there is no syntax to create them, and
RelationHasUnloggedIndex() will never return true in practice because
RelationNeedsWAL() will always return false first). This is a locking
protocol violation and a performance pessimisation in support of a
feature we don't have. If we add support for that in some future
release, we can figure out how to do it properly then, no?
--
Thomas Munro
https://enterprisedb.com