Improve and extend asserts for a snapshot being set.
authorAndres Freund <[email protected]>
Tue, 7 Apr 2020 04:28:55 +0000 (21:28 -0700)
committerAndres Freund <[email protected]>
Tue, 7 Apr 2020 04:28:55 +0000 (21:28 -0700)
contrib/amcheck/verify_nbtree.c
src/backend/access/heap/heapam.c
src/backend/access/index/indexam.c
src/backend/catalog/indexing.c
src/backend/utils/time/snapmgr.c
src/include/utils/snapmgr.h

index ceaaa271680c22829cc494e9c05e9dbcb6f4d91b..8f43f3e9dfb0e457145dd8cd1d2aefe1fa410672 100644 (file)
@@ -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
index c4a5aa616a340976caa188dd8c38f2900f64d4a2..0af51880cccb442fbaa4031e43e5859677c2e061 100644 (file)
@@ -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
index a3f77169a79d6300fc37a2facbacb6fb491e3c11..5d6354dedf575ab3f28102f1930b14408317abfb 100644 (file)
@@ -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);
index d63fcf58cf1dd3914609aea178b84ff0bd7075bf..8ba6b3dfa5edc6677a927b31322f2400173c4286 100644 (file)
@@ -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);
 }
index b5cff157bf63012cea892ce94465169bf194ef4b..3b148ae30a66ce6a69b91903b40d566041d99884 100644 (file)
@@ -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
index b28d13ce841e7be5a6842022e338a152b71ad8ac..7738d6a8e01bb80adfd6296e9837127348265b90 100644 (file)
@@ -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);