Merge pull request #1725 from citusdata/restore_point_2pc

Block only 2PCs instead of all writes in citus_create_restore_point

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
+ * BlockDistributedTransactions blocks distributed transactions that use 2PC
- * by taking ShareRowExclusive locks on all distributed tables.
+ * 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);
-
+	LockRelationOid(DistTransactionRelationId(), ExclusiveLock);
-	foreach(distributedTableCell, distributedTableList)
-	{
-		DistTableCacheEntry *cacheEntry =
-			(DistTableCacheEntry *) lfirst(distributedTableCell);
-
-		/* block all modifications */
-		LockRelationOid(cacheEntry->relationId, ShareRowExclusiveLock);
-	}
 }
 
 

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');
- <waiting ...>
-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');
- <waiting ...>
-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');
- <waiting ...>
-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');
  <waiting ...>
@@ -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');
+ <waiting ...>
+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';
+ <waiting ...>
+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;
+ <waiting ...>
+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;
+ <waiting ...>
+step s2-commit: 
+	COMMIT;
+
+step s1-multi-statement: <... completed>

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"