From 979b48e1a7763fbafc2d600938994bdecf63f34a Mon Sep 17 00:00:00 2001 From: Hanefi Onaldi Date: Tue, 20 Jun 2023 15:49:15 +0300 Subject: [PATCH] Disallow index DDL on worker if coord not in metadata --- .../distributed/commands/utility_hook.c | 14 +++ .../expected/isolation_index_vs_all_on_mx.out | 112 ++++++------------ .../spec/isolation_index_vs_all_on_mx.spec | 23 +--- 3 files changed, 53 insertions(+), 96 deletions(-) diff --git a/src/backend/distributed/commands/utility_hook.c b/src/backend/distributed/commands/utility_hook.c index aba1c5db1..e274731b8 100644 --- a/src/backend/distributed/commands/utility_hook.c +++ b/src/backend/distributed/commands/utility_hook.c @@ -1126,6 +1126,20 @@ ExecuteDistributedDDLJob(DDLJob *ddlJob) EnsureCoordinator(); } + if (!IsCoordinator() && !CoordinatorAddedAsWorkerNode() && + !EnableAcquiringUnsafeLockFromWorkers) + { + ereport(ERROR, + (errmsg( + "Cannot execute DDL command from a worker node since the " + "coordinator is not in the metadata."), + errhint( + "Either run this command on the coordinator or add the coordinator " + "in the metadata by using: SELECT citus_set_coordinator_host('', );\n" + "Alternatively, though it is not recommended, you can allow this command by running: " + "SET citus.allow_unsafe_locks_from_workers TO 'on';"))); + } + ObjectAddress targetObjectAddress = ddlJob->targetObjectAddress; if (OidIsValid(targetObjectAddress.classId)) diff --git a/src/test/regress/expected/isolation_index_vs_all_on_mx.out b/src/test/regress/expected/isolation_index_vs_all_on_mx.out index 9d02196cc..7b819ac29 100644 --- a/src/test/regress/expected/isolation_index_vs_all_on_mx.out +++ b/src/test/regress/expected/isolation_index_vs_all_on_mx.out @@ -1,6 +1,11 @@ Parsed test spec with 4 sessions starting permutation: w1-start-session-level-connection w1-begin-on-worker w2-start-session-level-connection w2-begin-on-worker w1-create-named-index w2-create-named-index w1-commit-worker w2-commit-worker w1-stop-connection w2-stop-connection coord-print-index-count +citus_set_coordinator_host +--------------------------------------------------------------------- + +(1 row) + step w1-start-session-level-connection: SELECT start_session_level_connection_to_node('localhost', 57637); @@ -97,8 +102,18 @@ result 2 (4 rows) +citus_remove_node +--------------------------------------------------------------------- + +(1 row) + starting permutation: w1-start-session-level-connection w1-begin-on-worker w2-start-session-level-connection w2-begin-on-worker w1-create-unnamed-index w2-create-unnamed-index w1-commit-worker w1-stop-connection w2-stop-connection coord-print-index-count +citus_set_coordinator_host +--------------------------------------------------------------------- + +(1 row) + step w1-start-session-level-connection: SELECT start_session_level_connection_to_node('localhost', 57637); @@ -184,77 +199,18 @@ result 1 (4 rows) - -starting permutation: w1-start-session-level-connection w2-start-session-level-connection w1-create-index-concurrently w2-create-index-concurrently w1-empty w1-stop-connection w2-stop-connection coord-print-index-count -step w1-start-session-level-connection: - SELECT start_session_level_connection_to_node('localhost', 57637); - -start_session_level_connection_to_node +citus_remove_node --------------------------------------------------------------------- (1 row) -step w2-start-session-level-connection: - SELECT start_session_level_connection_to_node('localhost', 57638); - -start_session_level_connection_to_node ---------------------------------------------------------------------- - -(1 row) - -step w1-create-index-concurrently: - SELECT run_commands_on_session_level_connection_to_node('CREATE INDEX CONCURRENTLY dist_table_index ON dist_table(id)'); - -step w2-create-index-concurrently: - SELECT run_commands_on_session_level_connection_to_node('CREATE INDEX CONCURRENTLY dist_table_index_2 ON dist_table(id)'); - -step w1-create-index-concurrently: <... completed> -run_commands_on_session_level_connection_to_node ---------------------------------------------------------------------- - -(1 row) - -step w1-empty: -step w2-create-index-concurrently: <... completed> -run_commands_on_session_level_connection_to_node ---------------------------------------------------------------------- - -(1 row) - -step w1-stop-connection: - SELECT stop_session_level_connection_to_node(); - -stop_session_level_connection_to_node ---------------------------------------------------------------------- - -(1 row) - -step w2-stop-connection: - SELECT stop_session_level_connection_to_node(); - -stop_session_level_connection_to_node ---------------------------------------------------------------------- - -(1 row) - -step coord-print-index-count: - SELECT - result - FROM - run_command_on_placements('dist_table', 'select count(*) from pg_indexes WHERE tablename = ''%s''') - ORDER BY - nodeport; - -result ---------------------------------------------------------------------- - 2 - 2 - 2 - 2 -(4 rows) - starting permutation: w1-start-session-level-connection w1-create-named-index w1-begin-on-worker w1-delete coord-begin coord-short-statement-timeout coord-take-lock w1-reindex deadlock-checker-call coord-rollback w1-commit-worker w1-stop-connection +citus_set_coordinator_host +--------------------------------------------------------------------- + +(1 row) + step w1-start-session-level-connection: SELECT start_session_level_connection_to_node('localhost', 57637); @@ -298,22 +254,23 @@ step coord-take-lock: step w1-reindex: SELECT run_commands_on_session_level_connection_to_node('REINDEX INDEX dist_table_index'); + +step deadlock-checker-call: + SELECT check_distributed_deadlocks(); +check_distributed_deadlocks +--------------------------------------------------------------------- +t +(1 row) + +step coord-take-lock: <... completed> +ERROR: canceling the transaction since it was involved in a distributed deadlock +step w1-reindex: <... completed> run_commands_on_session_level_connection_to_node --------------------------------------------------------------------- (1 row) -step deadlock-checker-call: - SELECT check_distributed_deadlocks(); - -check_distributed_deadlocks ---------------------------------------------------------------------- -f -(1 row) - -step coord-take-lock: <... completed> -ERROR: canceling statement due to statement timeout step coord-rollback: ROLLBACK; @@ -333,3 +290,8 @@ stop_session_level_connection_to_node (1 row) +citus_remove_node +--------------------------------------------------------------------- + +(1 row) + diff --git a/src/test/regress/spec/isolation_index_vs_all_on_mx.spec b/src/test/regress/spec/isolation_index_vs_all_on_mx.spec index ece33e6ea..f33b38528 100644 --- a/src/test/regress/spec/isolation_index_vs_all_on_mx.spec +++ b/src/test/regress/spec/isolation_index_vs_all_on_mx.spec @@ -5,11 +5,13 @@ setup CREATE TABLE dist_table(id int, data int); SELECT create_distributed_table('dist_table', 'id'); COPY dist_table FROM PROGRAM 'echo 1, 10 && echo 2, 20 && echo 3, 30 && echo 4, 40 && echo 5, 50' WITH CSV; + SELECT citus_set_coordinator_host('localhost', 57636); } teardown { DROP TABLE IF EXISTS dist_table CASCADE; + SELECT citus_remove_node('localhost', 57636); } session "w1" @@ -34,14 +36,6 @@ step "w1-create-unnamed-index" SELECT run_commands_on_session_level_connection_to_node('CREATE INDEX ON dist_table(id,data)'); } -step "w1-create-index-concurrently" -{ - SELECT run_commands_on_session_level_connection_to_node('CREATE INDEX CONCURRENTLY dist_table_index ON dist_table(id)'); -} - -// an empty step to have consistent output for CONCURRENTLY -step "w1-empty" {} - step "w1-reindex" { SELECT run_commands_on_session_level_connection_to_node('REINDEX INDEX dist_table_index'); @@ -85,11 +79,6 @@ step "w2-create-unnamed-index" SELECT run_commands_on_session_level_connection_to_node('CREATE INDEX ON dist_table(id,data)'); } -step "w2-create-index-concurrently" -{ - SELECT run_commands_on_session_level_connection_to_node('CREATE INDEX CONCURRENTLY dist_table_index_2 ON dist_table(id)'); -} - step "w2-commit-worker" { SELECT run_commands_on_session_level_connection_to_node('COMMIT'); @@ -153,14 +142,6 @@ permutation "w1-start-session-level-connection" "w1-begin-on-worker" // open tra "w1-stop-connection" "w2-stop-connection" // close connections to workers "coord-print-index-count" // show indexes on coordinator -permutation "w1-start-session-level-connection" // start session on worker 1 - "w2-start-session-level-connection" // start session on worker 2 - "w1-create-index-concurrently"(*) // create both indexes with concurrently option - "w2-create-index-concurrently"(*) - "w1-empty" // empty steps to have consistent output for CONCURRENTLY - "w1-stop-connection" "w2-stop-connection" // close connections to workers - "coord-print-index-count" // show indexes on coordinator - // the following permutation is expected to fail with a distributed deadlock. However, we do not detect the deadlock, and get blocked until statement_timeout. permutation "w1-start-session-level-connection" // start session on worker 1 only "w1-create-named-index" // create index on worker 1