From 4e213f29851ceee73f9b20846df84f6f9662714c Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Sat, 29 Feb 2020 19:33:21 -0800 Subject: [PATCH v1 2/2] 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 | 4 ++-- 6 files changed, 45 insertions(+), 5 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 29694b8aa4a..912cadeb03a 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -1137,6 +1137,8 @@ heap_beginscan(Relation relation, Snapshot snapshot, { HeapScanDesc scan; + Assert(SnapshotSet()); + /* * increment relation ref count while scanning relation * @@ -1546,7 +1548,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 */ @@ -5629,7 +5631,7 @@ heap_abort_speculative(Relation relation, ItemPointer tid) * RecentGlobalXmin. That's not pretty, but it doesn't seem worth * inventing a nicer API for this. */ - Assert(TransactionIdIsValid(RecentGlobalXmin)); + Assert(SnapshotSet()); PageSetPrunable(page, RecentGlobalXmin); /* store transaction information of xact deleting the tuple */ diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c index a5210d0b342..558b490d079 100644 --- a/src/backend/access/index/indexam.c +++ b/src/backend/access/index/indexam.c @@ -178,6 +178,8 @@ index_insert(Relation indexRelation, RELATION_CHECKS; CHECK_REL_PROCEDURE(aminsert); + Assert(SnapshotSet()); + if (!(indexRelation->rd_indam->ampredlocks)) CheckForSerializableConflictIn(indexRelation, (ItemPointer) NULL, @@ -250,6 +252,8 @@ index_beginscan_internal(Relation indexRelation, { IndexScanDesc scan; + Assert(SnapshotSet()); + RELATION_CHECKS; CHECK_REL_PROCEDURE(ambeginscan); @@ -513,7 +517,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 @@ -568,6 +572,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..f7e1665aae6 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 registered means invalidation + * processing can 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..50a46dca933 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 note on */ - Assert(TransactionIdIsValid(RecentGlobalXmin)); + Assert(SnapshotSet()); /* * Initialize state for entire verification operation -- 2.25.0.114.g5b0ca878e0