Control multi-shard modify locks with enable_deadlock_prevention

src/backend/distributed/executor/multi_router_executor.c

@@ -369,11 +369,23 @@ 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.
 			 */
 
+			if (EnableDeadlockPrevention)
+			{
 				lockMode = ShareUpdateExclusiveLock;
 			}
 			else
+			{
+				lockMode = RowExclusiveLock;
+			}
+		}
+		else
 		{
 			/*
 			 * When there is replication, prevent all concurrent writes to the same

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_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