Fix for deadlock between select and drop_chunks

Acquire foreign key related table locks prior to dropping chunks.  This
helps avoid deadlocks in scenarios where select and drop chunks are
trying to acquire locks on the same set of tables.
Changes to isolation test suite to prevent diffs related to displaying
hypertable id differences across test suite runs.

Fixes #865

.travis.yml

@@ -41,7 +41,7 @@ script:
   - ci_env=`bash <(curl -s https://codecov.io/env)`
   - docker exec -it $ci_env pgbuild /bin/bash -c "cd /build/debug && bash <(curl -s https://codecov.io/bash) || echo \"Codecov did not collect coverage reports\" "
 after_failure:
-  - docker exec -it pgbuild cat /build/debug/test/regression.diffs /build/debug/test/pgtest/regressions.diffs
+  - docker exec -it pgbuild cat /build/debug/test/regression.diffs /build/debug/test/pgtest/regressions.diffs /build/debug/test/isolation/regression.diffs
 after_script:
   - docker rm -f pgbuild
 

src/chunk.c

@@ -694,6 +694,7 @@ chunk_tuple_found(TupleInfo *ti, void *arg)
 	return SCAN_DONE;
 }
 
+
 /* Fill in a chunk stub. The stub data structure needs the chunk ID and constraints set.
  * The rest of the fields will be filled in from the table data. */
 static Chunk *
@@ -1890,7 +1891,52 @@ ts_chunk_drop_chunks(PG_FUNCTION_ARGS)
 	foreach(lc, ht_oids)
 	{
 		Oid			table_relid = lfirst_oid(lc);
+		List	   *fk_relids = NIL;
+		ListCell   *lf;
 
+		/* get foreign key tables associated with the hypertable */
+		{
+			List	   *cachedfkeys = NIL;
+			ListCell   *lf;
+			Relation	table_rel;
+
+			table_rel = heap_open(table_relid, AccessShareLock);
+
+			/*
+			 * this list is from the relcache and can disappear with a cache
+			 * flush, so no further catalog access till we save the fk relids
+			 */
+			cachedfkeys = RelationGetFKeyList(table_rel);
+			foreach(lf, cachedfkeys)
+			{
+				ForeignKeyCacheInfo *cachedfk = (ForeignKeyCacheInfo *) lfirst(lf);
+
+				/*
+				 * conrelid should always be that of the table we're
+				 * considering
+				 */
+				Assert(cachedfk->conrelid == RelationGetRelid(table_rel));
+				fk_relids = lappend_oid(fk_relids, cachedfk->confrelid);
+			}
+			heap_close(table_rel, AccessShareLock);
+		}
+
+		/*
+		 * We have a FK between hypertable H and PAR. Hypertable H has number
+		 * of chunks C1, C2, etc. When we execute "drop table C", PG acquires
+		 * locks on C and PAR. If we have a query as "select * from
+		 * hypertable", this acquires a lock on C and PAR as well. But the
+		 * order of the locks is not the same and results in deadlocks. -
+		 * github issue 865 We hope to alleviate the problem by aquiring a
+		 * lock on PAR before executing the drop table stmt. This is not
+		 * fool-proof as we could have multiple fkrelids and the order of lock
+		 * acquistion for these could differ as well. Do not unlock - let the
+		 * transaction semantics take care of it.
+		 */
+		foreach(lf, fk_relids)
+		{
+			LockRelationOid(lfirst_oid(lf), AccessExclusiveLock);
+		}
 		do_drop_chunks(table_relid, older_than_datum, newer_than_datum, older_than_type, newer_than_type, cascade);
 	}
 

test/isolation/expected/deadlock_dropchunks_select.out

@@ -0,0 +1,68 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1a s1b s2a s2b
+step s1a: select drop_chunks( '2018-12-25 00:00'::timestamptz, 'dt');
+drop_chunks    
+
+               
+step s1b: COMMIT;
+step s2a: SELECT typ, loc, mtim FROM DT , SL , ST WHERE SL.lid = DT.lid AND ST.sid = DT.sid AND mtim >= '2018-12-01 03:00:00+00' AND mtim <= '2018-12-01 04:00:00+00' AND typ = 'T1' ;
+typ            loc            mtim           
+
+step s2b: COMMIT;
+
+starting permutation: s1a s2a s1b s2b
+step s1a: select drop_chunks( '2018-12-25 00:00'::timestamptz, 'dt');
+drop_chunks    
+
+               
+step s2a: SELECT typ, loc, mtim FROM DT , SL , ST WHERE SL.lid = DT.lid AND ST.sid = DT.sid AND mtim >= '2018-12-01 03:00:00+00' AND mtim <= '2018-12-01 04:00:00+00' AND typ = 'T1' ; <waiting ...>
+step s1b: COMMIT;
+step s2a: <... completed>
+typ            loc            mtim           
+
+step s2b: COMMIT;
+
+starting permutation: s1a s2a s2b s1b
+step s1a: select drop_chunks( '2018-12-25 00:00'::timestamptz, 'dt');
+drop_chunks    
+
+               
+step s2a: SELECT typ, loc, mtim FROM DT , SL , ST WHERE SL.lid = DT.lid AND ST.sid = DT.sid AND mtim >= '2018-12-01 03:00:00+00' AND mtim <= '2018-12-01 04:00:00+00' AND typ = 'T1' ; <waiting ...>
+step s2a: <... completed>
+ERROR:  canceling statement due to user request
+step s2b: COMMIT;
+step s1b: COMMIT;
+
+starting permutation: s2a s1a s1b s2b
+step s2a: SELECT typ, loc, mtim FROM DT , SL , ST WHERE SL.lid = DT.lid AND ST.sid = DT.sid AND mtim >= '2018-12-01 03:00:00+00' AND mtim <= '2018-12-01 04:00:00+00' AND typ = 'T1' ;
+typ            loc            mtim           
+
+step s1a: select drop_chunks( '2018-12-25 00:00'::timestamptz, 'dt'); <waiting ...>
+step s1a: <... completed>
+ERROR:  canceling statement due to user request
+step s1b: COMMIT;
+step s2b: COMMIT;
+
+starting permutation: s2a s1a s2b s1b
+step s2a: SELECT typ, loc, mtim FROM DT , SL , ST WHERE SL.lid = DT.lid AND ST.sid = DT.sid AND mtim >= '2018-12-01 03:00:00+00' AND mtim <= '2018-12-01 04:00:00+00' AND typ = 'T1' ;
+typ            loc            mtim           
+
+step s1a: select drop_chunks( '2018-12-25 00:00'::timestamptz, 'dt'); <waiting ...>
+step s2b: COMMIT;
+step s1a: <... completed>
+drop_chunks    
+
+               
+step s1b: COMMIT;
+
+starting permutation: s2a s2b s1a s1b
+step s2a: SELECT typ, loc, mtim FROM DT , SL , ST WHERE SL.lid = DT.lid AND ST.sid = DT.sid AND mtim >= '2018-12-01 03:00:00+00' AND mtim <= '2018-12-01 04:00:00+00' AND typ = 'T1' ;
+typ            loc            mtim           
+
+step s2b: COMMIT;
+step s1a: select drop_chunks( '2018-12-25 00:00'::timestamptz, 'dt');
+drop_chunks    
+
+               
+step s1b: COMMIT;

test/isolation/expected/isolation_nop.out

@@ -1,9 +1,9 @@
 Parsed test spec with 1 sessions
 
 starting permutation: s1a
-create_hypertable
+table_name     
 
-(1,public,ts_cluster_test,t)
+ts_cluster_test
 step s1a: SELECT pg_sleep(0);
 pg_sleep       
 

test/isolation/expected/read_committed_insert.out

@@ -1,18 +1,18 @@
 Parsed test spec with 2 sessions
 
 starting permutation: s1a s1c s2a s2b
-create_hypertable
+table_name     
 
-(2,public,ts_cluster_test,t)
+ts_cluster_test
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1);
 step s1c: COMMIT;
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1);
 step s2b: COMMIT;
 
 starting permutation: s1a s2a s1c s2b
-create_hypertable
+table_name     
 
-(3,public,ts_cluster_test,t)
+ts_cluster_test
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1);
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1); <waiting ...>
 step s1c: COMMIT;
@@ -20,9 +20,9 @@ step s2a: <... completed>
 step s2b: COMMIT;
 
 starting permutation: s1a s2a s2b s1c
-create_hypertable
+table_name     
 
-(4,public,ts_cluster_test,t)
+ts_cluster_test
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1);
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1); <waiting ...>
 step s2a: <... completed>
@@ -31,9 +31,9 @@ step s2b: COMMIT;
 step s1c: COMMIT;
 
 starting permutation: s2a s1a s1c s2b
-create_hypertable
+table_name     
 
-(5,public,ts_cluster_test,t)
+ts_cluster_test
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1);
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1); <waiting ...>
 step s1a: <... completed>
@@ -42,9 +42,9 @@ step s1c: COMMIT;
 step s2b: COMMIT;
 
 starting permutation: s2a s1a s2b s1c
-create_hypertable
+table_name     
 
-(6,public,ts_cluster_test,t)
+ts_cluster_test
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1);
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1); <waiting ...>
 step s2b: COMMIT;
@@ -52,9 +52,9 @@ step s1a: <... completed>
 step s1c: COMMIT;
 
 starting permutation: s2a s2b s1a s1c
-create_hypertable
+table_name     
 
-(7,public,ts_cluster_test,t)
+ts_cluster_test
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1);
 step s2b: COMMIT;
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1);

test/isolation/expected/read_uncommitted_insert.out

@@ -1,18 +1,18 @@
 Parsed test spec with 2 sessions
 
 starting permutation: s1a s1c s2a s2b
-create_hypertable
+table_name     
 
-(8,public,ts_cluster_test,t)
+ts_cluster_test
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1);
 step s1c: COMMIT;
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1);
 step s2b: COMMIT;
 
 starting permutation: s1a s2a s1c s2b
-create_hypertable
+table_name     
 
-(9,public,ts_cluster_test,t)
+ts_cluster_test
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1);
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1); <waiting ...>
 step s1c: COMMIT;
@@ -20,9 +20,9 @@ step s2a: <... completed>
 step s2b: COMMIT;
 
 starting permutation: s1a s2a s2b s1c
-create_hypertable
+table_name     
 
-(10,public,ts_cluster_test,t)
+ts_cluster_test
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1);
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1); <waiting ...>
 step s2a: <... completed>
@@ -31,9 +31,9 @@ step s2b: COMMIT;
 step s1c: COMMIT;
 
 starting permutation: s2a s1a s1c s2b
-create_hypertable
+table_name     
 
-(11,public,ts_cluster_test,t)
+ts_cluster_test
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1);
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1); <waiting ...>
 step s1a: <... completed>
@@ -42,9 +42,9 @@ step s1c: COMMIT;
 step s2b: COMMIT;
 
 starting permutation: s2a s1a s2b s1c
-create_hypertable
+table_name     
 
-(12,public,ts_cluster_test,t)
+ts_cluster_test
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1);
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1); <waiting ...>
 step s2b: COMMIT;
@@ -52,9 +52,9 @@ step s1a: <... completed>
 step s1c: COMMIT;
 
 starting permutation: s2a s2b s1a s1c
-create_hypertable
+table_name     
 
-(13,public,ts_cluster_test,t)
+ts_cluster_test
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1);
 step s2b: COMMIT;
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1);

test/isolation/expected/repeatable_read_insert.out

@@ -1,18 +1,18 @@
 Parsed test spec with 2 sessions
 
 starting permutation: s1a s1c s2a s2b
-create_hypertable
+table_name     
 
-(14,public,ts_cluster_test,t)
+ts_cluster_test
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1);
 step s1c: COMMIT;
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1);
 step s2b: COMMIT;
 
 starting permutation: s1a s2a s1c s2b
-create_hypertable
+table_name     
 
-(15,public,ts_cluster_test,t)
+ts_cluster_test
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1);
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1); <waiting ...>
 step s1c: COMMIT;
@@ -20,9 +20,9 @@ step s2a: <... completed>
 step s2b: COMMIT;
 
 starting permutation: s1a s2a s2b s1c
-create_hypertable
+table_name     
 
-(16,public,ts_cluster_test,t)
+ts_cluster_test
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1);
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1); <waiting ...>
 step s2a: <... completed>
@@ -31,9 +31,9 @@ step s2b: COMMIT;
 step s1c: COMMIT;
 
 starting permutation: s2a s1a s1c s2b
-create_hypertable
+table_name     
 
-(17,public,ts_cluster_test,t)
+ts_cluster_test
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1);
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1); <waiting ...>
 step s1a: <... completed>
@@ -42,9 +42,9 @@ step s1c: COMMIT;
 step s2b: COMMIT;
 
 starting permutation: s2a s1a s2b s1c
-create_hypertable
+table_name     
 
-(18,public,ts_cluster_test,t)
+ts_cluster_test
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1);
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1); <waiting ...>
 step s2b: COMMIT;
@@ -52,9 +52,9 @@ step s1a: <... completed>
 step s1c: COMMIT;
 
 starting permutation: s2a s2b s1a s1c
-create_hypertable
+table_name     
 
-(19,public,ts_cluster_test,t)
+ts_cluster_test
 step s2a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090002', 0.72, 1);
 step s2b: COMMIT;
 step s1a: INSERT INTO ts_cluster_test VALUES ('2017-01-20T090001', 23.4, 1);

test/isolation/specs/deadlock_dropchunks_select.spec

@@ -0,0 +1,27 @@
+##github issue 865 deadlock between select and drop chunks
+
+setup
+{
+ CREATE TABLE ST ( sid int PRIMARY KEY, typ text) ;
+ CREATE TABLE SL ( lid int PRIMARY KEY, loc text) ;
+ CREATE TABLE DT ( sid int REFERENCES ST(sid), lid int REFERENCES SL(lid), mtim timestamp with time zone ) ;
+ SELECT create_hypertable('DT', 'mtim', chunk_time_interval => interval '1 day');
+ INSERT INTO SL  VALUES (1, 'LA');
+ INSERT INTO ST  VALUES (1, 'T1');
+ INSERT INTO DT (sid,lid,mtim)
+ SELECT 1,1, generate_series( '2018-12-01 00:00'::timestamp, '2018-12-31 12:00','1 minute') ;
+}
+
+teardown 
+{ DROP TABLE DT; DROP table ST; DROP table SL; }
+
+session "s1"
+setup	{ BEGIN;}
+step "s1a"	{ select drop_chunks( '2018-12-25 00:00'::timestamptz, 'dt'); }
+step "s1b"	{ COMMIT; }
+
+session "s2"
+setup	{ BEGIN;  }
+step "s2a"	{  SELECT typ, loc, mtim FROM DT , SL , ST WHERE SL.lid = DT.lid AND ST.sid = DT.sid AND mtim >= '2018-12-01 03:00:00+00' AND mtim <= '2018-12-01 04:00:00+00' AND typ = 'T1' ; }
+step "s2b"	{ COMMIT; }
+

test/isolation/specs/isolation_nop.spec

@@ -1,6 +1,6 @@
 setup{
     CREATE TABLE ts_cluster_test(time timestamptz, temp float, location int);
-    SELECT create_hypertable('ts_cluster_test', 'time', chunk_time_interval => interval '1 day');
+    SELECT table_name from create_hypertable('ts_cluster_test', 'time', chunk_time_interval => interval '1 day');
 }
 
 teardown {

test/isolation/specs/read_committed_insert.spec

@@ -1,7 +1,8 @@
 setup
 {
  CREATE TABLE ts_cluster_test(time timestamptz, temp float, location int);
- SELECT create_hypertable('ts_cluster_test', 'time', chunk_time_interval => interval '1 day');
+ SELECT table_name from create_hypertable('ts_cluster_test', 'time', chunk_time_interval => interval '1 day');
+ 
 }
 
 teardown { DROP TABLE ts_cluster_test; }

test/isolation/specs/read_uncommitted_insert.spec

@@ -1,7 +1,7 @@
 setup
 {
  CREATE TABLE ts_cluster_test(time timestamptz, temp float, location int);
- SELECT create_hypertable('ts_cluster_test', 'time', chunk_time_interval => interval '1 day');
+ SELECT table_name from create_hypertable('ts_cluster_test', 'time', chunk_time_interval => interval '1 day');
 }
 
 teardown { DROP TABLE ts_cluster_test; }

test/isolation/specs/repeatable_read_insert.spec

@@ -1,7 +1,7 @@
 setup
 {
  CREATE TABLE ts_cluster_test(time timestamptz, temp float, location int);
- SELECT create_hypertable('ts_cluster_test', 'time', chunk_time_interval => interval '1 day');
+ SELECT table_name from create_hypertable('ts_cluster_test', 'time', chunk_time_interval => interval '1 day');
 }
 
 teardown { DROP TABLE ts_cluster_test; }