From 6d2a9ae0ed172341f78b6d2f4b30fb315d9ce528 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 2 Jan 2018 19:16:16 -0300
Subject: [PATCH] Fix deadlock hazard in CREATE INDEX CONCURRENTLY
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

Multiple sessions doing CREATE INDEX CONCURRENTLY simultaneously are
supposed to be able to work in parallel, as evidenced by fixes in commit
c3d09b3bd23f specifically to support this case.  In reality, one of the
sessions would be aborted by a misterious "deadlock detected" error.

Jeff Janes diagnosed that this is because of leftover snapshots used for
system catalog scans -- this was broken by 8aa3e47510b9 keeping track of
(registering) the catalog snapshot.  To fix the deadlocks, it's enough
to de-register that snapshot prior to waiting.

Backpatch to 9.4, which introduced MVCC catalog scans.

Include an isolationtester spec that 8 out of 10 times reproduces the
deadlock with the unpatched code for me (Álvaro).

Author: Jeff Janes
Diagnosed-by: Jeff Janes
Reported-by: Jeremy Finzel
Discussion: https://postgr.es/m/CAMa1XUhHjCv8Qkx0WOr1Mpm_R4qxN26EibwCrj0Oor2YBUFUTg%40mail.gmail.com
---
 src/backend/commands/indexcmds.c             |  3 ++
 src/test/isolation/expected/multiple-cic.out | 19 ++++++++++
 src/test/isolation/isolation_schedule        |  1 +
 src/test/isolation/specs/multiple-cic.spec   | 40 ++++++++++++++++++++
 4 files changed, 63 insertions(+)
 create mode 100644 src/test/isolation/expected/multiple-cic.out
 create mode 100644 src/test/isolation/specs/multiple-cic.spec

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 620704ec49..84b6fc84a3 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -840,11 +840,14 @@ DefineIndex(Oid relationId,
 	 * doing CREATE INDEX CONCURRENTLY, which would see our snapshot as one
 	 * they must wait for.  But first, save the snapshot's xmin to use as
 	 * limitXmin for GetCurrentVirtualXIDs().
+	 *
+	 * Our catalog snapshot could have the same effect, so drop that one too.
 	 */
 	limitXmin = snapshot->xmin;
 
 	PopActiveSnapshot();
 	UnregisterSnapshot(snapshot);
+	InvalidateCatalogSnapshot();
 
 	/*
 	 * The index is now valid in the sense that it contains all currently
diff --git a/src/test/isolation/expected/multiple-cic.out b/src/test/isolation/expected/multiple-cic.out
new file mode 100644
index 0000000000..cc57940392
--- /dev/null
+++ b/src/test/isolation/expected/multiple-cic.out
@@ -0,0 +1,19 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s2l s1i s2i
+step s2l: SELECT pg_advisory_lock(281457);
+pg_advisory_lock
+
+               
+step s1i: 
+		CREATE INDEX CONCURRENTLY mcic_one_pkey ON mcic_one (id)
+		WHERE lck_shr(281457);
+ <waiting ...>
+step s2i: 
+		CREATE INDEX CONCURRENTLY mcic_two_pkey ON mcic_two (id)
+		WHERE unlck();
+
+step s1i: <... completed>
+s1             
+
+               
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 7dad3c2316..5060b06dea 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -55,6 +55,7 @@ test: skip-locked-2
 test: skip-locked-3
 test: skip-locked-4
 test: drop-index-concurrently-1
+test: multiple-cic
 test: alter-table-1
 test: alter-table-2
 test: alter-table-3
diff --git a/src/test/isolation/specs/multiple-cic.spec b/src/test/isolation/specs/multiple-cic.spec
new file mode 100644
index 0000000000..a7ba4eb4fd
--- /dev/null
+++ b/src/test/isolation/specs/multiple-cic.spec
@@ -0,0 +1,40 @@
+# Test multiple CREATE INDEX CONCURRENTLY working simultaneously
+
+setup
+{
+  CREATE TABLE mcic_one (
+	id int
+  );
+  CREATE TABLE mcic_two (
+	id int
+  );
+  CREATE FUNCTION lck_shr(bigint) RETURNS bool IMMUTABLE LANGUAGE plpgsql AS $$
+     BEGIN PERFORM pg_advisory_lock_shared($1); RETURN true; END;
+  $$;
+  CREATE FUNCTION unlck() RETURNS bool IMMUTABLE LANGUAGE plpgsql AS $$
+     BEGIN PERFORM pg_advisory_unlock_all(); RETURN true; END;
+  $$;
+}
+teardown
+{
+  DROP TABLE mcic_one, mcic_two;
+  DROP FUNCTION lck_shr(bigint);
+  DROP FUNCTION unlck();
+}
+
+session "s1"
+step "s1i"	{
+		CREATE INDEX CONCURRENTLY mcic_one_pkey ON mcic_one (id)
+		WHERE lck_shr(281457);
+	}
+teardown	{ SELECT pg_advisory_unlock_all() AS "s1"; }
+
+
+session "s2"
+step "s2l"  { SELECT pg_advisory_lock(281457); }
+step "s2i"	{
+		CREATE INDEX CONCURRENTLY mcic_two_pkey ON mcic_two (id)
+		WHERE unlck();
+	}
+
+permutation "s2l" "s1i" "s2i"
-- 
2.40.0