From 7acb74c2e149ce3b8500924732a3b2a16f425ee6 Mon Sep 17 00:00:00 2001 From: Mikhail Nikalayeu Date: Fri, 9 Oct 2026 22:06:58 +0200 Subject: [PATCH v8 2/7] Periodically reset snapshots in concurrent index builds Long-running concurrent index builds hold back vacuum by pinning xmin. Replace the snapshot periodically during the first heap scan so the horizon can advance. The earlier attempt to let vacuum ignore these snapshots (d9d0762, reverted by e28bb88) was unsafe because HOT pruning could cause the build to miss tuples. Add the SO_RESET_SNAPSHOT scan option and let index access methods opt in with amcanresetsnapshot. RotateSnapshot() replaces the active snapshot and releases the catalog snapshot while keeping xmin valid throughout. Reset at page boundaries after concurrent_index_reset_snapshot_interval milliseconds; 0 resets on every page and -1 disables resets. Skip resets at REPEATABLE READ or above, where the transaction snapshot pins xmin anyway. For now, enable this only for serial, non-unique builds, excluding GIN and BRIN. Later commits add those cases. The validation scan still needs a single snapshot. Reviewed-by: Matthias van de Meent --- contrib/amcheck/verify_nbtree.c | 3 +- contrib/bloom/blutils.c | 1 + contrib/pgstattuple/pgstattuple.c | 2 +- doc/src/sgml/config.sgml | 21 ++ doc/src/sgml/indexam.sgml | 12 + src/backend/access/brin/brin.c | 2 + src/backend/access/gin/ginutil.c | 2 + src/backend/access/gist/gist.c | 1 + src/backend/access/hash/hash.c | 1 + src/backend/access/heap/heapam.c | 72 ++++++ src/backend/access/heap/heapam_handler.c | 49 +++- src/backend/access/index/genam.c | 2 +- src/backend/access/nbtree/nbtree.c | 1 + src/backend/access/spgist/spgutils.c | 1 + src/backend/catalog/index.c | 80 ++++++- src/backend/commands/indexcmds.c | 14 +- src/backend/nodes/makefuncs.c | 1 + src/backend/utils/misc/guc_parameters.dat | 10 + src/backend/utils/time/snapmgr.c | 84 +++++++ src/bin/pg_amcheck/t/006_cic.pl | 3 + src/include/access/amapi.h | 6 + src/include/access/heapam.h | 1 + src/include/access/tableam.h | 22 +- src/include/miscadmin.h | 1 + src/include/nodes/execnodes.h | 7 +- src/include/utils/snapmgr.h | 4 + src/include/utils/snapshot.h | 12 + .../modules/dummy_index_am/dummy_index_am.c | 1 + src/test/modules/injection_points/Makefile | 2 +- .../expected/cic_reset_snapshots.out | 213 ++++++++++++++++++ src/test/modules/injection_points/meson.build | 1 + .../sql/cic_reset_snapshots.sql | 143 ++++++++++++ 32 files changed, 737 insertions(+), 38 deletions(-) create mode 100644 src/test/modules/injection_points/expected/cic_reset_snapshots.out create mode 100644 src/test/modules/injection_points/sql/cic_reset_snapshots.sql diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c index 97c519b47f..24158f1fe1 100644 --- a/contrib/amcheck/verify_nbtree.c +++ b/contrib/amcheck/verify_nbtree.c @@ -544,7 +544,8 @@ bt_check_every_level(Relation rel, Relation heaprel, bool heapkeyspace, 0, /* number of keys */ NULL, /* scan key */ true, /* buffer access strategy OK */ - true); /* syncscan OK? */ + true, /* syncscan OK? */ + false); /* reset_snapshot OK? */ /* * Scan will behave as the first scan of a CREATE INDEX CONCURRENTLY diff --git a/contrib/bloom/blutils.c b/contrib/bloom/blutils.c index 5111cdc6dd..b8cd719069 100644 --- a/contrib/bloom/blutils.c +++ b/contrib/bloom/blutils.c @@ -123,6 +123,7 @@ blhandler(PG_FUNCTION_ARGS) .ampredlocks = false, .amcanparallel = false, .amcanbuildparallel = false, + .amcanresetsnapshot = true, .amcaninclude = false, .amusemaintenanceworkmem = false, .amparallelvacuumoptions = diff --git a/contrib/pgstattuple/pgstattuple.c b/contrib/pgstattuple/pgstattuple.c index 6a7f8cb4a7..a0e7ed2e13 100644 --- a/contrib/pgstattuple/pgstattuple.c +++ b/contrib/pgstattuple/pgstattuple.c @@ -335,7 +335,7 @@ pgstat_heap(Relation rel, FunctionCallInfo fcinfo) errmsg("only heap AM is supported"))); /* Disable syncscan because we assume we scan from block zero upwards */ - scan = table_beginscan_strat(rel, SnapshotAny, 0, NULL, true, false); + scan = table_beginscan_strat(rel, SnapshotAny, 0, NULL, true, false, false); hscan = (HeapScanDesc) scan; InitDirtySnapshot(SnapshotDirty); diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index f7db2995c3..99f54395ce 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -12780,6 +12780,27 @@ dynamic_library_path = '/usr/local/lib/postgresql:$libdir' + + concurrent_index_reset_snapshot_interval (integer) + + concurrent_index_reset_snapshot_interval configuration parameter + + + + + Sets the minimum time between snapshot resets during the first heap + scan of CREATE INDEX CONCURRENTLY and + REINDEX CONCURRENTLY. The scan resets its snapshot + at the next page boundary after this interval, allowing the xmin + horizon to advance. + If no unit is specified, the value is in milliseconds. The default + is 100ms. A value of 0 resets at + every opportunity, including the start of the scan, and is useful + for testing. -1 disables resets. + + + + backtrace_functions (string) diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml index f48da31853..c58efc639d 100644 --- a/doc/src/sgml/indexam.sgml +++ b/doc/src/sgml/indexam.sgml @@ -137,6 +137,8 @@ typedef struct IndexAmRoutine bool amcanparallel; /* does AM support parallel build? */ bool amcanbuildparallel; + /* does AM support snapshot resets during concurrent builds? */ + bool amcanresetsnapshot; /* does AM support columns included with clause INCLUDE? */ bool amcaninclude; /* does AM use maintenance_work_mem? */ @@ -312,6 +314,16 @@ ambuild (Relation heapRelation, this flag set to false. + + Set amcanresetsnapshot to true + if ambuild supports periodic snapshot resets + during concurrent builds. The table scan may switch to a newer MVCC + snapshot between pages so that xmin can advance. Copy any values retained + after the build callback returns, fetching external TOAST data rather than + retaining pointers to it. The access method must not hold an additional + snapshot across the scan. + + void diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 5059da8dce..bf6fad98f6 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -274,6 +274,8 @@ brinhandler(PG_FUNCTION_ARGS) .ampredlocks = false, .amcanparallel = false, .amcanbuildparallel = true, + /* not yet: a range summary keeps values across many heap pages */ + .amcanresetsnapshot = false, .amcaninclude = false, .amusemaintenanceworkmem = false, .amsummarizing = true, diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c index e7cba81d47..2a961ad61b 100644 --- a/src/backend/access/gin/ginutil.c +++ b/src/backend/access/gin/ginutil.c @@ -59,6 +59,8 @@ ginhandler(PG_FUNCTION_ARGS) .ampredlocks = true, .amcanparallel = false, .amcanbuildparallel = true, + /* not yet: the build accumulator keeps keys across heap pages */ + .amcanresetsnapshot = false, .amcaninclude = false, .amusemaintenanceworkmem = true, .amsummarizing = false, diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index 8565e225be..3fac805c26 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -79,6 +79,7 @@ gisthandler(PG_FUNCTION_ARGS) .ampredlocks = true, .amcanparallel = false, .amcanbuildparallel = false, + .amcanresetsnapshot = true, .amcaninclude = true, .amusemaintenanceworkmem = false, .amsummarizing = false, diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c index 63846c1084..584fb5b78e 100644 --- a/src/backend/access/hash/hash.c +++ b/src/backend/access/hash/hash.c @@ -90,6 +90,7 @@ hashhandler(PG_FUNCTION_ARGS) .ampredlocks = true, .amcanparallel = false, .amcanbuildparallel = false, + .amcanresetsnapshot = true, .amcaninclude = false, .amusemaintenanceworkmem = false, .amsummarizing = false, diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 6a09a1a151..c374ffa1ae 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -56,6 +56,9 @@ #include "utils/spccache.h" #include "utils/syscache.h" +/* GUCs */ +int concurrent_index_reset_snapshot_interval = 100; + static HeapTuple heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid, CommandId cid, uint32 options); @@ -701,6 +704,43 @@ heap_prepare_pagescan(TableScanDesc sscan) LockBuffer(buffer, BUFFER_LOCK_UNLOCK); } +/* + * heap_reset_scan_snapshot - periodically replace the scan snapshot + * + * The scan uses the active snapshot without registering another copy, + * allowing RotateSnapshot() to release it and advance xmin. + * + * The caller leaves it active for the rest of the build and pops it when the + * build ends. + */ +static inline void +heap_reset_scan_snapshot(HeapScanDesc scan) +{ + /* The scan and rotation state must refer to the active snapshot. */ + Assert(GetActiveSnapshot() == scan->rs_base.rs_snapshot); + Assert(scan->rs_rotation.snapshot == scan->rs_base.rs_snapshot); + + if (!SnapshotRotationDue(&scan->rs_rotation)) + return; + + INJECTION_POINT("heap_reset_scan_snapshot_swap", NULL); + RotateSnapshot(&scan->rs_rotation); + scan->rs_base.rs_snapshot = scan->rs_rotation.snapshot; + +#ifdef USE_INJECTION_POINTS + + /* + * An assigned xid or an older registered snapshot can hold back xmin + * despite the reset. This is safe, but defeats the purpose of resets. + * Tests attach here when they expect neither condition. + */ + if (TransactionIdIsValid(MyProc->xid)) + INJECTION_POINT("heap_reset_scan_snapshot_xid_assigned", NULL); + if (unlikely(MyProc->xmin != scan->rs_base.rs_snapshot->xmin)) + INJECTION_POINT("heap_reset_scan_snapshot_xmin_pinned", NULL); +#endif +} + /* * heap_fetch_next_buffer - read and pin the next block from MAIN_FORKNUM. * @@ -742,7 +782,11 @@ heap_fetch_next_buffer(HeapScanDesc scan, ScanDirection dir) scan->rs_cbuf = read_stream_next_buffer(scan->rs_read_stream, NULL); if (BufferIsValid(scan->rs_cbuf)) + { scan->rs_cblock = BufferGetBlockNumber(scan->rs_cbuf); + if (scan->rs_base.rs_flags & SO_RESET_SNAPSHOT) + heap_reset_scan_snapshot(scan); + } } /* @@ -1174,6 +1218,17 @@ heap_beginscan(Relation relation, Snapshot snapshot, { HeapScanDesc scan; + if (flags & SO_RESET_SNAPSHOT) + { + /* Not for parallel scans yet; see index_build_resets_snapshots(). */ + Assert(parallel_scan == NULL); + + /* The scan snapshot is required to be the active one. */ + Assert(GetActiveSnapshot() == snapshot); + Assert(!(flags & SO_TEMP_SNAPSHOT)); + INJECTION_POINT("heap_beginscan_reset_snapshot", NULL); + } + /* * increment relation ref count while scanning relation * @@ -1203,6 +1258,8 @@ heap_beginscan(Relation relation, Snapshot snapshot, scan->rs_base.rs_snapshot = snapshot; scan->rs_base.rs_nkeys = nkeys; scan->rs_base.rs_flags = flags; + if (flags & SO_RESET_SNAPSHOT) + StartSnapshotRotation(&scan->rs_rotation); scan->rs_base.rs_parallel = parallel_scan; scan->rs_base.rs_instrument = NULL; scan->rs_strategy = NULL; /* set in initscan */ @@ -1336,6 +1393,12 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params, { HeapScanDesc scan = (HeapScanDesc) sscan; + /* + * A rescan uses the current snapshot, which must still be active. + */ + if (scan->rs_base.rs_flags & SO_RESET_SNAPSHOT) + Assert(GetActiveSnapshot() == scan->rs_base.rs_snapshot); + if (set_params) { if (allow_strat) @@ -1425,6 +1488,15 @@ heap_endscan(TableScanDesc sscan) if (scan->rs_parallelworkerdata != NULL) pfree(scan->rs_parallelworkerdata); + if (scan->rs_base.rs_flags & SO_RESET_SNAPSHOT) + { + /* + * Leave the snapshot active for the rest of the build. The caller is + * responsible for popping it. + */ + Assert(!(scan->rs_base.rs_flags & SO_TEMP_SNAPSHOT)); + Assert(GetActiveSnapshot() == sscan->rs_snapshot); + } if (scan->rs_base.rs_flags & SO_TEMP_SNAPSHOT) UnregisterSnapshot(scan->rs_base.rs_snapshot); diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 7d2065dec3..7c406e3959 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -1190,11 +1190,10 @@ heapam_index_build_range_scan(Relation heapRelation, predicate = ExecPrepareQual(indexInfo->ii_Predicate, estate); /* - * Prepare for scan of the base relation. In a normal index build, we use - * SnapshotAny because we must retrieve all tuples and do our own time - * qual checks (because we have to index RECENTLY_DEAD tuples). In a - * concurrent build, or during bootstrap, we take a regular MVCC snapshot - * and index whatever's live according to that. + * Non-concurrent builds use SnapshotAny and check tuple visibility + * themselves, including RECENTLY_DEAD tuples. Concurrent builds and + * bootstrap use an MVCC snapshot. Eligible concurrent builds replace + * that snapshot periodically during the scan. */ OldestXmin = InvalidTransactionId; @@ -1206,24 +1205,43 @@ heapam_index_build_range_scan(Relation heapRelation, { /* * Serial index build. - * - * Must begin our own heap scan in this case. We may also need to - * register a snapshot whose lifetime is under our direct control. */ if (!TransactionIdIsValid(OldestXmin)) { - snapshot = RegisterSnapshot(GetTransactionSnapshot()); - need_unregister_snapshot = true; + /* + * Must begin our own heap scan in this case. We may also need + * to register a snapshot whose lifetime is under our direct + * control. + */ + if (indexInfo->ii_ResetSnapshot) + { + /* + * Use the build's active snapshot, which the scan can + * replace. index_build() ensures it is the only active-stack + * entry so no older entry holds back xmin. + */ + Assert(ActiveSnapshotSet()); + snapshot = GetActiveSnapshot(); + } + else + { + snapshot = RegisterSnapshot(GetTransactionSnapshot()); + need_unregister_snapshot = true; + } } else + { + Assert(!indexInfo->ii_Concurrent); snapshot = SnapshotAny; + } scan = table_beginscan_strat(heapRelation, /* relation */ snapshot, /* snapshot */ 0, /* number of keys */ NULL, /* scan key */ true, /* buffer access strategy OK */ - allow_sync); /* syncscan OK? */ + allow_sync, /* syncscan OK? */ + indexInfo->ii_ResetSnapshot); } else { @@ -1251,6 +1269,12 @@ heapam_index_build_range_scan(Relation heapRelation, Assert(snapshot == SnapshotAny ? TransactionIdIsValid(OldestXmin) : !TransactionIdIsValid(OldestXmin)); Assert(snapshot == SnapshotAny || !anyvisible); + Assert(snapshot == SnapshotAny || ActiveSnapshotSet() || + IsBootstrapProcessingMode()); + + /* The scan may replace and free this snapshot. */ + if (indexInfo->ii_ResetSnapshot) + snapshot = InvalidSnapshot; /* Publish number of blocks to scan */ if (progress) @@ -1758,7 +1782,8 @@ heapam_index_validate_scan(Relation heapRelation, 0, /* number of keys */ NULL, /* scan key */ true, /* buffer access strategy OK */ - false); /* syncscan not OK */ + false, /* syncscan not OK */ + false); /* reset_snapshot not OK */ hscan = (HeapScanDesc) scan; pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_TOTAL, diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c index 98ac5bc79c..1ed36e6e32 100644 --- a/src/backend/access/index/genam.c +++ b/src/backend/access/index/genam.c @@ -485,7 +485,7 @@ systable_beginscan(Relation heapRelation, */ sysscan->scan = table_beginscan_strat(heapRelation, snapshot, nkeys, key, - true, false); + true, false, false); sysscan->iscan = NULL; } diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c index 0abdd7b49f..ad9d166dce 100644 --- a/src/backend/access/nbtree/nbtree.c +++ b/src/backend/access/nbtree/nbtree.c @@ -138,6 +138,7 @@ bthandler(PG_FUNCTION_ARGS) .ampredlocks = true, .amcanparallel = true, .amcanbuildparallel = true, + .amcanresetsnapshot = true, .amcaninclude = true, .amusemaintenanceworkmem = false, .amsummarizing = false, diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c index f2ee333f60..59decf6560 100644 --- a/src/backend/access/spgist/spgutils.c +++ b/src/backend/access/spgist/spgutils.c @@ -64,6 +64,7 @@ spghandler(PG_FUNCTION_ARGS) .ampredlocks = false, .amcanparallel = false, .amcanbuildparallel = false, + .amcanresetsnapshot = true, .amcaninclude = true, .amusemaintenanceworkmem = false, .amsummarizing = false, diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 4d232b85ad..e87ddfa0ac 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -69,6 +69,7 @@ #include "storage/bufmgr.h" #include "storage/lmgr.h" #include "storage/predicate.h" +#include "storage/proc.h" #include "storage/smgr.h" #include "utils/builtins.h" #include "utils/fmgroids.h" @@ -1525,8 +1526,15 @@ index_concurrently_build(Oid heapRelationId, Relation indexRelation; IndexInfo *indexInfo; - /* This had better make sure that a snapshot is active */ - Assert(ActiveSnapshotSet()); + Assert(!ActiveSnapshotSet()); + Assert(!TransactionIdIsValid(MyProc->xid)); + + /* + * Keep a snapshot while loading pg_index expressions, which may be + * TOASTed, and evaluating them in BuildIndexInfo(). index_build() + * manages its own snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); /* Open and lock the parent heap relation */ heapRel = table_open(heapRelationId, ShareUpdateExclusiveLock); @@ -1550,6 +1558,8 @@ index_concurrently_build(Oid heapRelationId, * the catalog level. */ indexInfo = BuildIndexInfo(indexRelation); + /* Done with snapshot */ + PopActiveSnapshot(); Assert(!indexInfo->ii_ReadyForInserts); indexInfo->ii_Concurrent = true; indexInfo->ii_BrokenHotChain = false; @@ -1567,12 +1577,19 @@ index_concurrently_build(Oid heapRelationId, table_close(heapRel, NoLock); index_close(indexRelation, NoLock); + /* + * Updating pg_index might involve TOAST table access, so ensure we + * have a valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); /* * Update the pg_index row to mark the index as ready for inserts. Once we * commit this transaction, any new transactions that open the table must * insert new entries into the index for insertions and non-HOT updates. */ index_set_state_flags(indexRelationId, INDEX_CREATE_SET_READY); + /* we can do away with our snapshot */ + PopActiveSnapshot(); } /* @@ -3102,6 +3119,23 @@ index_update_stats(Relation rel, } +/* + * index_build_resets_snapshots -- check whether this build supports resets + * + * The index AM must opt in with amcanresetsnapshot. The scan decides whether + * to perform resets; see SO_RESET_SNAPSHOT. + * + * Only serial, non-unique builds support resets at this point. + */ +static bool +index_build_resets_snapshots(Relation indexRelation, IndexInfo *indexInfo) +{ + return indexInfo->ii_Concurrent && + !indexInfo->ii_Unique && + indexInfo->ii_ParallelWorkers == 0 && + indexRelation->rd_indam->amcanresetsnapshot; +} + /* * index_build - invoke access-method-specific index build procedure * @@ -3148,9 +3182,27 @@ index_build(Relation heapRelation, */ if (parallel && IsNormalProcessingMode() && indexRelation->rd_indam->amcanbuildparallel) + { + /* + * Concurrent builds arrive without an active snapshot. The planner + * needs one for expressions and catalog access, so provide it here + * and release it before the build starts. + */ + if (indexInfo->ii_Concurrent) + { + Assert(!ActiveSnapshotSet()); + PushActiveSnapshot(GetTransactionSnapshot()); + } indexInfo->ii_ParallelWorkers = plan_create_index_workers(RelationGetRelid(heapRelation), RelationGetRelid(indexRelation)); + if (indexInfo->ii_Concurrent) + PopActiveSnapshot(); + } + + /* Decide once whether this build uses periodic snapshot resets. */ + indexInfo->ii_ResetSnapshot = + index_build_resets_snapshots(indexRelation, indexInfo); if (indexInfo->ii_ParallelWorkers == 0) ereport(DEBUG1, @@ -3195,6 +3247,12 @@ index_build(Relation heapRelation, pgstat_progress_update_multi_param(6, progress_index, progress_vals); } + if (indexInfo->ii_Concurrent) + { + Assert(!ActiveSnapshotSet()); + PushActiveSnapshot(GetTransactionSnapshot()); + } + /* * Call the access method's build procedure */ @@ -3295,6 +3353,9 @@ index_build(Relation heapRelation, if (indexInfo->ii_ExclusionOps != NULL) IndexCheckExclusion(heapRelation, indexRelation, indexInfo); + if (indexInfo->ii_Concurrent) + PopActiveSnapshot(); + /* Roll back any GUC changes executed by index functions */ AtEOXact_GUC(false, save_nestlevel); @@ -3359,7 +3420,8 @@ IndexCheckExclusion(Relation heapRelation, 0, /* number of keys */ NULL, /* scan key */ true, /* buffer access strategy OK */ - true); /* syncscan OK */ + true, /* syncscan OK */ + false); /* reset_snapshot not OK */ while (table_scan_getnextslot(scan, ForwardScanDirection, slot)) { @@ -3419,14 +3481,20 @@ IndexCheckExclusion(Relation heapRelation, * incompatible HOT update done to it. We now build the index normally via * index_build(), while holding a weak lock that allows concurrent * insert/update/delete. Also, we index only tuples that are valid - * as of the start of the scan (see table_index_build_scan), whereas a normal + * as of the scan's snapshot (see table_index_build_scan), whereas a normal * build takes care to include recently-dead tuples. This is OK because * we won't mark the index valid until all transactions that might be able * to see those tuples are gone. The reason for doing that is to avoid * bogus unique-index failures due to concurrent UPDATEs (we might see * different versions of the same row as being valid when we pass over them, - * if we used HeapTupleSatisfiesVacuum). This leaves us with an index that - * does not contain any tuples added to the table while we built the index. + * if we used HeapTupleSatisfiesVacuum). The build can still miss tuples + * added while it is running. + * + * That snapshot is not held for the whole scan: a non-unique build replaces + * it with a fresh one every so often (SO_RESET_SNAPSHOT), so that the xmin + * horizon can advance while a large table is scanned. Each page is still + * read under one snapshot, and whatever a later snapshot shows that an + * earlier one did not is picked up by the validation described next. * * Next, we mark the index "indisready" (but still not "indisvalid") and * commit the second transaction and start a third. Again we wait for all diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 5a0312fe77..b5b0fe2747 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1730,23 +1730,17 @@ DefineIndex(ParseState *pstate, * chains can be created where the new tuple and the old tuple in the * chain have different index keys. * - * We now take a new snapshot, and build the index using all tuples that - * are visible in this snapshot. We can be sure that any HOT updates to + * We build the index using all tuples that are visible using single or + * multiple refreshing snapshots. We can be sure that any HOT updates to * these tuples will be compatible with the index, since any updates made * by transactions that didn't know about the index are now committed or * rolled back. Thus, each visible tuple is either the end of its * HOT-chain or the extension of the chain is HOT-safe for this index. */ - /* Set ActiveSnapshot since functions in the indexes may need it */ - PushActiveSnapshot(GetTransactionSnapshot()); - /* Perform concurrent build of index */ index_concurrently_build(tableId, indexRelationId); - /* we can do away with our snapshot */ - PopActiveSnapshot(); - /* * Commit this transaction to make the indisready update visible. */ @@ -4291,9 +4285,6 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein if (newidx->safe) set_indexsafe_procflags(); - /* Set ActiveSnapshot since functions in the indexes may need it */ - PushActiveSnapshot(GetTransactionSnapshot()); - /* * Update progress for the index to build, with the correct parent * table involved. @@ -4308,7 +4299,6 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein /* Perform concurrent build of new index */ index_concurrently_build(newidx->tableId, newidx->indexId); - PopActiveSnapshot(); CommitTransactionCommand(); } diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c index cdc02b274f..f772ace60d 100644 --- a/src/backend/nodes/makefuncs.c +++ b/src/backend/nodes/makefuncs.c @@ -848,6 +848,7 @@ makeIndexInfo(int numattrs, int numkeyattrs, Oid amoid, List *expressions, n->ii_CheckedUnchanged = false; n->ii_IndexUnchanged = false; n->ii_Concurrent = concurrent; + n->ii_ResetSnapshot = false; n->ii_Summarizing = summarizing; n->ii_WithoutOverlaps = withoutoverlaps; diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat index 380679a76c..b9d1d925dd 100644 --- a/src/backend/utils/misc/guc_parameters.dat +++ b/src/backend/utils/misc/guc_parameters.dat @@ -509,6 +509,16 @@ options => 'compute_query_id_options', }, +{ name => 'concurrent_index_reset_snapshot_interval', type => 'int', context => 'PGC_SUSET', group => 'DEVELOPER_OPTIONS', + short_desc => 'Sets the minimum time between snapshot resets during concurrent index builds.', + long_desc => 'The first heap scan resets its snapshot at the next page boundary after this interval. 0 resets on every page. -1 disables resets.', + flags => 'GUC_NOT_IN_SAMPLE | GUC_UNIT_MS', + variable => 'concurrent_index_reset_snapshot_interval', + boot_val => '100', + min => '-1', + max => 'INT_MAX', +}, + { name => 'config_file', type => 'string', context => 'PGC_POSTMASTER', group => 'FILE_LOCATIONS', short_desc => 'Sets the server\'s main configuration file.', flags => 'GUC_DISALLOW_IN_FILE | GUC_SUPERUSER_ONLY', diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 048450da2d..84d8a4e001 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -376,6 +376,90 @@ GetLatestSnapshot(void) return SecondarySnapshot; } +/* + * StartSnapshotRotation + * Initialize rotation state with the active snapshot and current time. + * + * The operation must not hold another reference to the snapshot, so + * RotateSnapshot() can release it. The caller remains responsible for + * popping the active snapshot when the operation ends. + */ +void +StartSnapshotRotation(SnapshotRotation *rotation) +{ + rotation->snapshot = GetActiveSnapshot(); + INSTR_TIME_SET_CURRENT_FAST(rotation->last_reset); +} + +/* + * SnapshotRotationDue + * Check whether the reset interval has elapsed, and restart the timer. + * + * Resets are disabled at REPEATABLE READ and above, where the transaction + * snapshot pins xmin regardless of the active snapshot. + */ +bool +SnapshotRotationDue(SnapshotRotation *rotation) +{ + instr_time now, + elapsed; + + if (concurrent_index_reset_snapshot_interval < 0 || + IsolationUsesXactSnapshot()) + return false; + + INSTR_TIME_SET_CURRENT_FAST(now); + elapsed = now; + INSTR_TIME_SUBTRACT(elapsed, rotation->last_reset); + if (INSTR_TIME_GET_MILLISEC(elapsed) < concurrent_index_reset_snapshot_interval) + return false; + + rotation->last_reset = now; + return true; +} + +/* + * RotateSnapshot + * Replace the active snapshot while keeping xmin valid throughout. + * + * Register the new snapshot before popping the old one. With the active + * stack empty, SnapshotResetXmin() can advance xmin to the oldest registered + * snapshot. An older snapshot held by another caller, such as a cursor, can + * still hold back xmin. + * + * Push the new snapshot and unregister it, leaving the active-stack entry as + * the operation's only reference for the next reset. + */ +void +RotateSnapshot(SnapshotRotation *rotation) +{ + Snapshot newsnap; + + /* + * The snapshot must be the only active-stack entry. A nested snapshot + * cannot be replaced here, and an older entry would keep xmin pinned. + */ + Assert(ActiveSnapshot != NULL && + ActiveSnapshot->as_snap == rotation->snapshot && + ActiveSnapshot->as_next == NULL); + Assert(!HistoricSnapshotActive()); + /* The operation is running, so the transaction snapshot was taken. */ + Assert(FirstSnapshotSet); + /* Not in parallel mode yet; see index_build_resets_snapshots(). */ + Assert(!IsInParallelMode()); + + newsnap = RegisterSnapshot(GetSnapshotData(&SecondarySnapshotData)); + + /* The catalog snapshot is registered too and would hold back xmin. */ + InvalidateCatalogSnapshot(); + + PopActiveSnapshot(); + PushActiveSnapshot(newsnap); + UnregisterSnapshot(newsnap); + + rotation->snapshot = GetActiveSnapshot(); +} + /* * GetCatalogSnapshot * Get a snapshot that is sufficiently up-to-date for scan of the diff --git a/src/bin/pg_amcheck/t/006_cic.pl b/src/bin/pg_amcheck/t/006_cic.pl index 1790ab0d7f..c19cedfd76 100644 --- a/src/bin/pg_amcheck/t/006_cic.pl +++ b/src/bin/pg_amcheck/t/006_cic.pl @@ -63,6 +63,9 @@ $node->append_conf('postgresql.conf', 'debug_parallel_query = off'); # Run autovacuum often enough to remove dead heap and TOAST tuples # while index builds are in progress. $node->append_conf('postgresql.conf', 'autovacuum_naptime = 1s'); +# Reset the snapshot of a concurrent build at every page boundary. +$node->append_conf('postgresql.conf', + 'concurrent_index_reset_snapshot_interval = 0'); $node->start; $node->safe_psql('postgres', q(CREATE EXTENSION amcheck)); $node->safe_psql( diff --git a/src/include/access/amapi.h b/src/include/access/amapi.h index 7924033353..ec0ca739a7 100644 --- a/src/include/access/amapi.h +++ b/src/include/access/amapi.h @@ -275,6 +275,12 @@ typedef struct IndexAmRoutine bool amcanparallel; /* does AM support parallel build? */ bool amcanbuildparallel; + /* + * Does the AM support snapshot resets during concurrent builds? Retained + * values must be copied, including external TOAST data. See indexam.sgml + * for the full requirements. + */ + bool amcanresetsnapshot; /* does AM support columns included with clause INCLUDE? */ bool amcaninclude; /* does AM use maintenance_work_mem? */ diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index 9e35961fb9..68fda612c8 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -69,6 +69,7 @@ typedef struct HeapScanDescData bool rs_inited; /* false = scan not init'd yet */ OffsetNumber rs_coffset; /* current offset # in non-page-at-a-time mode */ BlockNumber rs_cblock; /* current block # in scan, if any */ + SnapshotRotation rs_rotation; /* snapshot resets (SO_RESET_SNAPSHOT) */ Buffer rs_cbuf; /* current buffer in scan, if any */ /* NB: if rs_cbuf is not InvalidBuffer, we hold a pin on that buffer */ diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index ea3f2a6be9..4e9b7208be 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -73,6 +73,17 @@ typedef enum ScanOptions /* collect scan instrumentation */ SO_SCAN_INSTRUMENT = 1 << 11, + /* + * Allow periodic snapshot resets so xmin can advance during the scan. + * concurrent_index_reset_snapshot_interval controls the interval; each + * reset also invalidates the catalog snapshot. Scans skip resets when + * the transaction snapshot pins xmin. Table AMs that do not implement + * this flag keep the original snapshot. + * + * The scan uses the active snapshot without registering another copy and + * leaves its last snapshot active for the caller. + */ + SO_RESET_SNAPSHOT = 1 << 12, } ScanOptions; /* @@ -86,7 +97,7 @@ typedef enum ScanOptions (SO_TYPE_SEQSCAN | SO_TYPE_BITMAPSCAN | SO_TYPE_SAMPLESCAN | \ SO_TYPE_TIDSCAN | SO_TYPE_TIDRANGESCAN | SO_TYPE_ANALYZE | \ SO_ALLOW_STRAT | SO_ALLOW_SYNC | SO_ALLOW_PAGEMODE | \ - SO_TEMP_SNAPSHOT) + SO_TEMP_SNAPSHOT | SO_RESET_SNAPSHOT) /* * Result codes for table_{update,delete,lock_tuple}, and for visibility @@ -967,7 +978,8 @@ extern TableScanDesc table_beginscan_catalog(Relation relation, int nkeys, static inline TableScanDesc table_beginscan_strat(Relation rel, Snapshot snapshot, int nkeys, ScanKeyData *key, - bool allow_strat, bool allow_sync) + bool allow_strat, bool allow_sync, + bool reset_snapshot) { uint32 flags = SO_TYPE_SEQSCAN | SO_ALLOW_PAGEMODE; @@ -975,6 +987,8 @@ table_beginscan_strat(Relation rel, Snapshot snapshot, flags |= SO_ALLOW_STRAT; if (allow_sync) flags |= SO_ALLOW_SYNC; + if (reset_snapshot) + flags |= SO_RESET_SNAPSHOT; return table_beginscan_common(rel, snapshot, nkeys, key, NULL, flags, SO_NONE); @@ -1862,6 +1876,10 @@ table_scan_analyze_next_tuple(TableScanDesc scan, * very hard to detect whether they're really incompatible with the chain tip. * This only really makes sense for heap AM, it might need to be generalized * for other AMs later. + * + * With ii_ResetSnapshot, the scan can replace its snapshot at page + * boundaries according to concurrent_index_reset_snapshot_interval. + * The last snapshot remains active for the caller. */ static inline double table_index_build_scan(Relation table_rel, diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index 8d6aacc4d5..942f13ae5b 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -272,6 +272,7 @@ extern PGDLLIMPORT int work_mem; extern PGDLLIMPORT double hash_mem_multiplier; extern PGDLLIMPORT int maintenance_work_mem; extern PGDLLIMPORT int max_parallel_maintenance_workers; +extern PGDLLIMPORT int concurrent_index_reset_snapshot_interval; /* * Upper and lower hard limits for the buffer access strategy ring size diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index e3cea52e7d..bd8a99ada8 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -168,8 +168,9 @@ typedef struct ExprState * entries for a particular index. Used for both index_build and * retail creation of index entries. * - * ii_Concurrent, ii_BrokenHotChain, and ii_ParallelWorkers are used only - * during index build; they're conventionally zeroed otherwise. + * ii_Concurrent, ii_ResetSnapshot, ii_BrokenHotChain, and + * ii_ParallelWorkers are used only during index build; they're + * conventionally zeroed otherwise. * ---------------- */ typedef struct IndexInfo @@ -221,6 +222,8 @@ typedef struct IndexInfo bool ii_IndexUnchanged; /* are we doing a concurrent index build? */ bool ii_Concurrent; + /* does this concurrent build use periodic snapshot resets? */ + bool ii_ResetSnapshot; /* did we detect any broken HOT chains? */ bool ii_BrokenHotChain; /* is it a summarizing index? */ diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 1c55009639..a96e7e6736 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -76,6 +76,10 @@ extern PGDLLIMPORT SnapshotData SnapshotToastData; extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); + +extern void StartSnapshotRotation(SnapshotRotation *rotation); +extern bool SnapshotRotationDue(SnapshotRotation *rotation); +extern void RotateSnapshot(SnapshotRotation *rotation); extern void SnapshotSetCommandId(CommandId curcid); extern Snapshot GetCatalogSnapshot(Oid relid); diff --git a/src/include/utils/snapshot.h b/src/include/utils/snapshot.h index 9766aabcad..9a4538d2aa 100644 --- a/src/include/utils/snapshot.h +++ b/src/include/utils/snapshot.h @@ -14,6 +14,7 @@ #define SNAPSHOT_H #include "lib/pairingheap.h" +#include "portability/instr_time.h" /* @@ -209,4 +210,15 @@ typedef struct SnapshotData uint64 snapXactCompletionCount; } SnapshotData; + +/* + * State for periodically replacing an operation's active snapshot. See + * StartSnapshotRotation(). + */ +typedef struct SnapshotRotation +{ + Snapshot snapshot; /* current active snapshot */ + instr_time last_reset; /* when it was last replaced */ +} SnapshotRotation; + #endif /* SNAPSHOT_H */ diff --git a/src/test/modules/dummy_index_am/dummy_index_am.c b/src/test/modules/dummy_index_am/dummy_index_am.c index 31f8d2b816..7827968fd2 100644 --- a/src/test/modules/dummy_index_am/dummy_index_am.c +++ b/src/test/modules/dummy_index_am/dummy_index_am.c @@ -313,6 +313,7 @@ dihandler(PG_FUNCTION_ARGS) .ampredlocks = false, .amcanparallel = false, .amcanbuildparallel = false, + .amcanresetsnapshot = false, .amcaninclude = false, .amusemaintenanceworkmem = false, .amsummarizing = false, diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index 775ad37860..362d891be2 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -11,7 +11,7 @@ PGFILEDESC = "injection_points - facility for injection points" EXTRA_INSTALL = contrib/test_decoding -REGRESS = injection_points hashagg reindex_conc vacuum +REGRESS = injection_points hashagg reindex_conc vacuum cic_reset_snapshots REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress # wait_cleanup terminates another backend, whose FATAL message can be lost diff --git a/src/test/modules/injection_points/expected/cic_reset_snapshots.out b/src/test/modules/injection_points/expected/cic_reset_snapshots.out new file mode 100644 index 0000000000..bf25cafb61 --- /dev/null +++ b/src/test/modules/injection_points/expected/cic_reset_snapshots.out @@ -0,0 +1,213 @@ +CREATE EXTENSION injection_points; +SELECT injection_points_set_local(); + injection_points_set_local +---------------------------- + +(1 row) + +-- Reset on every page boundary to make resets deterministic. +SET concurrent_index_reset_snapshot_interval = 0; +SELECT injection_points_attach('heap_reset_scan_snapshot_xmin_pinned', 'error'); + injection_points_attach +------------------------- + +(1 row) + +SELECT injection_points_attach('heap_reset_scan_snapshot_xid_assigned', 'notice'); + injection_points_attach +------------------------- + +(1 row) + +SELECT injection_points_attach('heap_beginscan_reset_snapshot', 'notice'); + injection_points_attach +------------------------- + +(1 row) + +CREATE SCHEMA cic_reset_snap; +-- Pad rows to fit exactly seven per page at any block size. A scan of +-- 35 rows then resets its snapshot five times with a zero interval. +-- Account for the page header (24), line pointer (4), and tuple header +-- plus two integers (36), and round the tuple size down to a multiple of 8. +SELECT ((current_setting('block_size')::int - 24) / 7 - 4) / 8 * 8 - 36 AS pad \gset +CREATE TABLE cic_reset_snap.tbl(i int primary key, j int, pad char(:pad)); +ALTER TABLE cic_reset_snap.tbl ALTER COLUMN pad SET STORAGE PLAIN; +INSERT INTO cic_reset_snap.tbl SELECT i, i * I, 'x' FROM generate_series(1, 35) s(i); +-- Count the snapshot resets during one scan. +SELECT injection_points_attach('heap_reset_scan_snapshot_swap', 'notice'); + injection_points_attach +------------------------- + +(1 row) + +CREATE INDEX CONCURRENTLY rotation_probe ON cic_reset_snap.tbl(i); +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_swap +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_swap +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_swap +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_swap +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_swap +SELECT injection_points_detach('heap_reset_scan_snapshot_swap'); + injection_points_detach +------------------------- + +(1 row) + +DROP INDEX CONCURRENTLY cic_reset_snap.rotation_probe; +CREATE FUNCTION cic_reset_snap.predicate_stable(integer) RETURNS bool IMMUTABLE + LANGUAGE plpgsql AS $$ +BEGIN + EXECUTE 'SELECT txid_current()'; + RETURN MOD($1, 2) = 0; +END; $$; +CREATE FUNCTION cic_reset_snap.predicate_stable_no_param() RETURNS bool IMMUTABLE + LANGUAGE plpgsql AS $$ +BEGIN + EXECUTE 'SELECT txid_current()'; + RETURN false; +END; $$; +---------------- +ALTER TABLE cic_reset_snap.tbl SET (parallel_workers=0); +CREATE UNIQUE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i); +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(MOD(i, 2), j) WHERE MOD(i, 2) = 0; +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i, j) WHERE cic_reset_snap.predicate_stable(i); +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i, j) WHERE cic_reset_snap.predicate_stable_no_param(); +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl USING BRIN(i); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl USING HASH(i); +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl USING GIST(point(i, j)); +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl USING SPGIST(point(i, j)); +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl USING GIN((ARRAY[i, j])); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +-- The same in parallel mode +ALTER TABLE cic_reset_snap.tbl SET (parallel_workers=2); +CREATE UNIQUE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(MOD(i, 2), j) WHERE MOD(i, 2) = 0; +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i, j) WHERE cic_reset_snap.predicate_stable(i); +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +NOTICE: notice triggered for injection point heap_reset_scan_snapshot_xid_assigned +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i, j) WHERE cic_reset_snap.predicate_stable_no_param(); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i DESC NULLS LAST); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl USING BRIN(i); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl USING GIN((ARRAY[i, j])); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +BEGIN TRANSACTION; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i); +ERROR: CREATE INDEX CONCURRENTLY cannot run inside a transaction block +ROLLBACK ; +-- REPEATABLE READ and above retain the transaction snapshot, so the +-- scan must not replace its snapshot. +ALTER TABLE cic_reset_snap.tbl SET (parallel_workers=0); +SELECT injection_points_attach('heap_reset_scan_snapshot_swap', 'notice'); + injection_points_attach +------------------------- + +(1 row) + +SET default_transaction_isolation = 'repeatable read'; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i); +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +SET default_transaction_isolation = serializable; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i); +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +NOTICE: notice triggered for injection point heap_beginscan_reset_snapshot +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +RESET default_transaction_isolation; +SELECT injection_points_detach('heap_reset_scan_snapshot_swap'); + injection_points_detach +------------------------- + +(1 row) + +DROP SCHEMA cic_reset_snap CASCADE; +NOTICE: drop cascades to 3 other objects +DETAIL: drop cascades to table cic_reset_snap.tbl +drop cascades to function cic_reset_snap.predicate_stable(integer) +drop cascades to function cic_reset_snap.predicate_stable_no_param() +SELECT injection_points_detach('heap_beginscan_reset_snapshot'); + injection_points_detach +------------------------- + +(1 row) + +DROP EXTENSION injection_points; diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index 60148f96fd..0800f4e444 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -63,6 +63,7 @@ tests += { 'hashagg', 'reindex_conc', 'vacuum', + 'cic_reset_snapshots', ], 'regress_args': ['--dlpath', meson.project_build_root() / 'src/test/regress'], # The injection points are cluster-wide, so disable installcheck diff --git a/src/test/modules/injection_points/sql/cic_reset_snapshots.sql b/src/test/modules/injection_points/sql/cic_reset_snapshots.sql new file mode 100644 index 0000000000..35a9e29082 --- /dev/null +++ b/src/test/modules/injection_points/sql/cic_reset_snapshots.sql @@ -0,0 +1,143 @@ +CREATE EXTENSION injection_points; + +SELECT injection_points_set_local(); +-- Reset on every page boundary to make resets deterministic. +SET concurrent_index_reset_snapshot_interval = 0; +SELECT injection_points_attach('heap_reset_scan_snapshot_xmin_pinned', 'error'); +SELECT injection_points_attach('heap_reset_scan_snapshot_xid_assigned', 'notice'); +SELECT injection_points_attach('heap_beginscan_reset_snapshot', 'notice'); + + +CREATE SCHEMA cic_reset_snap; +-- Pad rows to fit exactly seven per page at any block size. A scan of +-- 35 rows then resets its snapshot five times with a zero interval. +-- Account for the page header (24), line pointer (4), and tuple header +-- plus two integers (36), and round the tuple size down to a multiple of 8. +SELECT ((current_setting('block_size')::int - 24) / 7 - 4) / 8 * 8 - 36 AS pad \gset +CREATE TABLE cic_reset_snap.tbl(i int primary key, j int, pad char(:pad)); +ALTER TABLE cic_reset_snap.tbl ALTER COLUMN pad SET STORAGE PLAIN; +INSERT INTO cic_reset_snap.tbl SELECT i, i * I, 'x' FROM generate_series(1, 35) s(i); + +-- Count the snapshot resets during one scan. +SELECT injection_points_attach('heap_reset_scan_snapshot_swap', 'notice'); +CREATE INDEX CONCURRENTLY rotation_probe ON cic_reset_snap.tbl(i); +SELECT injection_points_detach('heap_reset_scan_snapshot_swap'); +DROP INDEX CONCURRENTLY cic_reset_snap.rotation_probe; + +CREATE FUNCTION cic_reset_snap.predicate_stable(integer) RETURNS bool IMMUTABLE + LANGUAGE plpgsql AS $$ +BEGIN + EXECUTE 'SELECT txid_current()'; + RETURN MOD($1, 2) = 0; +END; $$; + +CREATE FUNCTION cic_reset_snap.predicate_stable_no_param() RETURNS bool IMMUTABLE + LANGUAGE plpgsql AS $$ +BEGIN + EXECUTE 'SELECT txid_current()'; + RETURN false; +END; $$; + +---------------- +ALTER TABLE cic_reset_snap.tbl SET (parallel_workers=0); + +CREATE UNIQUE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(MOD(i, 2), j) WHERE MOD(i, 2) = 0; +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i, j) WHERE cic_reset_snap.predicate_stable(i); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i, j) WHERE cic_reset_snap.predicate_stable_no_param(); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl USING BRIN(i); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl USING HASH(i); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl USING GIST(point(i, j)); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl USING SPGIST(point(i, j)); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl USING GIN((ARRAY[i, j])); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +-- The same in parallel mode +ALTER TABLE cic_reset_snap.tbl SET (parallel_workers=2); + +CREATE UNIQUE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(MOD(i, 2), j) WHERE MOD(i, 2) = 0; +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i, j) WHERE cic_reset_snap.predicate_stable(i); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i, j) WHERE cic_reset_snap.predicate_stable_no_param(); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i DESC NULLS LAST); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl USING BRIN(i); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl USING GIN((ARRAY[i, j])); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +BEGIN TRANSACTION; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i); +ROLLBACK ; + +-- REPEATABLE READ and above retain the transaction snapshot, so the +-- scan must not replace its snapshot. +ALTER TABLE cic_reset_snap.tbl SET (parallel_workers=0); +SELECT injection_points_attach('heap_reset_scan_snapshot_swap', 'notice'); +SET default_transaction_isolation = 'repeatable read'; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; + +SET default_transaction_isolation = serializable; +CREATE INDEX CONCURRENTLY idx ON cic_reset_snap.tbl(i); +REINDEX INDEX CONCURRENTLY cic_reset_snap.idx; +DROP INDEX CONCURRENTLY cic_reset_snap.idx; +RESET default_transaction_isolation; +SELECT injection_points_detach('heap_reset_scan_snapshot_swap'); + +DROP SCHEMA cic_reset_snap CASCADE; + +SELECT injection_points_detach('heap_beginscan_reset_snapshot'); + +DROP EXTENSION injection_points; -- 2.43.0