On Sat, May 04, 2019 at 09:59:20PM +0900, Michael Paquier wrote: > The result should be no deadlocks happening in the two sessions > running the reindex. I can see the deadlock easily with three psql > sessions, running manually the queries.
+ * If the OID isn't valid, it means the index as concurrently dropped, + * which is not a problem for us; just return normally. Typo here s/as/is/. I have looked closer at the patch and the change proposed looks good to me. Now, what do we do about the potential deadlock issues in WaitForOlderSnapshots? The attached is an isolation test able to reproduce the deadlock within WaitForOlderSnapshots() with two parallel REINDEX CONCURRENTLY. I'd like to think that the best way to do that would be to track in vacuumFlags the backends running a REINDEX and just exclude them from GetCurrentVirtualXIDs() because we don't actually care about missing index entries in this case like VACUUM. But it looks also to me that is issue is broader and goes down to utility commands which can take a lock on a table which cannot be run in transaction blocks, hence code paths used by CREATE INDEX CONCURRENTLY and DROP INDEX CONCURRENTLY could also cause a similar deadlock, no? -- Michael
diff --git a/src/test/isolation/expected/reindex-concurrently-ddl.out b/src/test/isolation/expected/reindex-concurrently-ddl.out
new file mode 100644
index 0000000000..14ac712ef4
--- /dev/null
+++ b/src/test/isolation/expected/reindex-concurrently-ddl.out
@@ -0,0 +1,41 @@
+Parsed test spec with 3 sessions
+
+starting permutation: beg1 lock1 index2 index3 end1
+step beg1: BEGIN;
+step lock1: LOCK reind_con_tab IN ROW EXCLUSIVE MODE;
+step index2: REINDEX INDEX CONCURRENTLY reind_con_tab_pkey; <waiting ...>
+step index3: REINDEX INDEX CONCURRENTLY reind_con_tab_pkey; <waiting ...>
+step end1: COMMIT;
+step index2: <... completed>
+step index3: <... completed>
+ERROR: deadlock detected
+
+starting permutation: beg1 lock1 index2 table3 end1
+step beg1: BEGIN;
+step lock1: LOCK reind_con_tab IN ROW EXCLUSIVE MODE;
+step index2: REINDEX INDEX CONCURRENTLY reind_con_tab_pkey; <waiting ...>
+step table3: REINDEX TABLE CONCURRENTLY reind_con_tab; <waiting ...>
+step end1: COMMIT;
+step index2: <... completed>
+step table3: <... completed>
+ERROR: deadlock detected
+
+starting permutation: beg1 lock1 table2 index3 end1
+step beg1: BEGIN;
+step lock1: LOCK reind_con_tab IN ROW EXCLUSIVE MODE;
+step table2: REINDEX TABLE CONCURRENTLY reind_con_tab; <waiting ...>
+step index3: REINDEX INDEX CONCURRENTLY reind_con_tab_pkey; <waiting ...>
+step end1: COMMIT;
+step table2: <... completed>
+step index3: <... completed>
+ERROR: deadlock detected
+
+starting permutation: beg1 lock1 table2 table3 end1
+step beg1: BEGIN;
+step lock1: LOCK reind_con_tab IN ROW EXCLUSIVE MODE;
+step table2: REINDEX TABLE CONCURRENTLY reind_con_tab; <waiting ...>
+step table3: REINDEX TABLE CONCURRENTLY reind_con_tab; <waiting ...>
+step end1: COMMIT;
+step table2: <... completed>
+step table3: <... completed>
+ERROR: deadlock detected
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 11cd24fc98..bb5648aa74 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -45,6 +45,7 @@ test: lock-committed-update
test: lock-committed-keyupdate
test: update-locked-tuple
test: reindex-concurrently
+test: reindex-concurrently-ddl
test: propagate-lock-delete
test: tuplelock-conflict
test: tuplelock-update
diff --git a/src/test/isolation/specs/reindex-concurrently-ddl.spec b/src/test/isolation/specs/reindex-concurrently-ddl.spec
new file mode 100644
index 0000000000..d2c675b9ff
--- /dev/null
+++ b/src/test/isolation/specs/reindex-concurrently-ddl.spec
@@ -0,0 +1,37 @@
+# REINDEX CONCURRENTLY with table locks
+#
+# Ensure that REINDEX CONCURRENTLY works correctly and serially with
+# locks taken on the table worked on.
+
+setup
+{
+ CREATE TABLE reind_con_tab(id serial primary key, data text);
+ INSERT INTO reind_con_tab(data) VALUES ('aa');
+ INSERT INTO reind_con_tab(data) VALUES ('aaa');
+ INSERT INTO reind_con_tab(data) VALUES ('aaaa');
+ INSERT INTO reind_con_tab(data) VALUES ('aaaaa');
+}
+
+teardown
+{
+ DROP TABLE reind_con_tab;
+}
+
+session "s1"
+step "beg1" { BEGIN; }
+step "lock1" { LOCK reind_con_tab IN ROW EXCLUSIVE MODE; }
+step "end1" { COMMIT; }
+
+session "s2"
+step "index2" { REINDEX INDEX CONCURRENTLY reind_con_tab_pkey; }
+step "table2" { REINDEX TABLE CONCURRENTLY reind_con_tab; }
+
+session "s3"
+step "index3" { REINDEX INDEX CONCURRENTLY reind_con_tab_pkey; }
+step "table3" { REINDEX TABLE CONCURRENTLY reind_con_tab; }
+
+# Each REINDEX blocks and should complete serially
+permutation "beg1" "lock1" "index2" "index3" "end1"
+permutation "beg1" "lock1" "index2" "table3" "end1"
+permutation "beg1" "lock1" "table2" "index3" "end1"
+permutation "beg1" "lock1" "table2" "table3" "end1"
signature.asc
Description: PGP signature
