From 1705639a73555d9b3f5884c7fd90540c268d3db5 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Mon, 31 Mar 2025 23:47:48 +0300 Subject: [PATCH v6 03/12] Add an explicit 'valid' flag to MVCCSnapshotData The lifetime of the "static" snapshots returned by GetTransactionSnapshot(), GetLatestSnapshot() and GetCatalogSnapshot() is a bit vague. By adding an explicit 'valid' flag, we can make it more clear when a function call updates a static snapshot, making it valid, and when another function makes it invalid again. It's currently only used in assertions, and can also be handy when debugging. --- src/backend/storage/ipc/procarray.c | 2 ++ src/backend/utils/time/snapmgr.c | 15 +++++++++++++++ src/include/utils/snapshot.h | 1 + 3 files changed, 18 insertions(+) diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 535755614a9..ba5ed8960dd 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -2135,6 +2135,7 @@ GetSnapshotDataReuse(MVCCSnapshot snapshot) snapshot->active_count = 0; snapshot->regd_count = 0; snapshot->copied = false; + snapshot->valid = true; return true; } @@ -2514,6 +2515,7 @@ GetSnapshotData(MVCCSnapshot snapshot) snapshot->active_count = 0; snapshot->regd_count = 0; snapshot->copied = false; + snapshot->valid = true; return snapshot; } diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 78adb6d575a..69ed86b2101 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -447,6 +447,7 @@ InvalidateCatalogSnapshot(void) { pairingheap_remove(&RegisteredSnapshots, &CatalogSnapshot->ph_node); CatalogSnapshot = NULL; + CatalogSnapshotData.valid = false; SnapshotResetXmin(); } } @@ -611,6 +612,7 @@ CopyMVCCSnapshot(MVCCSnapshot snapshot) newsnap->regd_count = 0; newsnap->active_count = 0; newsnap->copied = true; + newsnap->valid = true; newsnap->snapXactCompletionCount = 0; /* setup XID array */ @@ -652,6 +654,7 @@ FreeMVCCSnapshot(MVCCSnapshot snapshot) Assert(snapshot->regd_count == 0); Assert(snapshot->active_count == 0); Assert(snapshot->copied); + Assert(snapshot->valid); pfree(snapshot); } @@ -688,6 +691,7 @@ PushActiveSnapshotWithLevel(Snapshot snapshot, int snap_level) Assert(snapshot->snapshot_type == SNAPSHOT_MVCC); origsnap = &snapshot->mvcc; + Assert(origsnap->valid); Assert(ActiveSnapshot == NULL || snap_level >= ActiveSnapshot->as_level); @@ -847,6 +851,7 @@ RegisterSnapshotOnOwner(Snapshot orig_snapshot, ResourceOwner owner) Assert(orig_snapshot->snapshot_type == SNAPSHOT_MVCC); snapshot = &orig_snapshot->mvcc; + Assert(snapshot->valid); /* Static snapshot? Create a persistent copy */ snapshot = snapshot->copied ? snapshot : CopyMVCCSnapshot(snapshot); @@ -968,6 +973,15 @@ SnapshotResetXmin(void) { MVCCSnapshot minSnapshot; + /* + * These static snapshots are not in the RegisteredSnapshots list, so we + * might advance MyProc->xmin past their xmin. (Note that in case of + * IsolationUsesXactSnapshot() == true, CurrentSnapshot points to the copy + * in FirstSnapshot rather than CurrentSnapshotData.) + */ + CurrentSnapshotData.valid = false; + SecondarySnapshotData.valid = false; + if (ActiveSnapshot != NULL) return; @@ -1871,6 +1885,7 @@ RestoreSnapshot(char *start_address) snapshot->regd_count = 0; snapshot->active_count = 0; snapshot->copied = true; + snapshot->valid = true; return snapshot; } diff --git a/src/include/utils/snapshot.h b/src/include/utils/snapshot.h index bca0ad16e68..1697c6df856 100644 --- a/src/include/utils/snapshot.h +++ b/src/include/utils/snapshot.h @@ -161,6 +161,7 @@ typedef struct MVCCSnapshotData bool takenDuringRecovery; /* recovery-shaped snapshot? */ bool copied; /* false if it's a static snapshot */ + bool valid; /* is this snapshot valid? */ CommandId curcid; /* in my xact, CID < curcid are visible */ -- 2.39.5