Merge pull request #2494 from citusdata/notsolocky

Relax multi-shard modify locks when enable_deadlock_prevention is disabled

src/backend/distributed/executor/multi_router_executor.c

@@ -369,9 +369,21 @@ AcquireExecutorMultiShardLocks(List *taskList)
 			 * In either case, ShareUpdateExclusive has the desired effect, since
 			 * it conflicts with itself and ExclusiveLock (taken by non-commutative
 			 * writes).
+			 *
+			 * However, some users find this too restrictive, so we allow them to
+			 * reduce to a RowExclusiveLock when citus.enable_deadlock_prevention
+			 * is enabled, which lets multi-shard modifications run in parallel as
+			 * long as they all disable the GUC.
 			 */
 
-			lockMode = ShareUpdateExclusiveLock;
+			if (EnableDeadlockPrevention)
+			{
+				lockMode = ShareUpdateExclusiveLock;
+			}
+			else
+			{
+				lockMode = RowExclusiveLock;
+			}
 		}
 		else
 		{

src/backend/distributed/shared_library_init.c

@@ -565,15 +565,17 @@ RegisterCitusConfigVariables(void)
 
 	DefineCustomBoolVariable(
 		"citus.enable_deadlock_prevention",
-		gettext_noop("Prevents transactions from expanding to multiple nodes"),
+		gettext_noop("Avoids deadlocks by preventing concurrent multi-shard commands"),
-		gettext_noop("When enabled, consecutive DML statements that write to "
+		gettext_noop("Multi-shard modifications such as UPDATE, DELETE, and "
-					 "shards on different nodes are prevented to avoid creating "
+					 "INSERT...SELECT are typically executed in parallel. If multiple "
-					 "undetectable distributed deadlocks when performed "
+					 "such commands run concurrently and affect the same rows, then "
-					 "concurrently."),
+					 "they are likely to deadlock. When enabled, this flag prevents "
+					 "multi-shard modifications from running concurrently when they "
+					 "affect the same shards in order to prevent deadlocks."),
 		&EnableDeadlockPrevention,
 		true,
 		PGC_USERSET,
-		GUC_NO_SHOW_ALL,
+		0,
 		NULL, NULL, NULL);
 
 	DefineCustomBoolVariable(

src/test/regress/expected/isolation_citus_dist_activity.out

@@ -30,16 +30,16 @@ step s3-view-worker:
 
 query          query_hostname query_hostport master_query_host_namemaster_query_host_portstate          wait_event_typewait_event     usename        datname        
 
-SELECT worker_apply_shard_ddl_command (105833, 'public', '
+SELECT worker_apply_shard_ddl_command (105961, 'public', '
     ALTER TABLE test_table ADD COLUMN x INT;
 ')localhost      57638          coordinator_host57636          idle in transactionClient         ClientRead     postgres       regression     
-SELECT worker_apply_shard_ddl_command (105832, 'public', '
+SELECT worker_apply_shard_ddl_command (105960, 'public', '
     ALTER TABLE test_table ADD COLUMN x INT;
 ')localhost      57637          coordinator_host57636          idle in transactionClient         ClientRead     postgres       regression     
-SELECT worker_apply_shard_ddl_command (105831, 'public', '
+SELECT worker_apply_shard_ddl_command (105959, 'public', '
     ALTER TABLE test_table ADD COLUMN x INT;
 ')localhost      57638          coordinator_host57636          idle in transactionClient         ClientRead     postgres       regression     
-SELECT worker_apply_shard_ddl_command (105830, 'public', '
+SELECT worker_apply_shard_ddl_command (105958, 'public', '
     ALTER TABLE test_table ADD COLUMN x INT;
 ')localhost      57637          coordinator_host57636          idle in transactionClient         ClientRead     postgres       regression     
 SELECT gid FROM pg_prepared_xacts WHERE gid LIKE 'citus\_0\_%'localhost      57638                         0              idle           Client         ClientRead     postgres       regression     
@@ -86,7 +86,7 @@ query          query_hostname query_hostport master_query_host_namemaster_query_
 
 SELECT gid FROM pg_prepared_xacts WHERE gid LIKE 'citus\_0\_%'localhost      57638                         0              idle           Client         ClientRead     postgres       regression     
 SELECT gid FROM pg_prepared_xacts WHERE gid LIKE 'citus\_0\_%'localhost      57637                         0              idle           Client         ClientRead     postgres       regression     
-INSERT INTO public.test_table_105836 (column1, column2) VALUES (100, 100)localhost      57637          coordinator_host57636          idle in transactionClient         ClientRead     postgres       regression     
+INSERT INTO public.test_table_105964 (column1, column2) VALUES (100, 100)localhost      57637          coordinator_host57636          idle in transactionClient         ClientRead     postgres       regression     
 step s2-rollback: 
 	ROLLBACK;
 
@@ -132,10 +132,10 @@ query          query_hostname query_hostport master_query_host_namemaster_query_
 
 SELECT gid FROM pg_prepared_xacts WHERE gid LIKE 'citus\_0\_%'localhost      57638                         0              idle           Client         ClientRead     postgres       regression     
 SELECT gid FROM pg_prepared_xacts WHERE gid LIKE 'citus\_0\_%'localhost      57637                         0              idle           Client         ClientRead     postgres       regression     
-COPY (SELECT count(*) AS count FROM test_table_105841 test_table WHERE true) TO STDOUTlocalhost      57638          coordinator_host57636          idle in transactionClient         ClientRead     postgres       regression     
+COPY (SELECT count(*) AS count FROM test_table_105969 test_table WHERE true) TO STDOUTlocalhost      57638          coordinator_host57636          idle in transactionClient         ClientRead     postgres       regression     
-COPY (SELECT count(*) AS count FROM test_table_105840 test_table WHERE true) TO STDOUTlocalhost      57637          coordinator_host57636          idle in transactionClient         ClientRead     postgres       regression     
+COPY (SELECT count(*) AS count FROM test_table_105968 test_table WHERE true) TO STDOUTlocalhost      57637          coordinator_host57636          idle in transactionClient         ClientRead     postgres       regression     
-COPY (SELECT count(*) AS count FROM test_table_105839 test_table WHERE true) TO STDOUTlocalhost      57638          coordinator_host57636          idle in transactionClient         ClientRead     postgres       regression     
+COPY (SELECT count(*) AS count FROM test_table_105967 test_table WHERE true) TO STDOUTlocalhost      57638          coordinator_host57636          idle in transactionClient         ClientRead     postgres       regression     
-COPY (SELECT count(*) AS count FROM test_table_105838 test_table WHERE true) TO STDOUTlocalhost      57637          coordinator_host57636          idle in transactionClient         ClientRead     postgres       regression     
+COPY (SELECT count(*) AS count FROM test_table_105966 test_table WHERE true) TO STDOUTlocalhost      57637          coordinator_host57636          idle in transactionClient         ClientRead     postgres       regression     
 step s2-rollback: 
 	ROLLBACK;
 
@@ -181,7 +181,7 @@ query          query_hostname query_hostport master_query_host_namemaster_query_
 
 SELECT gid FROM pg_prepared_xacts WHERE gid LIKE 'citus\_0\_%'localhost      57638                         0              idle           Client         ClientRead     postgres       regression     
 SELECT gid FROM pg_prepared_xacts WHERE gid LIKE 'citus\_0\_%'localhost      57637                         0              idle           Client         ClientRead     postgres       regression     
-SELECT count(*) AS count FROM public.test_table_105843 test_table WHERE (column1 OPERATOR(pg_catalog.=) 55)localhost      57638                         0              idle           Client         ClientRead     postgres       regression     
+SELECT count(*) AS count FROM public.test_table_105971 test_table WHERE (column1 OPERATOR(pg_catalog.=) 55)localhost      57638                         0              idle           Client         ClientRead     postgres       regression     
 COMMIT         localhost      57637                         0              idle           Client         ClientRead     postgres       regression     
 step s2-rollback: 
 	ROLLBACK;

src/test/regress/expected/isolation_citus_dist_activity_0.out

@@ -30,16 +30,16 @@ step s3-view-worker:
 
 query          query_hostname query_hostport master_query_host_namemaster_query_host_portstate          wait_event_typewait_event     usename        datname        
 
-SELECT worker_apply_shard_ddl_command (105297, 'public', '
+SELECT worker_apply_shard_ddl_command (105425, 'public', '
     ALTER TABLE test_table ADD COLUMN x INT;
 ')localhost      57638          coordinator_host57636          idle in transaction                              postgres       regression     
-SELECT worker_apply_shard_ddl_command (105296, 'public', '
+SELECT worker_apply_shard_ddl_command (105424, 'public', '
     ALTER TABLE test_table ADD COLUMN x INT;
 ')localhost      57637          coordinator_host57636          idle in transaction                              postgres       regression     
-SELECT worker_apply_shard_ddl_command (105295, 'public', '
+SELECT worker_apply_shard_ddl_command (105423, 'public', '
     ALTER TABLE test_table ADD COLUMN x INT;
 ')localhost      57638          coordinator_host57636          idle in transaction                              postgres       regression     
-SELECT worker_apply_shard_ddl_command (105294, 'public', '
+SELECT worker_apply_shard_ddl_command (105422, 'public', '
     ALTER TABLE test_table ADD COLUMN x INT;
 ')localhost      57637          coordinator_host57636          idle in transaction                              postgres       regression     
 SELECT gid FROM pg_prepared_xacts WHERE gid LIKE 'citus\_0\_%'localhost      57638                         0              idle                                         postgres       regression     
@@ -86,7 +86,7 @@ query          query_hostname query_hostport master_query_host_namemaster_query_
 
 SELECT gid FROM pg_prepared_xacts WHERE gid LIKE 'citus\_0\_%'localhost      57638                         0              idle                                         postgres       regression     
 SELECT gid FROM pg_prepared_xacts WHERE gid LIKE 'citus\_0\_%'localhost      57637                         0              idle                                         postgres       regression     
-INSERT INTO public.test_table_105300 (column1, column2) VALUES (100, 100)localhost      57637          coordinator_host57636          idle in transaction                              postgres       regression     
+INSERT INTO public.test_table_105428 (column1, column2) VALUES (100, 100)localhost      57637          coordinator_host57636          idle in transaction                              postgres       regression     
 step s2-rollback: 
 	ROLLBACK;
 
@@ -132,10 +132,10 @@ query          query_hostname query_hostport master_query_host_namemaster_query_
 
 SELECT gid FROM pg_prepared_xacts WHERE gid LIKE 'citus\_0\_%'localhost      57638                         0              idle                                         postgres       regression     
 SELECT gid FROM pg_prepared_xacts WHERE gid LIKE 'citus\_0\_%'localhost      57637                         0              idle                                         postgres       regression     
-COPY (SELECT count(*) AS count FROM test_table_105305 test_table WHERE true) TO STDOUTlocalhost      57638          coordinator_host57636          idle in transaction                              postgres       regression     
+COPY (SELECT count(*) AS count FROM test_table_105433 test_table WHERE true) TO STDOUTlocalhost      57638          coordinator_host57636          idle in transaction                              postgres       regression     
-COPY (SELECT count(*) AS count FROM test_table_105304 test_table WHERE true) TO STDOUTlocalhost      57637          coordinator_host57636          idle in transaction                              postgres       regression     
+COPY (SELECT count(*) AS count FROM test_table_105432 test_table WHERE true) TO STDOUTlocalhost      57637          coordinator_host57636          idle in transaction                              postgres       regression     
-COPY (SELECT count(*) AS count FROM test_table_105303 test_table WHERE true) TO STDOUTlocalhost      57638          coordinator_host57636          idle in transaction                              postgres       regression     
+COPY (SELECT count(*) AS count FROM test_table_105431 test_table WHERE true) TO STDOUTlocalhost      57638          coordinator_host57636          idle in transaction                              postgres       regression     
-COPY (SELECT count(*) AS count FROM test_table_105302 test_table WHERE true) TO STDOUTlocalhost      57637          coordinator_host57636          idle in transaction                              postgres       regression     
+COPY (SELECT count(*) AS count FROM test_table_105430 test_table WHERE true) TO STDOUTlocalhost      57637          coordinator_host57636          idle in transaction                              postgres       regression     
 step s2-rollback: 
 	ROLLBACK;
 
@@ -181,7 +181,7 @@ query          query_hostname query_hostport master_query_host_namemaster_query_
 
 SELECT gid FROM pg_prepared_xacts WHERE gid LIKE 'citus\_0\_%'localhost      57638                         0              idle                                         postgres       regression     
 SELECT gid FROM pg_prepared_xacts WHERE gid LIKE 'citus\_0\_%'localhost      57637                         0              idle                                         postgres       regression     
-SELECT count(*) AS count FROM public.test_table_105307 test_table WHERE (column1 OPERATOR(pg_catalog.=) 55)localhost      57638                         0              idle                                         postgres       regression     
+SELECT count(*) AS count FROM public.test_table_105435 test_table WHERE (column1 OPERATOR(pg_catalog.=) 55)localhost      57638                         0              idle                                         postgres       regression     
 COMMIT         localhost      57637                         0              idle                                         postgres       regression     
 step s2-rollback: 
 	ROLLBACK;

src/test/regress/expected/isolation_multi_shard_modify_vs_all.out

@@ -62,6 +62,53 @@ step s2-commit:
 	COMMIT;
 
 
+starting permutation: s1-begin s1-update_even_concurrently s2-begin s2-update_odd_concurrently s1-commit s2-commit
+step s1-begin: 
+    BEGIN;
+
+step s1-update_even_concurrently: 
+	SET citus.enable_deadlock_prevention TO off;
+	UPDATE users_test_table SET value_1 = 3 WHERE user_id % 2 = 0;
+	SET citus.enable_deadlock_prevention TO on;
+
+step s2-begin: 
+	BEGIN;
+
+step s2-update_odd_concurrently: 
+	SET citus.enable_deadlock_prevention = off;
+	UPDATE users_test_table SET value_1 = 3 WHERE user_id % 2 = 1;
+	SET citus.enable_deadlock_prevention TO on;
+
+step s1-commit: 
+    COMMIT;
+
+step s2-commit: 
+	COMMIT;
+
+
+starting permutation: s1-begin s1-update_even_concurrently s2-begin s2-update_value_1_of_4_or_6_to_4 s1-commit s2-commit
+step s1-begin: 
+    BEGIN;
+
+step s1-update_even_concurrently: 
+	SET citus.enable_deadlock_prevention TO off;
+	UPDATE users_test_table SET value_1 = 3 WHERE user_id % 2 = 0;
+	SET citus.enable_deadlock_prevention TO on;
+
+step s2-begin: 
+	BEGIN;
+
+step s2-update_value_1_of_4_or_6_to_4: 
+	UPDATE users_test_table SET value_1 = 4 WHERE user_id = 4 or user_id = 6;
+ <waiting ...>
+step s1-commit: 
+    COMMIT;
+
+step s2-update_value_1_of_4_or_6_to_4: <... completed>
+step s2-commit: 
+	COMMIT;
+
+
 starting permutation: s1-begin s1-update_value_1_of_1_or_3_to_5 s2-begin s2-update_value_1_of_4_or_6_to_4 s1-commit s2-commit s2-select
 step s1-begin: 
     BEGIN;

src/test/regress/specs/isolation_multi_shard_modify_vs_all.spec

@@ -55,6 +55,13 @@ step "s1-update_all_value_1"
 	UPDATE users_test_table SET value_1 = 3;
 }
 
+step "s1-update_even_concurrently"
+{
+	SET citus.enable_deadlock_prevention TO off;
+	UPDATE users_test_table SET value_1 = 3 WHERE user_id % 2 = 0;
+	SET citus.enable_deadlock_prevention TO on;
+}
+
 step "s1-update_value_1_of_1_or_3_to_5"
 {
 	UPDATE users_test_table SET value_1 = 5 WHERE user_id = 1 or user_id = 3;
@@ -107,6 +114,13 @@ step "s2-update_all_value_1"
 	UPDATE users_test_table SET value_1 = 6;
 }
 
+step "s2-update_odd_concurrently"
+{
+	SET citus.enable_deadlock_prevention = off;
+	UPDATE users_test_table SET value_1 = 3 WHERE user_id % 2 = 1;
+	SET citus.enable_deadlock_prevention TO on;
+}
+
 step "s2-update_value_1_of_1_or_3_to_8"
 {
 	UPDATE users_test_table SET value_1 = 8 WHERE user_id = 1 or user_id = 3;
@@ -125,10 +139,19 @@ step "s2-commit"
 # test with parallel connections
 permutation "s1-begin" "s1-update_all_value_1" "s2-begin" "s2-select" "s1-commit" "s2-select" "s2-commit"
 permutation "s1-begin" "s1-update_all_value_1" "s2-begin" "s2-update_all_value_1" "s1-commit" "s2-commit"
+
+# test without deadlock prevention (first does not conflict, second does)
+permutation "s1-begin" "s1-update_even_concurrently" "s2-begin" "s2-update_odd_concurrently" "s1-commit" "s2-commit"
+permutation "s1-begin" "s1-update_even_concurrently" "s2-begin" "s2-update_value_1_of_4_or_6_to_4" "s1-commit" "s2-commit"
+
+# test with shard pruning (should not conflict)
 permutation "s1-begin" "s1-update_value_1_of_1_or_3_to_5" "s2-begin" "s2-update_value_1_of_4_or_6_to_4" "s1-commit" "s2-commit" "s2-select" 
 permutation "s1-begin" "s1-update_value_1_of_1_or_3_to_5" "s2-begin" "s2-update_value_1_of_1_or_3_to_8" "s1-commit" "s2-commit" "s2-select" 
+
+# test with inserts
 permutation "s1-begin" "s1-update_all_value_1" "s2-begin" "s2-insert-to-table" "s1-commit" "s2-commit" "s2-select" 
 permutation "s1-begin" "s1-update_all_value_1" "s2-begin" "s2-insert-into-select" "s1-commit" "s2-commit" "s2-select"
+
 # multi-shard update affecting the same rows
 permutation "s1-begin" "s2-begin" "s1-update_value_1_of_1_or_3_to_5" "s2-update_value_1_of_1_or_3_to_8" "s1-commit" "s2-commit"
 # multi-shard update affecting the different rows