diff --git a/src/backend/distributed/master/master_delete_protocol.c b/src/backend/distributed/master/master_delete_protocol.c index 0d3525d84..ed8418aa4 100644 --- a/src/backend/distributed/master/master_delete_protocol.c +++ b/src/backend/distributed/master/master_delete_protocol.c @@ -51,6 +51,7 @@ #include "optimizer/predtest.h" #include "optimizer/restrictinfo.h" #include "storage/lock.h" +#include "storage/lmgr.h" #include "tcop/tcopprot.h" #include "utils/array.h" #include "utils/builtins.h" @@ -105,9 +106,7 @@ master_apply_delete_command(PG_FUNCTION_ARGS) Node *queryTreeNode = NULL; DeleteStmt *deleteStatement = NULL; int droppedShardCount = 0; - LOCKTAG lockTag; - bool sessionLock = false; - bool dontWait = false; + LOCKMODE lockMode = 0; char partitionMethod = 0; bool failOK = false; @@ -124,7 +123,15 @@ master_apply_delete_command(PG_FUNCTION_ARGS) schemaName = deleteStatement->relation->schemaname; relationName = deleteStatement->relation->relname; - relationId = RangeVarGetRelid(deleteStatement->relation, NoLock, failOK); + + /* + * We take an exclusive lock while dropping shards to prevent concurrent + * writes. We don't want to block SELECTs, which means queries might fail + * if they access a shard that has just been dropped. + */ + lockMode = ExclusiveLock; + + relationId = RangeVarGetRelid(deleteStatement->relation, lockMode, failOK); /* schema-prefix if it is not specified already */ if (schemaName == NULL) @@ -166,10 +173,6 @@ master_apply_delete_command(PG_FUNCTION_ARGS) CheckDeleteCriteria(deleteCriteria); CheckPartitionColumn(relationId, deleteCriteria); - /* acquire lock */ - SET_LOCKTAG_ADVISORY(lockTag, MyDatabaseId, relationId, 0, 0); - LockAcquire(&lockTag, ExclusiveLock, sessionLock, dontWait); - shardIntervalList = LoadShardIntervalList(relationId); /* drop all shards if where clause is not present */ @@ -214,6 +217,14 @@ master_drop_all_shards(PG_FUNCTION_ARGS) CheckTableSchemaNameForDrop(relationId, &schemaName, &relationName); + /* + * master_drop_all_shards is typically called from the DROP TABLE trigger, + * but could be called by a user directly. Make sure we have an + * AccessExlusiveLock to prevent any other commands from running on this table + * concurrently. + */ + LockRelationOid(relationId, AccessExclusiveLock); + shardIntervalList = LoadShardIntervalList(relationId); droppedShardCount = DropShards(relationId, schemaName, relationName, shardIntervalList); diff --git a/src/test/regress/expected/isolation_drop_shards.out b/src/test/regress/expected/isolation_drop_shards.out new file mode 100644 index 000000000..d007f502c --- /dev/null +++ b/src/test/regress/expected/isolation_drop_shards.out @@ -0,0 +1,242 @@ +Parsed test spec with 2 sessions + +starting permutation: s1-begin s1-drop-all-shards s2-truncate s1-commit +?column? + +1 +step s1-begin: + BEGIN; + +step s1-drop-all-shards: + SELECT master_drop_all_shards('append_table', 'public', 'append_table'); + +master_drop_all_shards + +16 +step s2-truncate: + TRUNCATE append_table; + +step s1-commit: + COMMIT; + +step s2-truncate: <... completed> + +starting permutation: s1-begin s1-drop-all-shards s2-apply-delete-command s1-commit +?column? + +1 +step s1-begin: + BEGIN; + +step s1-drop-all-shards: + SELECT master_drop_all_shards('append_table', 'public', 'append_table'); + +master_drop_all_shards + +16 +step s2-apply-delete-command: + SELECT master_apply_delete_command('DELETE FROM append_table'); + +step s1-commit: + COMMIT; + +step s2-apply-delete-command: <... completed> +master_apply_delete_command + +0 + +starting permutation: s1-begin s1-drop-all-shards s2-drop-all-shards s1-commit +?column? + +1 +step s1-begin: + BEGIN; + +step s1-drop-all-shards: + SELECT master_drop_all_shards('append_table', 'public', 'append_table'); + +master_drop_all_shards + +16 +step s2-drop-all-shards: + SELECT master_drop_all_shards('append_table', 'public', 'append_table'); + +step s1-commit: + COMMIT; + +step s2-drop-all-shards: <... completed> +master_drop_all_shards + +0 + +starting permutation: s1-begin s1-drop-all-shards s2-select s1-commit +?column? + +1 +step s1-begin: + BEGIN; + +step s1-drop-all-shards: + SELECT master_drop_all_shards('append_table', 'public', 'append_table'); + +master_drop_all_shards + +16 +step s2-select: + SELECT * FROM append_table; + +step s1-commit: + COMMIT; + +step s2-select: <... completed> +test_id data + + +starting permutation: s1-begin s1-apply-delete-command s2-truncate s1-commit +?column? + +1 +step s1-begin: + BEGIN; + +step s1-apply-delete-command: + SELECT master_apply_delete_command('DELETE FROM append_table'); + +master_apply_delete_command + +16 +step s2-truncate: + TRUNCATE append_table; + +step s1-commit: + COMMIT; + +step s2-truncate: <... completed> + +starting permutation: s1-begin s1-apply-delete-command s2-apply-delete-command s1-commit +?column? + +1 +step s1-begin: + BEGIN; + +step s1-apply-delete-command: + SELECT master_apply_delete_command('DELETE FROM append_table'); + +master_apply_delete_command + +16 +step s2-apply-delete-command: + SELECT master_apply_delete_command('DELETE FROM append_table'); + +step s1-commit: + COMMIT; + +step s2-apply-delete-command: <... completed> +master_apply_delete_command + +0 + +starting permutation: s1-begin s1-apply-delete-command s2-drop-all-shards s1-commit +?column? + +1 +step s1-begin: + BEGIN; + +step s1-apply-delete-command: + SELECT master_apply_delete_command('DELETE FROM append_table'); + +master_apply_delete_command + +16 +step s2-drop-all-shards: + SELECT master_drop_all_shards('append_table', 'public', 'append_table'); + +step s1-commit: + COMMIT; + +step s2-drop-all-shards: <... completed> +master_drop_all_shards + +0 + +starting permutation: s1-begin s1-truncate s2-truncate s1-commit +?column? + +1 +step s1-begin: + BEGIN; + +step s1-truncate: + TRUNCATE append_table; + +step s2-truncate: + TRUNCATE append_table; + +step s1-commit: + COMMIT; + +step s2-truncate: <... completed> + +starting permutation: s1-begin s1-truncate s2-apply-delete-command s1-commit +?column? + +1 +step s1-begin: + BEGIN; + +step s1-truncate: + TRUNCATE append_table; + +step s2-apply-delete-command: + SELECT master_apply_delete_command('DELETE FROM append_table'); + +step s1-commit: + COMMIT; + +step s2-apply-delete-command: <... completed> +master_apply_delete_command + +0 + +starting permutation: s1-begin s1-truncate s2-drop-all-shards s1-commit +?column? + +1 +step s1-begin: + BEGIN; + +step s1-truncate: + TRUNCATE append_table; + +step s2-drop-all-shards: + SELECT master_drop_all_shards('append_table', 'public', 'append_table'); + +step s1-commit: + COMMIT; + +step s2-drop-all-shards: <... completed> +master_drop_all_shards + +0 + +starting permutation: s1-begin s1-truncate s2-select s1-commit +?column? + +1 +step s1-begin: + BEGIN; + +step s1-truncate: + TRUNCATE append_table; + +step s2-select: + SELECT * FROM append_table; + +step s1-commit: + COMMIT; + +step s2-select: <... completed> +test_id data + diff --git a/src/test/regress/isolation_schedule b/src/test/regress/isolation_schedule index 51c9d4f97..0dcf6e45c 100644 --- a/src/test/regress/isolation_schedule +++ b/src/test/regress/isolation_schedule @@ -1,3 +1,4 @@ test: isolation_cluster_management test: isolation_dml_vs_repair test: isolation_concurrent_dml +test: isolation_drop_shards diff --git a/src/test/regress/specs/isolation_drop_shards.spec b/src/test/regress/specs/isolation_drop_shards.spec new file mode 100644 index 000000000..fd9fce2d7 --- /dev/null +++ b/src/test/regress/specs/isolation_drop_shards.spec @@ -0,0 +1,80 @@ +setup +{ + CREATE TABLE append_table (test_id integer NOT NULL, data text); + SELECT create_distributed_table('append_table', 'test_id', 'append'); + + SELECT 1 FROM ( + SELECT min(master_create_empty_shard('append_table')) FROM generate_series(1,16) + ) a; +} + +teardown +{ + DROP TABLE append_table; +} + +session "s1" + +step "s1-begin" +{ + BEGIN; +} + +step "s1-truncate" +{ + TRUNCATE append_table; +} + +step "s1-apply-delete-command" +{ + SELECT master_apply_delete_command('DELETE FROM append_table'); +} + +step "s1-drop-all-shards" +{ + SELECT master_drop_all_shards('append_table', 'public', 'append_table'); +} + +step "s1-commit" +{ + COMMIT; +} + +session "s2" + +step "s2-truncate" +{ + TRUNCATE append_table; +} + +step "s2-apply-delete-command" +{ + SELECT master_apply_delete_command('DELETE FROM append_table'); +} + +step "s2-drop-all-shards" +{ + SELECT master_drop_all_shards('append_table', 'public', 'append_table'); +} + +step "s2-select" +{ + SELECT * FROM append_table; +} + +permutation "s1-begin" "s1-drop-all-shards" "s2-truncate" "s1-commit" +permutation "s1-begin" "s1-drop-all-shards" "s2-apply-delete-command" "s1-commit" +permutation "s1-begin" "s1-drop-all-shards" "s2-drop-all-shards" "s1-commit" +permutation "s1-begin" "s1-drop-all-shards" "s2-select" "s1-commit" + +# We can't verify master_apply_delete_command + SELECT since it blocks on the +# the workers, but this is not visible on the master, meaning the isolation +# test cannot proceed. +permutation "s1-begin" "s1-apply-delete-command" "s2-truncate" "s1-commit" +permutation "s1-begin" "s1-apply-delete-command" "s2-apply-delete-command" "s1-commit" +permutation "s1-begin" "s1-apply-delete-command" "s2-drop-all-shards" "s1-commit" + +permutation "s1-begin" "s1-truncate" "s2-truncate" "s1-commit" +permutation "s1-begin" "s1-truncate" "s2-apply-delete-command" "s1-commit" +permutation "s1-begin" "s1-truncate" "s2-drop-all-shards" "s1-commit" +permutation "s1-begin" "s1-truncate" "s2-select" "s1-commit"