From 08ed6d8269fcbb58044be1773acfbafdcdb95791 Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Wed, 9 Aug 2017 11:46:11 +0200 Subject: [PATCH] Prevent pg_dist_node changes during master_create_empty_shard --- .../master/master_stage_protocol.c | 7 ++ ...lation_create_table_vs_add_remove_node.out | 99 ++++++++++++++++++- ...ation_create_table_vs_add_remove_node.spec | 14 ++- 3 files changed, 114 insertions(+), 6 deletions(-) diff --git a/src/backend/distributed/master/master_stage_protocol.c b/src/backend/distributed/master/master_stage_protocol.c index ac60b5f4d..60f39103a 100644 --- a/src/backend/distributed/master/master_stage_protocol.c +++ b/src/backend/distributed/master/master_stage_protocol.c @@ -44,6 +44,7 @@ #include "distributed/transaction_management.h" #include "distributed/worker_manager.h" #include "distributed/worker_protocol.h" +#include "storage/lmgr.h" #include "utils/builtins.h" #include "utils/fmgroids.h" #include "utils/inval.h" @@ -94,6 +95,12 @@ master_create_empty_shard(PG_FUNCTION_ARGS) EnsureTablePermissions(relationId, ACL_INSERT); CheckDistributedTable(relationId); + /* don't allow the table to be dropped */ + LockRelationOid(relationId, AccessShareLock); + + /* don't allow concurrent node list changes that require an exclusive lock */ + LockRelationOid(DistNodeRelationId(), RowShareLock); + /* * We check whether the table is a foreign table or not. If it is, we set * storage type as foreign also. Only exception is if foreign table is a diff --git a/src/test/regress/expected/isolation_create_table_vs_add_remove_node.out b/src/test/regress/expected/isolation_create_table_vs_add_remove_node.out index 03c26669e..023e320c6 100644 --- a/src/test/regress/expected/isolation_create_table_vs_add_remove_node.out +++ b/src/test/regress/expected/isolation_create_table_vs_add_remove_node.out @@ -364,7 +364,7 @@ master_remove_node -starting permutation: s1-add-node-2 s1-begin s2-create-table-2 s1-remove-node-2 s1-commit s2-select +starting permutation: s1-add-node-2 s2-begin s2-create-table-2 s1-remove-node-2 s2-commit s2-select node_name node_port localhost 57637 @@ -374,7 +374,7 @@ step s1-add-node-2: ?column? 1 -step s1-begin: +step s2-begin: BEGIN; step s2-create-table-2: @@ -388,11 +388,12 @@ create_distributed_table step s1-remove-node-2: SELECT * FROM master_remove_node('localhost', 57638); - -ERROR: you cannot remove the primary node of a node group which has shard placements -step s1-commit: + +step s2-commit: COMMIT; +step s1-remove-node-2: <... completed> +error in steps s2-commit s1-remove-node-2: ERROR: you cannot remove the primary node of a node group which has shard placements step s2-select: SELECT * FROM dist_table; @@ -402,3 +403,91 @@ master_remove_node + +starting permutation: s1-add-node-2 s1-begin s1-remove-node-2 s2-create-append-table s1-commit s2-select +node_name node_port + +localhost 57637 +step s1-add-node-2: + SELECT 1 FROM master_add_node('localhost', 57638); + +?column? + +1 +step s1-begin: + BEGIN; + +step s1-remove-node-2: + SELECT * FROM master_remove_node('localhost', 57638); + +master_remove_node + + +step s2-create-append-table: + SET citus.shard_replication_factor TO 1; + CREATE TABLE dist_table (x int, y int); + SELECT create_distributed_table('dist_table', 'x', 'append'); + SELECT 1 FROM master_create_empty_shard('dist_table'); + +step s1-commit: + COMMIT; + +step s2-create-append-table: <... completed> +create_distributed_table + + +?column? + +1 +step s2-select: + SELECT * FROM dist_table; + +x y + +master_remove_node + + + +starting permutation: s1-add-node-2 s2-begin s2-create-append-table s1-remove-node-2 s2-commit s2-select +node_name node_port + +localhost 57637 +step s1-add-node-2: + SELECT 1 FROM master_add_node('localhost', 57638); + +?column? + +1 +step s2-begin: + BEGIN; + +step s2-create-append-table: + SET citus.shard_replication_factor TO 1; + CREATE TABLE dist_table (x int, y int); + SELECT create_distributed_table('dist_table', 'x', 'append'); + SELECT 1 FROM master_create_empty_shard('dist_table'); + +create_distributed_table + + +?column? + +1 +step s1-remove-node-2: + SELECT * FROM master_remove_node('localhost', 57638); + +step s2-commit: + COMMIT; + +step s1-remove-node-2: <... completed> +master_remove_node + + +step s2-select: + SELECT * FROM dist_table; + +x y + +master_remove_node + + diff --git a/src/test/regress/specs/isolation_create_table_vs_add_remove_node.spec b/src/test/regress/specs/isolation_create_table_vs_add_remove_node.spec index 749d5131f..4986beee3 100644 --- a/src/test/regress/specs/isolation_create_table_vs_add_remove_node.spec +++ b/src/test/regress/specs/isolation_create_table_vs_add_remove_node.spec @@ -83,6 +83,14 @@ step "s2-create-table-2" SELECT create_distributed_table('dist_table', 'x'); } +step "s2-create-append-table" +{ + SET citus.shard_replication_factor TO 1; + CREATE TABLE dist_table (x int, y int); + SELECT create_distributed_table('dist_table', 'x', 'append'); + SELECT 1 FROM master_create_empty_shard('dist_table'); +} + step "s2-select" { SELECT * FROM dist_table; @@ -105,4 +113,8 @@ permutation "s1-add-node-2" "s2-begin" "s2-create-table-1" "s1-remove-node-2" "s # session 1 removes a node, session 2 creates a distributed table with replication factor 2, should throw a sane error permutation "s1-add-node-2" "s1-begin" "s1-remove-node-2" "s2-create-table-2" "s1-commit" "s2-select" -permutation "s1-add-node-2" "s1-begin" "s2-create-table-2" "s1-remove-node-2" "s1-commit" "s2-select" +permutation "s1-add-node-2" "s2-begin" "s2-create-table-2" "s1-remove-node-2" "s2-commit" "s2-select" + +# session 1 removes a node, session 2 creates a shard in an append-distributed table +permutation "s1-add-node-2" "s1-begin" "s1-remove-node-2" "s2-create-append-table" "s1-commit" "s2-select" +permutation "s1-add-node-2" "s2-begin" "s2-create-append-table" "s1-remove-node-2" "s2-commit" "s2-select"