From aff37cf1bc7a1363d9902885bb7d085a35359508 Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Mon, 26 Nov 2018 14:07:06 +0100 Subject: [PATCH 1/2] Control multi-shard modify locks with enable_deadlock_prevention --- .../executor/multi_router_executor.c | 14 +++++- src/backend/distributed/shared_library_init.c | 14 +++--- .../isolation_multi_shard_modify_vs_all.out | 47 +++++++++++++++++++ .../isolation_multi_shard_modify_vs_all.spec | 25 +++++++++- 4 files changed, 92 insertions(+), 8 deletions(-) diff --git a/src/backend/distributed/executor/multi_router_executor.c b/src/backend/distributed/executor/multi_router_executor.c index 19081a0b8..3a2256b28 100644 --- a/src/backend/distributed/executor/multi_router_executor.c +++ b/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 { diff --git a/src/backend/distributed/shared_library_init.c b/src/backend/distributed/shared_library_init.c index c490e954b..34e8fcc59 100644 --- a/src/backend/distributed/shared_library_init.c +++ b/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("When enabled, consecutive DML statements that write to " - "shards on different nodes are prevented to avoid creating " - "undetectable distributed deadlocks when performed " - "concurrently."), + gettext_noop("Avoids deadlocks by preventing concurrent multi-shard commands"), + gettext_noop("Multi-shard modifications such as UPDATE, DELETE, and " + "INSERT...SELECT are typically executed in parallel. If multiple " + "such commands run concurrently and affect the same rows, then " + "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( diff --git a/src/test/regress/expected/isolation_multi_shard_modify_vs_all.out b/src/test/regress/expected/isolation_multi_shard_modify_vs_all.out index 3c024c699..d82e297eb 100644 --- a/src/test/regress/expected/isolation_multi_shard_modify_vs_all.out +++ b/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; + +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; diff --git a/src/test/regress/specs/isolation_multi_shard_modify_vs_all.spec b/src/test/regress/specs/isolation_multi_shard_modify_vs_all.spec index 982776f97..c4f5612c2 100644 --- a/src/test/regress/specs/isolation_multi_shard_modify_vs_all.spec +++ b/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 @@ -143,4 +166,4 @@ permutation "s1-begin" "s1-change_connection_mode_to_sequential" "s1-update_valu # multi-shard update affecting the same rows permutation "s1-begin" "s2-begin" "s1-change_connection_mode_to_sequential" "s2-change_connection_mode_to_sequential" "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 -permutation "s1-begin" "s2-begin" "s1-change_connection_mode_to_sequential" "s2-change_connection_mode_to_sequential" "s2-update_value_1_of_1_or_3_to_8" "s1-update_value_1_of_2_or_4_to_5" "s1-commit" "s2-commit" \ No newline at end of file +permutation "s1-begin" "s2-begin" "s1-change_connection_mode_to_sequential" "s2-change_connection_mode_to_sequential" "s2-update_value_1_of_1_or_3_to_8" "s1-update_value_1_of_2_or_4_to_5" "s1-commit" "s2-commit" From 0393910c65db69e56f210719d1f722b69f2dc5db Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Mon, 26 Nov 2018 17:06:43 +0100 Subject: [PATCH 2/2] Shard IDs in isolation_citus_dist_stat_activity output changed --- .../isolation_citus_dist_activity.out | 20 +++++++++---------- .../isolation_citus_dist_activity_0.out | 20 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/test/regress/expected/isolation_citus_dist_activity.out b/src/test/regress/expected/isolation_citus_dist_activity.out index 1c83fb29a..ea5b4bc17 100644 --- a/src/test/regress/expected/isolation_citus_dist_activity.out +++ b/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_105840 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_105838 test_table WHERE true) TO STDOUTlocalhost 57637 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_105968 test_table WHERE true) TO STDOUTlocalhost 57637 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_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; diff --git a/src/test/regress/expected/isolation_citus_dist_activity_0.out b/src/test/regress/expected/isolation_citus_dist_activity_0.out index ad4c30f9e..e95fa871f 100644 --- a/src/test/regress/expected/isolation_citus_dist_activity_0.out +++ b/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_105304 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_105302 test_table WHERE true) TO STDOUTlocalhost 57637 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_105432 test_table WHERE true) TO STDOUTlocalhost 57637 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_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;