From 076c589dff7e08f0a6b562b185f179da4fbfc13a Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Mon, 6 Apr 2020 21:28:55 -0700 Subject: [PATCH v7 02/11] Improve and extend asserts for a snapshot being set. --- 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