Re: old_snapshot_threshold vs indexes

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Kevin Grittner <kgrittn(at)gmail(dot)com>
Subject: Re: old_snapshot_threshold vs indexes
Date: 2019-08-26 21:28:57
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Thomas Munro <thomas(dot)munro(at)gmail(dot)com> writes:
> On my laptop, all prewarmed, no concurrency, the mere existence of 10
> brin indexes causes a sequential scan to take ~5% longer and an
> uncorrelated index scan to take ~45% longer (correlated index scans
> don't suffer). Here's a draft patch for v13 that fixes that problem
> by caching the result of RelationHasUnloggedIndex().

I agree that this code is absolutely horrid as it stands. However,
it doesn't look to me like caching RelationHasUnloggedIndex is quite
enough to fix it. The other problem is that the calls in question
seem to be mostly in TestForOldSnapshot, which is called in places
like heapgetpage:

LockBuffer(buffer, BUFFER_LOCK_SHARE);

dp = BufferGetPage(buffer);
TestForOldSnapshot(snapshot, scan->rs_base.rs_rd, dp);
lines = PageGetMaxOffsetNumber(dp);
ntup = 0;

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.

Your 0001 patch looks reasonable for the purpose of caching the
result, though.

regards, tom lane

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-08-26 21:51:21 Re: Misleading comment in tuplesort_set_bound
Previous Message Tom Lane 2019-08-26 20:53:25 Re: Check the number of potential synchronous standbys