From abf3f05426143edf5e2953acbe07f5db30d34050 Mon Sep 17 00:00:00 2001
From: Mikhail Nikalayeu <mihailnikalayeu@gmail.com>
Date: Mon, 21 Apr 2025 14:11:53 +0200
Subject: [PATCH v38 7/7] Refresh snapshot periodically during index validation

Enhances validation phase of concurrently built indexes by periodically refreshing snapshots rather than using a single reference snapshot. This addresses issues with xmin propagation during long-running validations.

The validation now takes a fresh snapshot at page boundaries, paced by time (debug_cic_validate_snapshot_interval), allowing the xmin horizon to advance. The snapshot switch is ordered so that the backend's xmin is never invalid and only moves forward, and stays robust if something else (say, a cursor opened by an index expression) keeps a snapshot registered. Injection points let tests verify both that resets happen and that they fully advance xmin.

This restores feature of commit d9d076222f5b, which was reverted in commit e28bb8851969. New STIR-based approach does not depend on single reference snapshot anymore.
---
 src/backend/access/heap/README.HOT            |   4 +-
 src/backend/access/heap/heapam_handler.c      | 120 ++++++++++++++-
 src/backend/access/spgist/spgvacuum.c         |  12 +-
 src/backend/catalog/index.c                   |  73 ++++++---
 src/backend/commands/indexcmds.c              |  52 +------
 src/backend/utils/misc/guc_parameters.dat     |   9 ++
 src/include/access/tableam.h                  |  25 ++--
 src/include/access/transam.h                  |  15 ++
 src/include/catalog/index.h                   |   2 +-
 src/include/miscadmin.h                       |   1 +
 src/test/modules/test_misc/meson.build        |   1 +
 .../test_misc/t/015_cic_validate_snapshot.pl  | 140 ++++++++++++++++++
 src/test/regress/expected/create_index.out    |   3 +
 src/test/regress/sql/create_index.sql         |   4 +
 14 files changed, 378 insertions(+), 83 deletions(-)
 create mode 100644 src/test/modules/test_misc/t/015_cic_validate_snapshot.pl

diff --git a/src/backend/access/heap/README.HOT b/src/backend/access/heap/README.HOT
index b1c797517ee..382fe1723a5 100644
--- a/src/backend/access/heap/README.HOT
+++ b/src/backend/access/heap/README.HOT
@@ -401,12 +401,12 @@ live tuple.
 We mark the index open for inserts (but still not ready for reads) then
 we again wait for transactions which have the table open.  Then validate
 the index.  This searches for tuples missing from the index in auxiliary
-index, and inserts any missing ones if they are visible to reference snapshot.
+index, and inserts any missing ones if they are visible to a fresh snapshot.
 Again, the index entries have to have TIDs equal to HOT-chain root TIDs, but
 the value to be inserted is the one from the live tuple.
 
 Then we wait until every transaction that could have a snapshot older than
-the second reference snapshot is finished.  This ensures that nobody is
+the latest used snapshot is finished.  This ensures that nobody is
 alive any longer who could need to see any tuples that might be missing
 from the index, as well as ensuring that no one can see any inconsistent
 rows in a broken HOT chain (the first condition is stronger than the
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 22e010aee1a..c7acfbd6d91 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -46,6 +46,7 @@
 #include "storage/procarray.h"
 #include "storage/smgr.h"
 #include "utils/builtins.h"
+#include "utils/injection_point.h"
 #include "utils/rel.h"
 #include "utils/tuplesort.h"
 #include "utils/tuplestore.h"
@@ -53,6 +54,12 @@
 /* GUC: percentage of maintenance_work_mem for CIC validation tuplestore */
 int			debug_cic_validate_store_mem_pct = 10;
 
+/*
+ * GUC: minimum time between snapshot refreshes during CIC validation,
+ * in milliseconds (0 = disable refreshes).
+ */
+int			debug_cic_validate_snapshot_interval = 100;
+
 static void reform_and_rewrite_tuple(HeapTuple tuple,
 									 Relation OldHeap, Relation NewHeap,
 									 Datum *values, bool *isnull, RewriteState rwstate);
@@ -1965,24 +1972,72 @@ heapam_index_validate_scan_read_stream_next(
 	return result;
 }
 
-static void
+/*
+ * validate_index_reset_snapshot - replace the validation scan's snapshot
+ * with a freshly taken one, advancing this backend's xmin.
+ *
+ * The ordering is chosen so that MyProc->xmin is never invalid and only
+ * moves forward: the new snapshot is registered before the old one is
+ * released, and the old snapshot is unregistered while the active stack is
+ * empty -- at that point SnapshotResetXmin() advances xmin directly to the
+ * oldest remaining registered snapshot, normally the new one.
+ *
+ * If something else keeps an older snapshot registered (say, a cursor
+ * opened by an index expression), xmin cannot advance all the way.  That is
+ * not a correctness problem -- the horizon merely lags behind -- but it
+ * defeats the purpose of resetting, so tests may attach to the
+ * "validate-index-xmin-not-advanced" injection point to catch such cases.
+ */
+static Snapshot
+validate_index_reset_snapshot(Snapshot snapshot)
+{
+	Snapshot	newsnap;
+
+	/* The catalog snapshot is registered too and would hold back xmin. */
+	InvalidateCatalogSnapshot();
+
+	newsnap = RegisterSnapshot(GetLatestSnapshot());
+	PopActiveSnapshot();
+	UnregisterSnapshot(snapshot);
+	PushActiveSnapshot(newsnap);
+
+	INJECTION_POINT("validate-index-snapshot-reset", NULL);
+	if (unlikely(MyProc->xmin != newsnap->xmin))
+		INJECTION_POINT("validate-index-xmin-not-advanced", NULL);
+
+	return newsnap;
+}
+
+static TransactionId
 heapam_index_validate_scan(Relation heapRelation,
 						   Relation indexRelation,
 						   IndexInfo *indexInfo,
-						   Snapshot snapshot,
 						   ValidateIndexState *state,
 						   ValidateIndexState *auxState)
 {
+	TransactionId limitXmin;
+
 	Datum		values[INDEX_MAX_KEYS];
 	bool		isnull[INDEX_MAX_KEYS];
 
+	Snapshot		snapshot;
 	TupleTableSlot  *slot;
 	EState			*estate;
 	ExprContext		*econtext;
 	BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD);
 
 	int64			num_to_check;
+	TimestampTz		last_reset_time;
 	Tuplestorestate *tuples_for_check;
+
+	/*
+	 * Under REPEATABLE READ or SERIALIZABLE (possible via
+	 * default_transaction_isolation), the transaction snapshot remains
+	 * registered until the transaction ends and pins xmin no matter what we
+	 * do here, so periodic snapshot refresh cannot achieve anything; skip
+	 * it.
+	 */
+	bool		reset_snapshot = XactIsoLevel <= XACT_READ_COMMITTED;
 	ValidateIndexScanState callback_private_data;
 
 	Buffer buf;
@@ -1992,6 +2047,8 @@ heapam_index_validate_scan(Relation heapRelation,
 	/* Use a percentage of maintenance_work_mem for tuple store. */
 	int		store_work_mem_part = maintenance_work_mem * debug_cic_validate_store_mem_pct / 100;
 
+	PushActiveSnapshot(GetTransactionSnapshot());
+
 	/*
 	 * Encode TIDs as int8 values for the sort, rather than directly sorting
 	 * item pointers.  This can be significantly faster, primarily because TID
@@ -2000,6 +2057,12 @@ heapam_index_validate_scan(Relation heapRelation,
 	 */
 	tuples_for_check = tuplestore_begin_datum(INT8OID, false, false, store_work_mem_part);
 
+	PopActiveSnapshot();
+	InvalidateCatalogSnapshot();
+
+	Assert(!reset_snapshot || !HaveRegisteredOrActiveSnapshot());
+	Assert(!reset_snapshot || !TransactionIdIsValid(MyProc->xmin));
+
 	/*
 	 * sanity checks
 	 */
@@ -2015,6 +2078,30 @@ heapam_index_validate_scan(Relation heapRelation,
 
 	state->tuplesort = auxState->tuplesort = NULL;
 
+	/*
+	 * Now take the first snapshot that will be used to filter candidate
+	 * tuples. We are going to replace it by newer snapshot every so often
+	 * to propagate horizon.
+	 *
+	 * Beware!  There might still be snapshots in use that treat some transaction
+	 * as in-progress that our temporary snapshot treats as committed.
+	 *
+	 * If such a recently-committed transaction deleted tuples in the table,
+	 * we will not include them in the index; yet those transactions which
+	 * see the deleting one as still-in-progress will expect such tuples to
+	 * be there once we mark the index as valid.
+	 *
+	 * We solve this by waiting for all endangered transactions to exit before
+	 * we mark the index as valid, for that reason limitXmin is supported.
+	 *
+	 * We also set ActiveSnapshot to this snap, since functions in indexes may
+	 * need a snapshot.
+	 */
+	snapshot = RegisterSnapshot(GetLatestSnapshot());
+	PushActiveSnapshot(snapshot);
+	limitXmin = snapshot->xmin;
+	last_reset_time = GetCurrentTimestamp();
+
 	estate = CreateExecutorState();
 	econtext = GetPerTupleExprContext(estate);
 	slot = MakeSingleTupleTableSlot(RelationGetDescr(heapRelation),
@@ -2118,6 +2205,24 @@ heapam_index_validate_scan(Relation heapRelation,
 		}
 
 		ReleaseBuffer(buf);
+
+		/*
+		 * Periodically switch to a fresh snapshot to let the xmin horizon
+		 * advance during long validations.  The check is made at page
+		 * boundaries, but time is the pacing criterion: the amount of work
+		 * per page varies too much for a page count to be a useful proxy.
+		 */
+		if (reset_snapshot &&
+			debug_cic_validate_snapshot_interval > 0 &&
+			TimestampDifferenceExceeds(last_reset_time, GetCurrentTimestamp(),
+									   debug_cic_validate_snapshot_interval))
+		{
+			snapshot = validate_index_reset_snapshot(snapshot);
+			last_reset_time = GetCurrentTimestamp();
+
+			/* Advance limitXmin so we wait for all snapshots seen so far */
+			limitXmin = TransactionIdNewer(limitXmin, snapshot->xmin);
+		}
 	}
 
 	ExecDropSingleTupleTableSlot(slot);
@@ -2127,11 +2232,22 @@ heapam_index_validate_scan(Relation heapRelation,
 	read_stream_end(read_stream);
 	tuplestore_end(tuples_for_check);
 
+	/*
+	 * Drop the latest snapshot.  We must do this before waiting out other
+	 * snapshot holders, else we will deadlock against other processes also
+	 * doing CREATE INDEX CONCURRENTLY, which would see our snapshot as one
+	 * they must wait for.
+	 */
+	PopActiveSnapshot();
+	UnregisterSnapshot(snapshot);
+	InvalidateCatalogSnapshot();
 	FreeAccessStrategy(bstrategy);
 
 	/* These may have been pointing to the now-gone estate */
 	indexInfo->ii_ExpressionsState = NIL;
 	indexInfo->ii_PredicateState = NULL;
+
+	return limitXmin;
 }
 
 /*
diff --git a/src/backend/access/spgist/spgvacuum.c b/src/backend/access/spgist/spgvacuum.c
index c461f8dc02d..ef192fb99c2 100644
--- a/src/backend/access/spgist/spgvacuum.c
+++ b/src/backend/access/spgist/spgvacuum.c
@@ -191,14 +191,16 @@ vacuumLeafPage(spgBulkDeleteState *bds, Relation index, Buffer buffer,
 			 * Add target TID to pending list if the redirection could have
 			 * happened since VACUUM started.  (If xid is invalid, assume it
 			 * must have happened before VACUUM started, since REINDEX
-			 * CONCURRENTLY locks out VACUUM.)
+			 * CONCURRENTLY locks out VACUUM, if myXmin is invalid it is
+			 * validation scan.)
 			 *
 			 * Note: we could make a tighter test by seeing if the xid is
 			 * "running" according to the active snapshot; but snapmgr.c
 			 * doesn't currently export a suitable API, and it's not entirely
 			 * clear that a tighter test is worth the cycles anyway.
 			 */
-			if (TransactionIdFollowsOrEquals(dt->xid, bds->myXmin))
+			if (!TransactionIdIsValid(bds->myXmin) ||
+					TransactionIdFollowsOrEquals(dt->xid, bds->myXmin))
 				spgAddPendingTID(bds, &dt->pointer);
 		}
 		else
@@ -808,7 +810,6 @@ spgvacuumscan(spgBulkDeleteState *bds)
 	/* Finish setting up spgBulkDeleteState */
 	initSpGistState(&bds->spgstate, index);
 	bds->pendingList = NULL;
-	bds->myXmin = GetActiveSnapshot()->xmin;
 	bds->lastFilledBlock = SPGIST_LAST_FIXED_BLKNO;
 
 	/*
@@ -959,6 +960,10 @@ spgbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	bds.stats = stats;
 	bds.callback = callback;
 	bds.callback_state = callback_state;
+	if (info->validate_index)
+		bds.myXmin = InvalidTransactionId;
+	else
+		bds.myXmin = GetActiveSnapshot()->xmin;
 
 	spgvacuumscan(&bds);
 
@@ -999,6 +1004,7 @@ spgvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
 		bds.stats = stats;
 		bds.callback = dummy_callback;
 		bds.callback_state = NULL;
+		bds.myXmin = GetActiveSnapshot()->xmin;
 
 		spgvacuumscan(&bds);
 	}
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index cea1eb0207b..89e6c550048 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"
@@ -3562,8 +3563,9 @@ IndexCheckExclusion(Relation heapRelation,
  * insert their new tuples into it. At the same moment we clear "indisready" for
  * auxiliary index, since it is no more required to be updated.
  *
- * We then take a new reference snapshot, any tuples that are valid according
- * to this snap, but are not in the index, must be added to the index.
+ * We then take a new snapshot, any tuples that are valid according
+ * to this snap, but are not in the index, must be added to the index. In
+ * order to propagate xmin we reset that snapshot every so often.
  * (Any tuples committed live after the snap will be inserted into the
  * index by their originating transaction.  Any tuples committed dead before
  * the snap need not be indexed, because we will wait out all transactions
@@ -3576,7 +3578,7 @@ IndexCheckExclusion(Relation heapRelation,
  * TIDs of both auxiliary and target indexes, and doing a "merge join" against
  * the TID lists to see which tuples from auxiliary index are missing from the
  * target index.  Thus we will ensure that all tuples valid according to the
- * reference snapshot are in the index. Notice we need to do bulkdelete in the
+ * latest snapshot are in the index. Notice we need to do bulkdelete in the
  * particular order: auxiliary first, target last.
  *
  * Building a unique index this way is tricky: we might try to insert a
@@ -3589,21 +3591,24 @@ IndexCheckExclusion(Relation heapRelation,
  * before it declares a uniqueness error.
  *
  * After completing validate_index(), we wait until all transactions that
- * were alive at the time of the reference snapshot are gone; this is
- * necessary to be sure there are none left with a transaction snapshot
- * older than the reference (and hence possibly able to see tuples we did
- * not index).  Then we mark the index "indisvalid" and commit.  Subsequent
- * transactions will be able to use it for queries.
+ * were alive at the time of the latest snapshot used during validation are
+ * gone; this is necessary to be sure there are none left with a transaction
+ * snapshot older than that (and hence possibly able to see tuples we did
+ * not index).  The snapshot is periodically refreshed during the heap scan
+ * to propagate the xmin horizon, so limitXmin tracks the most recent one.
+ * Then we mark the index "indisvalid" and commit.  Subsequent transactions
+ * will be able to use it for queries.
  *
  * Also, some actions to concurrent drop the auxiliary index are performed.
  */
-void
-validate_index(Oid heapId, Oid indexId, Oid auxIndexId, Snapshot snapshot)
+TransactionId
+validate_index(Oid heapId, Oid indexId, Oid auxIndexId)
 {
 	Relation	heapRelation,
 				indexRelation,
 				auxIndexRelation;
 	IndexInfo  *indexInfo;
+	TransactionId limitXmin;
 	IndexVacuumInfo ivinfo, auxivinfo;
 	ValidateIndexState state, auxState;
 	Oid			save_userid;
@@ -3616,6 +3621,17 @@ validate_index(Oid heapId, Oid indexId, Oid auxIndexId, Snapshot snapshot)
 	int			main_work_mem_part = (int)((int64) maintenance_work_mem * 8 / 10);
 	int			aux_work_mem_part = maintenance_work_mem / 10;
 
+	/*
+	 * Under REPEATABLE READ or SERIALIZABLE (possible via
+	 * default_transaction_isolation), the transaction snapshot remains
+	 * registered until the transaction ends and pins xmin no matter what we
+	 * do here, so periodic snapshot refresh cannot achieve anything; skip
+	 * it.
+	 */
+#ifdef USE_ASSERT_CHECKING
+	bool		reset_snapshot = XactIsoLevel <= XACT_READ_COMMITTED;
+#endif
+
 	{
 		const int	progress_index[] = {
 			PROGRESS_CREATEIDX_PHASE,
@@ -3653,8 +3669,12 @@ validate_index(Oid heapId, Oid indexId, Oid auxIndexId, Snapshot snapshot)
 	 * Fetch info needed for index_insert.  (You might think this should be
 	 * passed in from DefineIndex, but its copy is long gone due to having
 	 * been built in a previous transaction.)
+	 *
+	 * We might need snapshot for index expressions or predicates.
 	 */
+	PushActiveSnapshot(GetTransactionSnapshot());
 	indexInfo = BuildIndexInfo(indexRelation);
+	PopActiveSnapshot();
 
 	/* mark build is concurrent just for consistency */
 	indexInfo->ii_Concurrent = true;
@@ -3690,6 +3710,9 @@ validate_index(Oid heapId, Oid indexId, Oid auxIndexId, Snapshot snapshot)
 										   NULL, TUPLESORT_NONE);
 	auxState.htups = auxState.itups = auxState.tups_inserted = 0;
 
+	/* tuplesort_begin_datum may require catalog snapshot */
+	InvalidateCatalogSnapshot();
+
 	(void) index_bulk_delete(&auxivinfo, NULL,
 							 validate_index_callback, &auxState);
 	/* If aux index is empty, merge may be skipped */
@@ -3709,7 +3732,13 @@ validate_index(Oid heapId, Oid indexId, Oid auxIndexId, Snapshot snapshot)
 		index_close(indexRelation, NoLock);
 		table_close(heapRelation, NoLock);
 
-		return;
+		PushActiveSnapshot(GetTransactionSnapshot());
+		limitXmin = GetActiveSnapshot()->xmin;
+		PopActiveSnapshot();
+		InvalidateCatalogSnapshot();
+
+		Assert(!reset_snapshot || !TransactionIdIsValid(MyProc->xmin));
+		return limitXmin;
 	}
 
 	state.tuplesort = tuplesort_begin_datum(INT8OID, Int8LessOperator,
@@ -3718,6 +3747,9 @@ validate_index(Oid heapId, Oid indexId, Oid auxIndexId, Snapshot snapshot)
 											NULL, TUPLESORT_NONE);
 	state.htups = state.itups = state.tups_inserted = 0;
 
+	/* tuplesort_begin_datum may require catalog snapshot */
+	InvalidateCatalogSnapshot();
+
 	/* ambulkdelete updates progress metrics */
 	(void) index_bulk_delete(&ivinfo, NULL,
 							 validate_index_callback, &state);
@@ -3737,19 +3769,24 @@ validate_index(Oid heapId, Oid indexId, Oid auxIndexId, Snapshot snapshot)
 		pgstat_progress_update_multi_param(3, progress_index, progress_vals);
 	}
 	tuplesort_performsort(state.tuplesort);
+	/* tuplesort_performsort may require catalog snapshot */
+	InvalidateCatalogSnapshot();
+
 	tuplesort_performsort(auxState.tuplesort);
+	/* tuplesort_performsort may require catalog snapshot */
+	InvalidateCatalogSnapshot();
+	Assert(!reset_snapshot || !TransactionIdIsValid(MyProc->xmin));
 
 	/*
 	 * Now merge both indexes
 	 */
 	pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
 								 PROGRESS_CREATEIDX_PHASE_VALIDATE_IDXMERGE);
-	table_index_validate_scan(heapRelation,
-							  indexRelation,
-							  indexInfo,
-							  snapshot,
-							  &state,
-							  &auxState);
+	limitXmin = table_index_validate_scan(heapRelation,
+										  indexRelation,
+										  indexInfo,
+										  &state,
+										  &auxState);
 
 	/* Tuple sort closed by table_index_validate_scan */
 	Assert(state.tuplesort == NULL && auxState.tuplesort == NULL);
@@ -3772,6 +3809,8 @@ validate_index(Oid heapId, Oid indexId, Oid auxIndexId, Snapshot snapshot)
 	index_close(auxIndexRelation, NoLock);
 	index_close(indexRelation, NoLock);
 	table_close(heapRelation, NoLock);
+
+	return limitXmin;
 }
 
 /*
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 96f34aadc44..0877a3fbceb 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -606,7 +606,6 @@ DefineIndex(ParseState *pstate,
 	LockRelId	heaprelid;
 	LOCKTAG		heaplocktag;
 	LOCKMODE	lockmode;
-	Snapshot	snapshot;
 	Oid			root_save_userid;
 	int			root_save_sec_context;
 	int			root_save_nestlevel;
@@ -1826,32 +1825,11 @@ DefineIndex(ParseState *pstate,
 	/* Tell concurrent index builds to ignore us, if index qualifies */
 	if (safe_index)
 		set_indexsafe_procflags();
-
-	/*
-	 * Now take the "reference snapshot" that will be used by validate_index()
-	 * to filter candidate tuples.  Beware!  There might still be snapshots in
-	 * use that treat some transaction as in-progress that our reference
-	 * snapshot treats as committed.  If such a recently-committed transaction
-	 * deleted tuples in the table, we will not include them in the index; yet
-	 * those transactions which see the deleting one as still-in-progress will
-	 * expect such tuples to be there once we mark the index as valid.
-	 *
-	 * We solve this by waiting for all endangered transactions to exit before
-	 * we mark the index as valid.
-	 *
-	 * We also set ActiveSnapshot to this snap, since functions in indexes may
-	 * need a snapshot.
-	 */
-	snapshot = RegisterSnapshot(GetTransactionSnapshot());
-	PushActiveSnapshot(snapshot);
 	/*
 	 * Merge content of auxiliary and target indexes - insert any missing index entries.
 	 */
-	validate_index(tableId, indexRelationId, auxIndexRelationId, snapshot);
-	limitXmin = snapshot->xmin;
+	limitXmin = validate_index(tableId, indexRelationId, auxIndexRelationId);
 
-	PopActiveSnapshot();
-	UnregisterSnapshot(snapshot);
 	/*
 	 * The snapshot subsystem could still contain registered snapshots that
 	 * are holding back our process's advertised xmin; in particular, if
@@ -1873,8 +1851,8 @@ DefineIndex(ParseState *pstate,
 	/*
 	 * The index is now valid in the sense that it contains all currently
 	 * interesting tuples.  But since it might not contain tuples deleted just
-	 * before the reference snap was taken, we have to wait out any
-	 * transactions that might have older snapshots.
+	 * before the last snapshot during validating was taken, we have to wait
+	 * out any transactions that might have older snapshots.
 	 */
 	INJECTION_POINT("define-index-before-set-valid", NULL);
 	pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
@@ -4573,7 +4551,6 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
 	{
 		ReindexIndexInfo *newidx = lfirst(lc);
 		TransactionId limitXmin;
-		Snapshot	snapshot;
 
 		StartTransactionCommand();
 
@@ -4588,13 +4565,6 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
 		if (newidx->safe)
 			set_indexsafe_procflags();
 
-		/*
-		 * Take the "reference snapshot" that will be used by validate_index()
-		 * to filter candidate tuples.
-		 */
-		snapshot = RegisterSnapshot(GetTransactionSnapshot());
-		PushActiveSnapshot(snapshot);
-
 		/*
 		 * Update progress for the index to build, with the correct parent
 		 * table involved.
@@ -4606,16 +4576,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
 		progress_vals[3] = newidx->amId;
 		pgstat_progress_update_multi_param(4, progress_index, progress_vals);
 
-		validate_index(newidx->tableId, newidx->indexId, newidx->auxIndexId, snapshot);
-
-		/*
-		 * We can now do away with our active snapshot, we still need to save
-		 * the xmin limit to wait for older snapshots.
-		 */
-		limitXmin = snapshot->xmin;
-
-		PopActiveSnapshot();
-		UnregisterSnapshot(snapshot);
+		limitXmin = validate_index(newidx->tableId, newidx->indexId, newidx->auxIndexId);
 
 		/*
 		 * To ensure no deadlocks, we must commit and start yet another
@@ -4625,10 +4586,13 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
 		CommitTransactionCommand();
 		StartTransactionCommand();
 
+		/* We should now definitely not be advertising any xmin. */
+		Assert(!TransactionIdIsValid(MyProc->xmin));
+
 		/*
 		 * The index is now valid in the sense that it contains all currently
 		 * interesting tuples.  But since it might not contain tuples deleted
-		 * just before the reference snap was taken, we have to wait out any
+		 * just before the latest snap was taken, we have to wait out any
 		 * transactions that might have older snapshots.
 		 *
 		 * Because we don't take a snapshot or Xid in this transaction,
diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat
index 1fb19b16078..ea4b85892d7 100644
--- a/src/backend/utils/misc/guc_parameters.dat
+++ b/src/backend/utils/misc/guc_parameters.dat
@@ -640,6 +640,15 @@
   boot_val => 'DEFAULT_ASSERT_ENABLED',
 },
 
+{ name => 'debug_cic_validate_snapshot_interval', type => 'int', context => 'PGC_USERSET', group => 'DEVELOPER_OPTIONS',
+  short_desc => 'Minimum time between snapshot refreshes during CIC validation (0 to disable).',
+  flags => 'GUC_NOT_IN_SAMPLE | GUC_UNIT_MS',
+  variable => 'debug_cic_validate_snapshot_interval',
+  boot_val => '100',
+  min => '0',
+  max => 'INT_MAX',
+},
+
 { name => 'debug_cic_validate_store_mem_pct', type => 'int', context => 'PGC_USERSET', group => 'DEVELOPER_OPTIONS',
   short_desc => 'Percentage of maintenance_work_mem used for CIC validation tuplestore.',
   flags => 'GUC_NOT_IN_SAMPLE',
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 27d4bca6afd..cc2b51130ee 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -739,12 +739,11 @@ typedef struct TableAmRoutine
 										   TableScanDesc scan);
 
 	/* see table_index_validate_scan for reference about parameters */
-	void		(*index_validate_scan) (Relation table_rel,
-										Relation index_rel,
-										IndexInfo *index_info,
-										Snapshot snapshot,
-										ValidateIndexState *state,
-										ValidateIndexState *aux_state);
+	TransactionId		(*index_validate_scan) (Relation table_rel,
+												Relation index_rel,
+												IndexInfo *index_info,
+												ValidateIndexState *state,
+												ValidateIndexState *aux_state);
 
 
 	/* ------------------------------------------------------------------------
@@ -1909,20 +1908,18 @@ table_index_build_range_scan(Relation table_rel,
  * Note: it is responsibility of that function to close sortstates in
  * both `state` and `auxstate`.
  */
-static inline void
+static inline TransactionId
 table_index_validate_scan(Relation table_rel,
 						  Relation index_rel,
 						  IndexInfo *index_info,
-						  Snapshot snapshot,
 						  ValidateIndexState *state,
 						  ValidateIndexState *auxstate)
 {
-	table_rel->rd_tableam->index_validate_scan(table_rel,
-											   index_rel,
-											   index_info,
-											   snapshot,
-											   state,
-											   auxstate);
+	return table_rel->rd_tableam->index_validate_scan(table_rel,
+													  index_rel,
+													  index_info,
+													  state,
+													  auxstate);
 }
 
 
diff --git a/src/include/access/transam.h b/src/include/access/transam.h
index 55a4ab26b34..923aadbab43 100644
--- a/src/include/access/transam.h
+++ b/src/include/access/transam.h
@@ -415,6 +415,21 @@ NormalTransactionIdOlder(TransactionId a, TransactionId b)
 	return b;
 }
 
+/* return the newer of the two IDs */
+static inline TransactionId
+TransactionIdNewer(TransactionId a, TransactionId b)
+{
+	if (!TransactionIdIsValid(a))
+		return b;
+
+	if (!TransactionIdIsValid(b))
+		return a;
+
+	if (TransactionIdFollows(a, b))
+		return a;
+	return b;
+}
+
 /* return the newer of the two IDs */
 static inline FullTransactionId
 FullTransactionIdNewer(FullTransactionId a, FullTransactionId b)
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index bbdc82448ee..ed1995007e5 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -160,7 +160,7 @@ extern void index_build(Relation heapRelation,
 						bool parallel,
 						bool progress);
 
-extern void validate_index(Oid heapId, Oid indexId, Oid auxIndexId, Snapshot snapshot);
+extern TransactionId validate_index(Oid heapId, Oid indexId, Oid auxIndexId);
 
 extern void index_set_state_flags(Oid indexId, IndexStateFlagsAction action);
 
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 6bf0dd63247..e81727cb2fa 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 debug_cic_validate_store_mem_pct;
+extern PGDLLIMPORT int debug_cic_validate_snapshot_interval;
 extern PGDLLIMPORT int max_parallel_maintenance_workers;
 
 /*
diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index ee290698b31..2de4ab61b4b 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -23,6 +23,7 @@ tests += {
       't/012_ddlutils.pl',
       't/013_temp_obj_multisession.pl',
       't/014_log_statement_max_length.pl',
+      't/015_cic_validate_snapshot.pl',
     ],
     # The injection points are cluster-wide, so disable installcheck
     'runningcheck': false,
diff --git a/src/test/modules/test_misc/t/015_cic_validate_snapshot.pl b/src/test/modules/test_misc/t/015_cic_validate_snapshot.pl
new file mode 100644
index 00000000000..ea82fcbea23
--- /dev/null
+++ b/src/test/modules/test_misc/t/015_cic_validate_snapshot.pl
@@ -0,0 +1,140 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Verify snapshot resets during the validation phase of CREATE INDEX
+# CONCURRENTLY:
+# - round 1 attaches an error to the unconditional
+#   "validate-index-snapshot-reset" injection point and expects CIC to
+#   fail, proving that resets do happen;
+# - round 2 attaches an error to the "validate-index-xmin-not-advanced"
+#   injection point and expects CIC to succeed, proving that every reset
+#   fully advances the backend's xmin; a notice on the reset point
+#   confirms that resets did happen during that run.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+plan skip_all => 'Injection points not supported by this build'
+  unless $ENV{enable_injection_points} eq 'yes';
+
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init();
+$node->append_conf('postgresql.conf', 'fsync = off');
+# Injection point notices must reach the server log (round 2 checks them).
+$node->append_conf('postgresql.conf', 'log_min_messages = notice');
+$node->start();
+
+plan skip_all => 'Extension injection_points not installed'
+  unless $node->check_extension('injection_points');
+
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+$node->safe_psql('postgres', 'CREATE EXTENSION amcheck;');
+
+$node->safe_psql(
+	'postgres', q[
+CREATE UNLOGGED TABLE tbl (i int);
+INSERT INTO tbl SELECT generate_series(1, 1000);
+-- The function lies about immutability so that the build scan of CIC can
+-- be paused on an advisory lock held by another session.
+CREATE FUNCTION indexpr(int) RETURNS int IMMUTABLE LANGUAGE plpgsql
+AS $$ BEGIN PERFORM pg_advisory_xact_lock(4971); RETURN $1; END $$;
+]);
+
+my $blocker = $node->background_psql('postgres', on_error_stop => 0);
+my $cic = $node->background_psql('postgres', on_error_stop => 0);
+
+# Pause the CIC build scan on the first evaluation of the index
+# expression, insert rows that will only be reachable through the
+# auxiliary index (making the validation phase do real work), then let
+# CIC finish.
+sub run_cic_with_candidates
+{
+	my ($marker, $first_new_row) = @_;
+
+	$blocker->query_safe(q[SELECT pg_advisory_lock(4971);]);
+
+	$cic->query_until(
+		qr/$marker/, qq[
+\\echo $marker
+SET debug_cic_validate_snapshot_interval = 1;
+CREATE INDEX CONCURRENTLY idx ON tbl (indexpr(i));
+]);
+
+	# Wait for the build scan to block on the advisory lock.
+	$node->poll_query_until(
+		'postgres', q[
+SELECT EXISTS (SELECT FROM pg_locks
+	WHERE locktype = 'advisory' AND objid = 4971 AND NOT granted)])
+	  or die "timed out waiting for CIC to block on the advisory lock";
+
+	# These rows are seen neither by the build scan snapshot nor by the
+	# target index (not ready yet), so the validation phase will have to
+	# fetch and insert all of them.
+	$node->safe_psql('postgres',
+		q[INSERT INTO tbl SELECT generate_series(100001, 200000);]);
+
+	$blocker->query_safe(q[SELECT pg_advisory_unlock(4971);]);
+}
+
+# Round 1: snapshot resets must happen during validation.
+$node->safe_psql('postgres',
+	q[SELECT injection_points_attach('validate-index-snapshot-reset', 'error');]);
+
+run_cic_with_candidates('starting_cic1', 100001);
+
+$node->wait_for_log(
+	qr/error triggered for injection point validate-index-snapshot-reset/);
+ok(1, 'snapshot reset happened during CIC validation');
+
+# The failed CIC left an invalid index behind; dropping it must take the
+# auxiliary index with it.
+$node->poll_query_until(
+	'postgres', q[
+SELECT NOT EXISTS (SELECT FROM pg_stat_activity
+	WHERE query LIKE 'CREATE INDEX CONCURRENTLY%' AND state = 'active')])
+  or die "timed out waiting for CIC to fail";
+$node->safe_psql('postgres', q[DROP INDEX idx;]);
+is( $node->safe_psql(
+		'postgres',
+		q[SELECT count(*) FROM pg_class WHERE relname IN ('idx', 'idx_ccaux')]),
+	'0',
+	'failed CIC cleaned up after DROP INDEX');
+
+$node->safe_psql('postgres',
+	q[SELECT injection_points_detach('validate-index-snapshot-reset');]);
+
+# Round 2: every reset must fully advance xmin.  The reset point now only
+# raises a notice, both to prove that resets do happen during this
+# (successful) run and to let it complete.
+$node->safe_psql('postgres',
+	q[SELECT injection_points_attach('validate-index-snapshot-reset', 'notice');]);
+$node->safe_psql('postgres',
+	q[SELECT injection_points_attach('validate-index-xmin-not-advanced', 'error');]);
+
+my $log_offset = -s $node->logfile;
+
+run_cic_with_candidates('starting_cic2', 200001);
+
+$node->poll_query_until(
+	'postgres', q[
+SELECT indisvalid FROM pg_index
+	WHERE indexrelid = (SELECT oid FROM pg_class WHERE relname = 'idx')])
+  or die "timed out waiting for CIC to complete";
+ok(1, 'CIC completed with snapshot resets fully advancing xmin');
+
+$node->wait_for_log(
+	qr/notice triggered for injection point validate-index-snapshot-reset/,
+	$log_offset);
+ok(1, 'snapshot resets happened during the successful CIC');
+
+# The index must contain everything despite the snapshot changes.
+is( $node->safe_psql('postgres',
+		q[SELECT bt_index_check('idx', heapallindexed => true)]),
+	'', 'index is complete');
+
+eval { $blocker->quit };
+eval { $cic->quit };
+
+done_testing();
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index e4072033d7f..bf793d9b94d 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -3392,6 +3392,9 @@ DROP INDEX aux_index_ind6;
 --------+---------+-----------+----------+---------
  c1     | integer |           |          | 
 
+SET default_transaction_isolation = 'repeatable read';
+CREATE INDEX CONCURRENTLY aux_index_ind6 ON aux_index_tab5 (c1);
+SET default_transaction_isolation = 'read committed';
 DROP TABLE aux_index_tab5;
 -- Check handling of indexes with expressions and predicates.  The
 -- definitions of the rebuilt indexes should match the original
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index 5fe373b7b34..30f33eb898c 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1404,6 +1404,10 @@ DROP INDEX aux_index_ind6;
 -- Make sure auxiliary index dropped too
 \d aux_index_tab5
 
+SET default_transaction_isolation = 'repeatable read';
+CREATE INDEX CONCURRENTLY aux_index_ind6 ON aux_index_tab5 (c1);
+SET default_transaction_isolation = 'read committed';
+
 DROP TABLE aux_index_tab5;
 
 -- Check handling of indexes with expressions and predicates.  The
-- 
2.43.0

