diff --git a/src/backend/distributed/master/citus_create_restore_point.c b/src/backend/distributed/master/citus_create_restore_point.c index 7c9c0015a..4f4c2d7e0 100644 --- a/src/backend/distributed/master/citus_create_restore_point.c +++ b/src/backend/distributed/master/citus_create_restore_point.c @@ -31,7 +31,7 @@ /* local functions forward declarations */ static List * OpenConnectionsToAllNodes(void); -static void BlockAllDistributedWrites(void); +static void BlockDistributedTransactions(void); static void CreateRemoteRestorePoints(char *restoreName, List *connectionList); @@ -94,7 +94,7 @@ citus_create_restore_point(PG_FUNCTION_ARGS) RemoteTransactionListBegin(connectionList); /* DANGER: finish as quickly as possible after this */ - BlockAllDistributedWrites(); + BlockDistributedTransactions(); /* do local restore point first to bail out early if something goes wrong */ localRestorePoint = XLogRestorePoint(restoreNameString); @@ -139,26 +139,16 @@ OpenConnectionsToAllNodes(void) /* - * BlockAllDistributedWrites blocks all modifications to distributed tables - * by taking ShareRowExclusive locks on all distributed tables. + * BlockDistributedTransactions blocks distributed transactions that use 2PC + * and changes to pg_dist_node (e.g. node addition) and pg_dist_partition + * (table creation). */ static void -BlockAllDistributedWrites(void) +BlockDistributedTransactions(void) { - ListCell *distributedTableCell = NULL; - List *distributedTableList = DistributedTableList(); - LockRelationOid(DistNodeRelationId(), ExclusiveLock); LockRelationOid(DistPartitionRelationId(), ExclusiveLock); - - foreach(distributedTableCell, distributedTableList) - { - DistTableCacheEntry *cacheEntry = - (DistTableCacheEntry *) lfirst(distributedTableCell); - - /* block all modifications */ - LockRelationOid(cacheEntry->relationId, ShareRowExclusiveLock); - } + LockRelationOid(DistTransactionRelationId(), ExclusiveLock); } diff --git a/src/test/regress/expected/isolation_create_restore_point.out b/src/test/regress/expected/isolation_create_restore_point.out index f4a29402f..36def0ad7 100644 --- a/src/test/regress/expected/isolation_create_restore_point.out +++ b/src/test/regress/expected/isolation_create_restore_point.out @@ -6,6 +6,7 @@ create_distributed_table step s1-begin: BEGIN; + SET citus.multi_shard_commit_protocol TO '2pc'; step s1-create-distributed: CREATE TABLE test_create_distributed_table (test_id integer NOT NULL, data text); @@ -31,20 +32,20 @@ create_distributed_table step s1-begin: BEGIN; + SET citus.multi_shard_commit_protocol TO '2pc'; step s1-insert: INSERT INTO restore_table VALUES (1,'hello'); step s2-create-restore: SELECT 1 FROM citus_create_restore_point('citus-test'); - -step s1-commit: - COMMIT; -step s2-create-restore: <... completed> ?column? 1 +step s1-commit: + COMMIT; + starting permutation: s1-begin s1-modify-multiple s2-create-restore s1-commit create_distributed_table @@ -52,23 +53,20 @@ create_distributed_table step s1-begin: BEGIN; + SET citus.multi_shard_commit_protocol TO '2pc'; step s1-modify-multiple: - SELECT master_modify_multiple_shards($$UPDATE restore_table SET data = 'world'$$); + UPDATE restore_table SET data = 'world'; -master_modify_multiple_shards - -0 step s2-create-restore: SELECT 1 FROM citus_create_restore_point('citus-test'); - -step s1-commit: - COMMIT; -step s2-create-restore: <... completed> ?column? 1 +step s1-commit: + COMMIT; + starting permutation: s1-begin s1-ddl s2-create-restore s1-commit create_distributed_table @@ -76,20 +74,20 @@ create_distributed_table step s1-begin: BEGIN; + SET citus.multi_shard_commit_protocol TO '2pc'; step s1-ddl: ALTER TABLE restore_table ADD COLUMN x int; step s2-create-restore: SELECT 1 FROM citus_create_restore_point('citus-test'); - -step s1-commit: - COMMIT; -step s2-create-restore: <... completed> ?column? 1 +step s1-commit: + COMMIT; + starting permutation: s1-begin s1-copy s2-create-restore s1-commit create_distributed_table @@ -97,10 +95,35 @@ create_distributed_table step s1-begin: BEGIN; + SET citus.multi_shard_commit_protocol TO '2pc'; step s1-copy: COPY restore_table FROM PROGRAM 'echo 1,hello' WITH CSV; +step s2-create-restore: + SELECT 1 FROM citus_create_restore_point('citus-test'); + +?column? + +1 +step s1-commit: + COMMIT; + + +starting permutation: s1-begin s1-recover s2-create-restore s1-commit +create_distributed_table + + +step s1-begin: + BEGIN; + SET citus.multi_shard_commit_protocol TO '2pc'; + +step s1-recover: + SELECT recover_prepared_transactions(); + +recover_prepared_transactions + +0 step s2-create-restore: SELECT 1 FROM citus_create_restore_point('citus-test'); @@ -118,6 +141,7 @@ create_distributed_table step s1-begin: BEGIN; + SET citus.multi_shard_commit_protocol TO '2pc'; step s1-drop: DROP TABLE restore_table; @@ -139,6 +163,7 @@ create_distributed_table step s1-begin: BEGIN; + SET citus.multi_shard_commit_protocol TO '2pc'; step s1-add-node: SELECT 1 FROM master_add_inactive_node('localhost', 9999); @@ -163,6 +188,7 @@ create_distributed_table step s1-begin: BEGIN; + SET citus.multi_shard_commit_protocol TO '2pc'; step s1-remove-node: SELECT master_remove_node('localhost', 9999); @@ -180,3 +206,95 @@ step s2-create-restore: <... completed> ?column? 1 + +starting permutation: s1-begin s1-create-restore s2-create-restore s1-commit +create_distributed_table + + +step s1-begin: + BEGIN; + SET citus.multi_shard_commit_protocol TO '2pc'; + +step s1-create-restore: + SELECT 1 FROM citus_create_restore_point('citus-test-2'); + +?column? + +1 +step s2-create-restore: + SELECT 1 FROM citus_create_restore_point('citus-test'); + +step s1-commit: + COMMIT; + +step s2-create-restore: <... completed> +?column? + +1 + +starting permutation: s2-begin s2-create-restore s1-modify-multiple s2-commit +create_distributed_table + + +step s2-begin: + BEGIN; + +step s2-create-restore: + SELECT 1 FROM citus_create_restore_point('citus-test'); + +?column? + +1 +step s1-modify-multiple: + UPDATE restore_table SET data = 'world'; + +step s2-commit: + COMMIT; + +step s1-modify-multiple: <... completed> + +starting permutation: s2-begin s2-create-restore s1-ddl s2-commit +create_distributed_table + + +step s2-begin: + BEGIN; + +step s2-create-restore: + SELECT 1 FROM citus_create_restore_point('citus-test'); + +?column? + +1 +step s1-ddl: + ALTER TABLE restore_table ADD COLUMN x int; + +step s2-commit: + COMMIT; + +step s1-ddl: <... completed> + +starting permutation: s2-begin s2-create-restore s1-multi-statement s2-commit +create_distributed_table + + +step s2-begin: + BEGIN; + +step s2-create-restore: + SELECT 1 FROM citus_create_restore_point('citus-test'); + +?column? + +1 +step s1-multi-statement: + SET citus.multi_shard_commit_protocol TO '2pc'; + BEGIN; + INSERT INTO restore_table VALUES (1,'hello'); + INSERT INTO restore_table VALUES (2,'hello'); + COMMIT; + +step s2-commit: + COMMIT; + +step s1-multi-statement: <... completed> diff --git a/src/test/regress/specs/isolation_create_restore_point.spec b/src/test/regress/specs/isolation_create_restore_point.spec index ed9bc7078..7688d7c48 100644 --- a/src/test/regress/specs/isolation_create_restore_point.spec +++ b/src/test/regress/specs/isolation_create_restore_point.spec @@ -1,5 +1,6 @@ setup { + SET citus.shard_replication_factor TO 1; CREATE TABLE restore_table (test_id integer NOT NULL, data text); SELECT create_distributed_table('restore_table', 'test_id'); } @@ -14,6 +15,7 @@ session "s1" step "s1-begin" { BEGIN; + SET citus.multi_shard_commit_protocol TO '2pc'; } step "s1-create-distributed" @@ -29,7 +31,16 @@ step "s1-insert" step "s1-modify-multiple" { - SELECT master_modify_multiple_shards($$UPDATE restore_table SET data = 'world'$$); + UPDATE restore_table SET data = 'world'; +} + +step "s1-multi-statement" +{ + SET citus.multi_shard_commit_protocol TO '2pc'; + BEGIN; + INSERT INTO restore_table VALUES (1,'hello'); + INSERT INTO restore_table VALUES (2,'hello'); + COMMIT; } step "s1-ddl" @@ -42,6 +53,16 @@ step "s1-copy" COPY restore_table FROM PROGRAM 'echo 1,hello' WITH CSV; } +step "s1-recover" +{ + SELECT recover_prepared_transactions(); +} + +step "s1-create-restore" +{ + SELECT 1 FROM citus_create_restore_point('citus-test-2'); +} + step "s1-drop" { DROP TABLE restore_table; @@ -64,26 +85,39 @@ step "s1-commit" session "s2" +step "s2-begin" +{ + BEGIN; +} + step "s2-create-restore" { SELECT 1 FROM citus_create_restore_point('citus-test'); } +step "s2-commit" +{ + COMMIT; +} + # verify that citus_create_restore_point is blocked by concurrent create_distributed_table permutation "s1-begin" "s1-create-distributed" "s2-create-restore" "s1-commit" -# verify that citus_create_restore_point is blocked by concurrent INSERT +# verify that citus_create_restore_point is not blocked by concurrent INSERT (only commit) permutation "s1-begin" "s1-insert" "s2-create-restore" "s1-commit" -# verify that citus_create_restore_point is blocked by concurrent master_modify_multiple_shards +# verify that citus_create_restore_point is not blocked by concurrent multi-shard UPDATE (only commit) permutation "s1-begin" "s1-modify-multiple" "s2-create-restore" "s1-commit" -# verify that citus_create_restore_point is blocked by concurrent DDL +# verify that citus_create_restore_point is not blocked by concurrent DDL (only commit) permutation "s1-begin" "s1-ddl" "s2-create-restore" "s1-commit" -# verify that citus_create_restore_point is blocked by concurrent COPY +# verify that citus_create_restore_point is not blocked by concurrent COPY (only commit) permutation "s1-begin" "s1-copy" "s2-create-restore" "s1-commit" +# verify that citus_create_restore_point is blocked by concurrent recover_prepared_transactions +permutation "s1-begin" "s1-recover" "s2-create-restore" "s1-commit" + # verify that citus_create_restore_point is blocked by concurrent DROP TABLE permutation "s1-begin" "s1-drop" "s2-create-restore" "s1-commit" @@ -92,3 +126,15 @@ permutation "s1-begin" "s1-add-node" "s2-create-restore" "s1-commit" # verify that citus_create_restore_point is blocked by concurrent master_remove_node permutation "s1-begin" "s1-remove-node" "s2-create-restore" "s1-commit" + +# verify that citus_create_restore_point is blocked by concurrent citus_create_restore_point +permutation "s1-begin" "s1-create-restore" "s2-create-restore" "s1-commit" + +# verify that multi-shard UPDATE is blocked by concurrent citus_create_restore_point +permutation "s2-begin" "s2-create-restore" "s1-modify-multiple" "s2-commit" + +# verify that DDL is blocked by concurrent citus_create_restore_point +permutation "s2-begin" "s2-create-restore" "s1-ddl" "s2-commit" + +# verify that multi-statement transactions is blocked by concurrent citus_create_restore_point +permutation "s2-begin" "s2-create-restore" "s1-multi-statement" "s2-commit"