Horton

Re: A few patches to clarify snapshot management

Heikki Linnakangas <hlinnaka@iki.fi>

View thread
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Noah Misch <noah@leadboat.com>
Cc: Andres Freund <andres@anarazel.de>, Nathan Bossart <nathandbossart@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2025-08-22T10:36:29Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Use ereport() rather than elog()

  2. Revert GetTransactionSnapshot() to return historic snapshot during LR

  3. Improve snapmgr.c comment

  4. Assert that a snapshot is active or registered before it's used

  5. Don't allow GetTransactionSnapshot() in logical decoding

  6. Remove unnecessary GetTransactionSnapshot() calls

  7. snapshot scalability: Don't compute global horizons while building snapshots.

On 20/08/2025 03:37, Noah Misch wrote:
> On Tue, Aug 19, 2025 at 11:45:01PM +0300, Heikki Linnakangas wrote:
>> How about the attached, then? It reverts the GetTransactionSnapshot()
>> change. But to still catch at least some of the invalid uses of the historic
>> snapshot, it adds checks to heap_beginscan() and index_beginscan(), to
>> complain if they are called on a non-catalog relation with a historic
>> snapshot.
> 
>> @@ -1143,6 +1143,15 @@ heap_beginscan(Relation relation, Snapshot snapshot,
>>   	if (!(snapshot && IsMVCCSnapshot(snapshot)))
>>   		scan->rs_base.rs_flags &= ~SO_ALLOW_PAGEMODE;
>>   
>> +	/* Check that a historic snapshot is not used for non-catalog tables */
>> +	if (snapshot &&
>> +		IsHistoricMVCCSnapshot(snapshot) &&
>> +		!RelationIsAccessibleInLogicalDecoding(relation))
>> +	{
>> +		elog(ERROR, "cannot query non-catalog table \"%s\" during logical decoding",
>> +			 RelationGetRelationName(relation));
>> +	}
>> +
> 
> I feel post-beta3 is late for debut of restrictions like this.  How about a
> pure revert, then add those restrictions in v19?  Should be s/elog/ereport/,
> also.

Ok, fair. I committed the revert to v18, and the revert + additional 
checks to master.

- Heikki