Re: old_snapshot_threshold vs indexes

Thomas Munro <thomas(dot)munro(at)gmail(dot)com> writes:
> On Tue, Aug 27, 2019 at 9:28 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> It is hard to express what a bad idea it is to be asking for complex
>> catalog searches while holding a buffer lock. We could easily get
>> into undetectable deadlocks that way, for example. We need to refactor
>> these call sites to arrange that the catalog lookup happens outside
>> the low-level page access.

> Hmm. Right. Perhaps the theory was that it was OK because it's
> shared (rather than exclusive), or perhaps the catalog lookup was
> sufficiently well hidden and was forgotten.

I strongly suspect the latter. Also, it may well be that the
unlogged-index check was not in the original design but was
added later with insufficient thought about where it'd be called
from.

> 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.

regards, tom lane