agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Antonin Houska <ah@cybertec.at>
To: Andres Freund <andres@anarazel.de>
Cc: Kirill Reshke <reshkekirill@gmail.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Melanie Plageman <melanieplageman@gmail.com>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>
Cc: pgsql-hackers@postgresql.org, Thomas Munro <thomas.munro@gmail.com>
Cc: Noah Misch <noah@leadboat.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Michael Paquier <michael.paquier@gmail.com>
Subject: Re: Buffer locking is special (hints, checksums, AIO writes)
Date: Fri, 13 Mar 2026 16:01:40 +0100
Message-ID: <38845.1773414100@localhost> (raw)
In-Reply-To: <tc2rw3axtdvxyo7ktjlxvi3di4xi3zqak4st4fveqcblamci2f@hgyllvwarq6f>
References: <ossv2eistssmubfsir6xjll76tynvxv5lup4zkrfzjkud7fycw@rf5vii6l6cha>
<4csodkvvfbfloxxjlkgsnl2lgfv2mtzdl7phqzd4jxjadxm4o5@usw7feyb5bzf>
<CALdSSPgyc7VuMLUZ8J7v2G-PebBa_vEi+mp-cLkcOwNycB56Hw@mail.gmail.com>
<kjdgvws34gpnz4y6xu5aaeul5mspgy3ahvyiw4lndb3ecsacdb@b2ccyowotpqh>
<jtg5cu4n6h5lib3kzx66ju4yhh6kmviaud7oq6dtut6c4q4rdi@xwsfoagt3c2b>
<61812.1770637345@localhost>
<aYpbjeiN2lKEBBn1@alap3.anarazel.de>
<19720.1770709587@localhost>
<aYtg622xasbSju7_@alap3.anarazel.de>
<196082.1770892568@localhost>
<tc2rw3axtdvxyo7ktjlxvi3di4xi3zqak4st4fveqcblamci2f@hgyllvwarq6f>
Andres Freund <andres@anarazel.de> wrote:
> Probably need to update the comments a bit. What about something like
>
>
> /*
> * Is the snapshot implemented as an MVCC snapshot (i.e. it uses
> * SNAPSHOT_MVCC). If so, there will be at most be one visible row in a chain
> * of updated tuples, and each visible tuple will be seen exactly once.
> */
> #define IsMVCCSnapshot(snapshot) \
The ", and each visible tuple ..." part seemed to me redundant, so I omitted
it. If you think I'm wrong, please add it yourself when committing the patch.
I also added a comment to the IsHistoricMVCCSnapshot(), trying to explain what
"historic" means.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
Attachments:
[text/x-diff] v2-Refine-checking-of-snapshot-type.patch (3.9K, ../38845.1773414100@localhost/2-v2-Refine-checking-of-snapshot-type.patch)
download | inline diff:
From 6c391d437ec61ff16b2a459a799d2030142e656f Mon Sep 17 00:00:00 2001
From: Antonin Houska <ah@cybertec.at>
Date: Thu, 12 Feb 2026 11:14:00 +0100
Subject: [PATCH] Refine checking of snapshot type.
It appears to be confusing if IsMVCCSnapshot() evaluates to true for both
"regular" and "historic" MVCC snapshot. This patch restricts the meaning of
the macro to the "regular" MVCC snapshot, and introduces a new macro
IsMVCCLikeSnapshot() to recognize both types.
IsMVCCLikeSnapshot() is only used in functions that can (supposedly) be called
during logical decoding.
---
src/backend/access/heap/heapam_handler.c | 2 +-
src/backend/access/index/indexam.c | 2 +-
src/backend/access/nbtree/nbtree.c | 2 +-
src/include/utils/snapmgr.h | 21 ++++++++++++++++++---
4 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 5137d2510ea..2d1ee9ac95d 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -159,7 +159,7 @@ heapam_index_fetch_tuple(struct IndexFetchTableData *scan,
* Only in a non-MVCC snapshot can more than one member of the HOT
* chain be visible.
*/
- *call_again = !IsMVCCSnapshot(snapshot);
+ *call_again = !IsMVCCLikeSnapshot(snapshot);
slot->tts_tableOid = RelationGetRelid(scan->rel);
ExecStoreBufferHeapTuple(&bslot->base.tupdata, slot, hscan->xs_cbuf);
diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
index 43f64a0e721..5eb7e99ad3e 100644
--- a/src/backend/access/index/indexam.c
+++ b/src/backend/access/index/indexam.c
@@ -445,7 +445,7 @@ index_markpos(IndexScanDesc scan)
void
index_restrpos(IndexScanDesc scan)
{
- Assert(IsMVCCSnapshot(scan->xs_snapshot));
+ Assert(IsMVCCLikeSnapshot(scan->xs_snapshot));
SCAN_CHECKS;
CHECK_SCAN_PROCEDURE(amrestrpos);
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 6d0a6f27f3f..cdd81d147cc 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -423,7 +423,7 @@ btrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
* Note: so->dropPin should never change across rescans.
*/
so->dropPin = (!scan->xs_want_itup &&
- IsMVCCSnapshot(scan->xs_snapshot) &&
+ IsMVCCLikeSnapshot(scan->xs_snapshot) &&
RelationNeedsWAL(scan->indexRelation) &&
scan->heapRelation != NULL);
diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h
index b8c01a291a1..4f1e910bc75 100644
--- a/src/include/utils/snapmgr.h
+++ b/src/include/utils/snapmgr.h
@@ -51,14 +51,29 @@ extern PGDLLIMPORT SnapshotData SnapshotToastData;
((snapshotdata).snapshot_type = SNAPSHOT_NON_VACUUMABLE, \
(snapshotdata).vistest = (vistestp))
-/* This macro encodes the knowledge of which snapshots are MVCC-safe */
+/*
+ * Is the snapshot implemented as an MVCC snapshot (i.e. it uses
+ * SNAPSHOT_MVCC)? If so, there will be at most one visible tuple in a chain
+ * of updated tuples.
+ */
#define IsMVCCSnapshot(snapshot) \
- ((snapshot)->snapshot_type == SNAPSHOT_MVCC || \
- (snapshot)->snapshot_type == SNAPSHOT_HISTORIC_MVCC)
+ ((snapshot)->snapshot_type == SNAPSHOT_MVCC)
+/*
+ * Special kind of MVCC snapshot, to be used during logical decoding. The
+ * visibility is checked from the perspective of an already committed
+ * transaction, which we're trying to decode.
+ */
#define IsHistoricMVCCSnapshot(snapshot) \
((snapshot)->snapshot_type == SNAPSHOT_HISTORIC_MVCC)
+/*
+ * Is the snapshot either an MVCC snapshot or has equivalent visibility
+ * semantics (see IsMVCCSnapshot())?
+ */
+#define IsMVCCLikeSnapshot(snapshot) \
+ (IsMVCCSnapshot(snapshot) || IsHistoricMVCCSnapshot(snapshot))
+
extern Snapshot GetTransactionSnapshot(void);
extern Snapshot GetLatestSnapshot(void);
extern void SnapshotSetCommandId(CommandId curcid);
--
2.47.3
view thread (120+ messages) latest in thread
Message-ID: <38845.1773414100@localhost>
Permalink: ../38845.1773414100@localhost/
Also on: postgresql.org/message-id/38845.1773414100@localhost
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: ah@cybertec.at, andres@anarazel.de, reshkekirill@gmail.com, hlinnaka@iki.fi, melanieplageman@gmail.com, boekewurm+postgres@gmail.com, thomas.munro@gmail.com, noah@leadboat.com, robertmhaas@gmail.com, michael.paquier@gmail.com
Subject: Re: Buffer locking is special (hints, checksums, AIO writes)
In-Reply-To: <38845.1773414100@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox