Re: A few patches to clarify snapshot management

Heikki Linnakangas <hlinnaka@iki.fi>

From: Heikki Linnakangas <hlinnaka@iki.fi>

To: Nathan Bossart <nathandbossart@gmail.com>

Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>

Date: 2025-01-06T21:30:33Z

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 →

Use ereport() rather than elog()
- 661f821ef0c3 19 (unreleased) landed
Revert GetTransactionSnapshot() to return historic snapshot during LR
- 27f20441c5e7 18.0 landed
- 50f770c3d92c 19 (unreleased) landed
Improve snapmgr.c comment
- 043745c3a01f 18.0 landed
Assert that a snapshot is active or registered before it's used
- 8076c00592e4 18.0 landed
Don't allow GetTransactionSnapshot() in logical decoding
- 1585ff7387db 18.0 landed
Remove unnecessary GetTransactionSnapshot() calls
- 952365cded63 18.0 landed
snapshot scalability: Don't compute global horizons while building snapshots.
- dc7420c2c927 14.0 cited

Attachments

v1-0001-Add-comment-with-more-details-on-active-snapshots.patch (text/x-patch) patch v1-0001
v1-0002-Add-assertions.patch (text/x-patch) patch v1-0002
v1-0003-wip-Split-SnapshotData-into-multiple-structs.patch (text/x-patch) patch v1-0003

On 20/12/2024 19:31, Heikki Linnakangas wrote:
>> /*
>>  * Struct representing all kind of possible snapshots.
>>  *
>>  * There are several different kinds of snapshots:
>>  * * Normal MVCC snapshots
>>  * * MVCC snapshots taken during recovery (in Hot-Standby mode)
>>  * * Historic MVCC snapshots used during logical decoding
>>  * * snapshots passed to HeapTupleSatisfiesDirty()
>>  * * snapshots passed to HeapTupleSatisfiesNonVacuumable()
>>  * * snapshots used for SatisfiesAny, Toast, Self where no members are
>>  *     accessed.
>>  *
>>  * TODO: It's probably a good idea to split this struct using a NodeTag
>>  * similar to how parser and executor nodes are handled, with one type 
>> for
>>  * each different kind of snapshot to avoid overloading the meaning of
>>  * individual fields.
>>  */
>> typedef struct SnapshotData
> 
> I'm thinking of implementing that TODO, splitting SnapshotData into 
> separate structs like MVCCSnapshotData, SnapshotDirtyData, etc. It seems 
> to me most places can assume that you're dealing with MVCC snapshots, 
> and if we had separate types for them, could be using MVCCSnapshot 
> instead of the generic Snapshot. Only the table and index AM functions 
> need to deal with non-MVCC snapshots.

Here's a draft of that. Going through this exercise clarified a few 
things to me that I didn't realize before:

- The executor only deals with MVCC snapshots. Special snapshots are 
only for the lower-level AM interfaces.
- Only MVCC snapshots can be pushed to the active stack
- Only MVCC or historic MVCC snapshots can be registered with a resource 
owner

Thoughts?

-- 
Heikki Linnakangas
Neon (https://neon.tech)