Re: old_snapshot_threshold vs indexes - Mailing list pgsql-hackers

From Tom Lane
Subject Re: old_snapshot_threshold vs indexes
Date
Msg-id 24821.1566854937@sss.pgh.pa.us
Whole thread Raw
In response to Re: old_snapshot_threshold vs indexes  (Thomas Munro <thomas.munro@gmail.com>)
Responses Re: old_snapshot_threshold vs indexes
List pgsql-hackers
Thomas Munro <thomas.munro@gmail.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



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Check the number of potential synchronous standbys
Next
From: Tom Lane
Date:
Subject: Re: Misleading comment in tuplesort_set_bound