Re: old_snapshot_threshold vs indexes

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