agora inbox for [email protected]
help / color / mirror / Atom feedFrom: Andres Freund <[email protected]>
Subject: [PATCH v7 02/11] Improve and extend asserts for a snapshot being set.
Date: Mon, 6 Apr 2020 21:28:55 -0700
---
src/include/utils/snapmgr.h | 2 ++
src/backend/access/heap/heapam.c | 6 ++++--
src/backend/access/index/indexam.c | 8 +++++++-
src/backend/catalog/indexing.c | 11 +++++++++++
src/backend/utils/time/snapmgr.c | 19 +++++++++++++++++++
contrib/amcheck/verify_nbtree.c | 6 +++---
6 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h
index b28d13ce841..7738d6a8e01 100644
--- a/src/include/utils/snapmgr.h
+++ b/src/include/utils/snapmgr.h
@@ -116,6 +116,8 @@ extern void PopActiveSnapshot(void);
extern Snapshot GetActiveSnapshot(void);
extern bool ActiveSnapshotSet(void);
+extern bool SnapshotSet(void);
+
extern Snapshot RegisterSnapshot(Snapshot snapshot);
extern void UnregisterSnapshot(Snapshot snapshot);
extern Snapshot RegisterSnapshotOnOwner(Snapshot snapshot, ResourceOwner owner);
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index c4a5aa616a3..0af51880ccc 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -1136,6 +1136,8 @@ heap_beginscan(Relation relation, Snapshot snapshot,
{
HeapScanDesc scan;
+ Assert(SnapshotSet());
+
/*
* increment relation ref count while scanning relation
*
@@ -1545,7 +1547,7 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
at_chain_start = first_call;
skip = !first_call;
- Assert(TransactionIdIsValid(RecentGlobalXmin));
+ Assert(SnapshotSet());
Assert(BufferGetBlockNumber(buffer) == blkno);
/* Scan through possible multiple members of HOT-chain */
@@ -5633,7 +5635,7 @@ heap_abort_speculative(Relation relation, ItemPointer tid)
* if so (vacuum can't subsequently move relfrozenxid to beyond
* TransactionXmin, so there's no race here).
*/
- Assert(TransactionIdIsValid(TransactionXmin));
+ Assert(SnapshotSet() && TransactionIdIsValid(TransactionXmin));
if (TransactionIdPrecedes(TransactionXmin, relation->rd_rel->relfrozenxid))
prune_xid = relation->rd_rel->relfrozenxid;
else
diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
index a3f77169a79..5d6354dedf5 100644
--- a/src/backend/access/index/indexam.c
+++ b/src/backend/access/index/indexam.c
@@ -184,6 +184,8 @@ index_insert(Relation indexRelation,
RELATION_CHECKS;
CHECK_REL_PROCEDURE(aminsert);
+ Assert(SnapshotSet());
+
if (!(indexRelation->rd_indam->ampredlocks))
CheckForSerializableConflictIn(indexRelation,
(ItemPointer) NULL,
@@ -256,6 +258,8 @@ index_beginscan_internal(Relation indexRelation,
{
IndexScanDesc scan;
+ Assert(SnapshotSet());
+
RELATION_CHECKS;
CHECK_REL_PROCEDURE(ambeginscan);
@@ -519,7 +523,7 @@ index_getnext_tid(IndexScanDesc scan, ScanDirection direction)
SCAN_CHECKS;
CHECK_SCAN_PROCEDURE(amgettuple);
- Assert(TransactionIdIsValid(RecentGlobalXmin));
+ Assert(SnapshotSet());
/*
* The AM's amgettuple proc finds the next index entry matching the scan
@@ -574,6 +578,8 @@ index_fetch_heap(IndexScanDesc scan, TupleTableSlot *slot)
bool all_dead = false;
bool found;
+ Assert(SnapshotSet());
+
found = table_index_fetch_tuple(scan->xs_heapfetch, &scan->xs_heaptid,
scan->xs_snapshot, slot,
&scan->xs_heap_continue, &all_dead);
diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c
index d63fcf58cf1..8ba6b3dfa5e 100644
--- a/src/backend/catalog/indexing.c
+++ b/src/backend/catalog/indexing.c
@@ -22,6 +22,7 @@
#include "catalog/indexing.h"
#include "executor/executor.h"
#include "utils/rel.h"
+#include "utils/snapmgr.h"
/*
@@ -184,6 +185,8 @@ CatalogTupleInsert(Relation heapRel, HeapTuple tup)
{
CatalogIndexState indstate;
+ Assert(SnapshotSet());
+
indstate = CatalogOpenIndexes(heapRel);
simple_heap_insert(heapRel, tup);
@@ -204,6 +207,8 @@ void
CatalogTupleInsertWithInfo(Relation heapRel, HeapTuple tup,
CatalogIndexState indstate)
{
+ Assert(SnapshotSet());
+
simple_heap_insert(heapRel, tup);
CatalogIndexInsert(indstate, tup);
@@ -225,6 +230,8 @@ CatalogTupleUpdate(Relation heapRel, ItemPointer otid, HeapTuple tup)
{
CatalogIndexState indstate;
+ Assert(SnapshotSet());
+
indstate = CatalogOpenIndexes(heapRel);
simple_heap_update(heapRel, otid, tup);
@@ -245,6 +252,8 @@ void
CatalogTupleUpdateWithInfo(Relation heapRel, ItemPointer otid, HeapTuple tup,
CatalogIndexState indstate)
{
+ Assert(SnapshotSet());
+
simple_heap_update(heapRel, otid, tup);
CatalogIndexInsert(indstate, tup);
@@ -268,5 +277,7 @@ CatalogTupleUpdateWithInfo(Relation heapRel, ItemPointer otid, HeapTuple tup,
void
CatalogTupleDelete(Relation heapRel, ItemPointer tid)
{
+ Assert(SnapshotSet());
+
simple_heap_delete(heapRel, tid);
}
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index b5cff157bf6..3b148ae30a6 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -857,6 +857,25 @@ ActiveSnapshotSet(void)
return ActiveSnapshot != NULL;
}
+/*
+ * Does this transaction have a snapshot.
+ */
+bool
+SnapshotSet(void)
+{
+ /* can't be safe, because somehow xmin is not set */
+ if (!TransactionIdIsValid(MyPgXact->xmin) && HistoricSnapshot == NULL)
+ return false;
+
+ /*
+ * Can't be safe because no snapshot being active/registered means that
+ * e.g. invalidation processing could change xmin horizon.
+ */
+ return ActiveSnapshot != NULL ||
+ !pairingheap_is_empty(&RegisteredSnapshots) ||
+ HistoricSnapshot != NULL;
+}
+
/*
* RegisterSnapshot
* Register a snapshot as being in use by the current resource owner
diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
index ceaaa271680..8f43f3e9dfb 100644
--- a/contrib/amcheck/verify_nbtree.c
+++ b/contrib/amcheck/verify_nbtree.c
@@ -412,10 +412,10 @@ bt_check_every_level(Relation rel, Relation heaprel, bool heapkeyspace,
Snapshot snapshot = SnapshotAny;
/*
- * RecentGlobalXmin assertion matches index_getnext_tid(). See note on
- * RecentGlobalXmin/B-Tree page deletion.
+ * This assertion matches the one in index_getnext_tid(). See page
+ * recycling/RecentGlobalXmin notes in nbtree README.
*/
- Assert(TransactionIdIsValid(RecentGlobalXmin));
+ Assert(SnapshotSet());
/*
* Initialize state for entire verification operation
--
2.25.0.114.g5b0ca878e0
--filbd3fs2o265vea
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0003-Fix-xlogreader-fd-leak-encountered-with-twophase-.patch"
view thread (5+ messages) latest in thread
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: [email protected]
Cc: [email protected]
Subject: Re: [PATCH v7 02/11] Improve and extend asserts for a snapshot being set.
In-Reply-To: <no-message-id-195058@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