From 4b3f741562590978b1bd1ed75152c08f922e22d3 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Mon, 31 Mar 2025 23:47:48 +0300 Subject: [PATCH v2 4/5] 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 | 16 ++++++++++++++++ src/include/utils/snapshot.h | 1 + 3 files changed, 19 insertions(+) diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 2292d8a09af..29a743ee49c 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -2083,6 +2083,7 @@ GetSnapshotDataReuse(MVCCSnapshot snapshot) snapshot->active_count = 0; snapshot->regd_count = 0; snapshot->copied = false; + snapshot->valid = true; return true; } @@ -2462,6 +2463,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 f16f3fd48e5..ecddb7f97da 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -459,6 +459,7 @@ InvalidateCatalogSnapshot(void) { pairingheap_remove(&RegisteredSnapshots, &CatalogSnapshot->ph_node); CatalogSnapshot = NULL; + CatalogSnapshotData.valid = false; SnapshotResetXmin(); INJECTION_POINT("invalidate-catalog-snapshot-end", NULL); } @@ -624,6 +625,7 @@ CopyMVCCSnapshot(MVCCSnapshot snapshot) newsnap->regd_count = 0; newsnap->active_count = 0; newsnap->copied = true; + newsnap->valid = true; newsnap->snapXactCompletionCount = 0; /* setup XID array */ @@ -665,6 +667,7 @@ FreeMVCCSnapshot(MVCCSnapshot snapshot) Assert(snapshot->regd_count == 0); Assert(snapshot->active_count == 0); Assert(snapshot->copied); + Assert(snapshot->valid); pfree(snapshot); } @@ -710,6 +713,8 @@ PushActiveSnapshotWithLevel(Snapshot snapshot, int snap_level) MVCCSnapshot origsnap = &snapshot->mvcc; MVCCSnapshot newsnap; + Assert(origsnap->valid); + /* * Checking SecondarySnapshot is probably useless here, but it seems * better to be sure. @@ -907,6 +912,7 @@ RegisterSnapshotOnOwner(Snapshot orig_snapshot, ResourceOwner owner) Assert(orig_snapshot->base.snapshot_type == SNAPSHOT_MVCC); snapshot = &orig_snapshot->mvcc; + Assert(snapshot->valid); /* Static snapshot? Create a persistent copy */ snapshot = snapshot->copied ? snapshot : CopyMVCCSnapshot(snapshot); @@ -1028,6 +1034,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; @@ -1946,6 +1961,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 61f675f86b6..ff07ec5aa51 100644 --- a/src/include/utils/snapshot.h +++ b/src/include/utils/snapshot.h @@ -178,6 +178,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.47.3