Date: Mon, 10 Aug 2020 19:38:15 -0400
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: remove spurious CREATE INDEX CONCURRENTLY wait
Message-ID: <20200810233815.GA18970@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="KsGdsel6WgEHnImy"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk


--KsGdsel6WgEHnImy
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

I previously[1] posted a patch to have multiple CREATE INDEX CONCURRENTLY
not wait for the slowest of them.  This is an update of that, with minor
conflicts fixed and a fresh thread.

To recap: currently, any CREATE INDEX CONCURRENTLY will wait for all
other CICs running concurrently to finish, because they can't be
distinguished amidst other old snapshots.  We can change things by
having CIC set a special flag in PGPROC (like PROC_IN_VACUUM) indicating
that it's doing CIC; other CICs will see that flag and will know that
they don't need to wait for those processes.  With this, CIC on small
tables don't have to wait for CIC on large tables to complete.

[1] https://postgr.es/m/20200805021109.GA9079@alvherre.pgsql


-- 
Álvaro Herrera                            http://www.linkedin.com/in/alvherre
"Escucha y olvidarás; ve y recordarás; haz y entenderás" (Confucio)

--KsGdsel6WgEHnImy
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0001-Flag-CREATE-INDEX-CONCURRENTLY-to-avoid-spurious-wai.patch"

From 2596c3033aacceb021463f58b50e2c4eed8a5ab2 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 4 Aug 2020 22:04:57 -0400
Subject: [PATCH] Flag CREATE INDEX CONCURRENTLY to avoid spurious waiting

---
 src/backend/commands/indexcmds.c | 13 +++++++++++--
 src/include/storage/proc.h       |  4 +++-
 src/include/storage/procarray.h  |  4 ++++
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a3cb3cd47f..241c8a337e 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -393,7 +393,7 @@ WaitForOlderSnapshots(TransactionId limitXmin, bool progress)
 	VirtualTransactionId *old_snapshots;
 
 	old_snapshots = GetCurrentVirtualXIDs(limitXmin, true, false,
-										  PROC_IS_AUTOVACUUM | PROC_IN_VACUUM,
+										  PROC_IS_AUTOVACUUM | PROC_IN_VACUUM | PROC_IN_CIC,
 										  &n_old_snapshots);
 	if (progress)
 		pgstat_progress_update_param(PROGRESS_WAITFOR_TOTAL, n_old_snapshots);
@@ -413,7 +413,7 @@ WaitForOlderSnapshots(TransactionId limitXmin, bool progress)
 
 			newer_snapshots = GetCurrentVirtualXIDs(limitXmin,
 													true, false,
-													PROC_IS_AUTOVACUUM | PROC_IN_VACUUM,
+													PROC_IS_AUTOVACUUM | PROC_IN_VACUUM | PROC_IN_CIC,
 													&n_newer_snapshots);
 			for (j = i; j < n_old_snapshots; j++)
 			{
@@ -1420,6 +1420,9 @@ DefineIndex(Oid relationId,
 	CommitTransactionCommand();
 	StartTransactionCommand();
 
+	/* Set flag for other concurrent index builds to ignore us */
+	MyPgXact->vacuumFlags |= PROC_IN_CIC;
+
 	/*
 	 * The index is now visible, so we can report the OID.
 	 */
@@ -1482,6 +1485,9 @@ DefineIndex(Oid relationId,
 	CommitTransactionCommand();
 	StartTransactionCommand();
 
+	/* Set flag for other concurrent index builds to ignore us */
+	MyPgXact->vacuumFlags |= PROC_IN_CIC;
+
 	/*
 	 * Phase 3 of concurrent index build
 	 *
@@ -1541,6 +1547,9 @@ DefineIndex(Oid relationId,
 	CommitTransactionCommand();
 	StartTransactionCommand();
 
+	/* Set flag for other concurrent index builds to ignore us */
+	MyPgXact->vacuumFlags |= PROC_IN_CIC;
+
 	/* We should now definitely not be advertising any xmin. */
 	Assert(MyPgXact->xmin == InvalidTransactionId);
 
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 5ceb2494ba..b030dcde6c 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -52,6 +52,8 @@ struct XidCache
  */
 #define		PROC_IS_AUTOVACUUM	0x01	/* is it an autovac worker? */
 #define		PROC_IN_VACUUM		0x02	/* currently running lazy vacuum */
+#define		PROC_IN_CIC			0x04	/* currently running CREATE INDEX
+										   CONCURRENTLY */
 #define		PROC_VACUUM_FOR_WRAPAROUND	0x08	/* set by autovac only */
 #define		PROC_IN_LOGICAL_DECODING	0x10	/* currently doing logical
 												 * decoding outside xact */
@@ -59,7 +61,7 @@ struct XidCache
 
 /* flags reset at EOXact */
 #define		PROC_VACUUM_STATE_MASK \
-	(PROC_IN_VACUUM | PROC_VACUUM_FOR_WRAPAROUND)
+	(PROC_IN_VACUUM | PROC_IN_CIC | PROC_VACUUM_FOR_WRAPAROUND)
 
 /*
  * We allow a small number of "weak" relation locks (AccessShareLock,
diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h
index 01040d76e1..c6edeb36e0 100644
--- a/src/include/storage/procarray.h
+++ b/src/include/storage/procarray.h
@@ -29,17 +29,21 @@
  */
 #define		PROCARRAY_VACUUM_FLAG			0x02	/* currently running lazy
 													 * vacuum */
+#define		PROCARRAY_CIC_FLAG				0x04	/* currently running CREATE INDEX
+													 * CONCURRENTLY */
 #define		PROCARRAY_LOGICAL_DECODING_FLAG 0x10	/* currently doing logical
 													 * decoding outside xact */
 
 #define		PROCARRAY_SLOTS_XMIN			0x20	/* replication slot xmin,
 													 * catalog_xmin */
+
 /*
  * Only flags in PROCARRAY_PROC_FLAGS_MASK are considered when matching
  * PGXACT->vacuumFlags. Other flags are used for different purposes and
  * have no corresponding PROC flag equivalent.
  */
 #define		PROCARRAY_PROC_FLAGS_MASK	(PROCARRAY_VACUUM_FLAG | \
+										 PROCARRAY_CIC_FLAG | \
 										 PROCARRAY_LOGICAL_DECODING_FLAG)
 
 /* Use the following flags as an input "flags" to GetOldestXmin function */
-- 
2.20.1


--KsGdsel6WgEHnImy--