Clean up multi_shard_commit_protocol guc leftovers (#6110)

src/backend/distributed/commands/multi_copy.c

@@ -30,11 +30,7 @@
  * By default, COPY uses normal transactions on the workers. In the case of
  * hash or range-partitioned tables, this can cause a problem when some of the
  * transactions fail to commit while others have succeeded. To ensure no data
- * is lost, COPY can use two-phase commit, by increasing max_prepared_transactions
+ * is lost, COPY uses two-phase commit.
- * on the worker and setting citus.multi_shard_commit_protocol to '2pc'. The default
- * is '1pc'. This is not a problem for append-partitioned tables because new
- * shards are created and in the case of failure, metadata changes are rolled
- * back on the master node.
  *
  * Parsing options are processed and enforced on the node where copy command
  * is run, while constraints are enforced on the worker. In either case,

src/backend/distributed/operations/modify_multiple_shards.c

@@ -5,9 +5,7 @@
  *
  * This file contains master_modify_multiple_shards function, which takes a update
  * or delete query and runs it worker shards of the distributed table. The distributed
- * modify operation can be done within a distributed transaction and committed in
+ * modify operation can be done within a distributed transaction.
- * one-phase or two-phase fashion, depending on the citus.multi_shard_commit_protocol
- * setting.
  *
  * Copyright (c) Citus Data, Inc.
  *

src/test/regress/expected/isolation_create_restore_point.out

@@ -8,7 +8,6 @@ create_reference_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);
@@ -40,7 +39,6 @@ create_reference_table
 
 step s1-begin:
 	BEGIN;
-	SET citus.multi_shard_commit_protocol TO '2pc';
 
 step s1-insert:
 	INSERT INTO restore_table VALUES (1,'hello');
@@ -65,7 +63,6 @@ create_reference_table
 
 step s1-begin:
 	BEGIN;
-	SET citus.multi_shard_commit_protocol TO '2pc';
 
 step s1-modify-multiple:
 	UPDATE restore_table SET data = 'world';
@@ -90,7 +87,6 @@ create_reference_table
 
 step s1-begin:
 	BEGIN;
-	SET citus.multi_shard_commit_protocol TO '2pc';
 
 step s1-ddl:
 	ALTER TABLE restore_table ADD COLUMN x int;
@@ -116,7 +112,6 @@ create_reference_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;
@@ -141,7 +136,6 @@ create_reference_table
 
 step s1-begin:
 	BEGIN;
-	SET citus.multi_shard_commit_protocol TO '2pc';
 
 step s1-recover:
 	SELECT recover_prepared_transactions();
@@ -172,7 +166,6 @@ create_reference_table
 
 step s1-begin:
 	BEGIN;
-	SET citus.multi_shard_commit_protocol TO '2pc';
 
 step s1-drop:
 	DROP TABLE restore_table;
@@ -198,7 +191,6 @@ create_reference_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);
@@ -229,7 +221,6 @@ create_reference_table
 
 step s1-begin:
 	BEGIN;
-	SET citus.multi_shard_commit_protocol TO '2pc';
 
 step s1-remove-node:
 	SELECT master_remove_node('localhost', 9999);
@@ -260,7 +251,6 @@ create_reference_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');
@@ -351,7 +341,6 @@ step s2-create-restore:
 (1 row)
 
 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');
@@ -370,7 +359,6 @@ create_reference_table
 
 step s1-begin:
 	BEGIN;
-	SET citus.multi_shard_commit_protocol TO '2pc';
 
 step s1-create-reference:
 	CREATE TABLE test_create_reference_table (test_id integer NOT NULL, data text);
@@ -402,7 +390,6 @@ create_reference_table
 
 step s1-begin:
 	BEGIN;
-	SET citus.multi_shard_commit_protocol TO '2pc';
 
 step s1-insert-ref:
 	INSERT INTO restore_ref_table VALUES (1,'hello');
@@ -428,7 +415,6 @@ create_reference_table
 
 step s1-begin:
 	BEGIN;
-	SET citus.multi_shard_commit_protocol TO '2pc';
 
 step s1-modify-multiple-ref:
 	UPDATE restore_ref_table SET data = 'world';
@@ -454,7 +440,6 @@ create_reference_table
 
 step s1-begin:
 	BEGIN;
-	SET citus.multi_shard_commit_protocol TO '2pc';
 
 step s1-ddl-ref:
 	ALTER TABLE restore_ref_table ADD COLUMN x int;
@@ -480,7 +465,6 @@ create_reference_table
 
 step s1-begin:
 	BEGIN;
-	SET citus.multi_shard_commit_protocol TO '2pc';
 
 step s1-copy-ref:
 	COPY restore_ref_table FROM PROGRAM 'echo 1,hello' WITH CSV;
@@ -506,7 +490,6 @@ create_reference_table
 
 step s1-begin:
 	BEGIN;
-	SET citus.multi_shard_commit_protocol TO '2pc';
 
 step s1-drop-ref:
 	DROP TABLE restore_ref_table;
@@ -592,7 +575,6 @@ step s2-create-restore:
 (1 row)
 
 step s1-multi-statement-ref:
-	SET citus.multi_shard_commit_protocol TO '2pc';
 	BEGIN;
 	INSERT INTO restore_ref_table VALUES (1,'hello');
 	INSERT INTO restore_ref_table VALUES (2,'hello');

src/test/regress/expected/isolation_distributed_deadlock_detection.out

@@ -90,57 +90,6 @@ step s2-commit:
   COMMIT;
 
 
-starting permutation: s1-begin s2-begin s1-set-2pc s2-set-2pc s1-update-1 s2-update-2 s2-update-1 deadlock-checker-call s1-update-2 deadlock-checker-call s1-commit s2-commit
-step s1-begin:
-  BEGIN;
-
-step s2-begin:
-  BEGIN;
-
-step s1-set-2pc:
- set citus.multi_shard_commit_protocol TO '2pc';
-
-step s2-set-2pc:
- set citus.multi_shard_commit_protocol TO '2pc';
-
-step s1-update-1:
-  UPDATE deadlock_detection_test SET some_val = 1 WHERE user_id = 1;
-
-step s2-update-2:
-  UPDATE deadlock_detection_test SET some_val = 2 WHERE user_id = 2;
-
-step s2-update-1:
-  UPDATE deadlock_detection_test SET some_val = 2 WHERE user_id = 1;
- <waiting ...>
-step deadlock-checker-call: 
-  SELECT check_distributed_deadlocks();
-
-check_distributed_deadlocks
----------------------------------------------------------------------
-f
-(1 row)
-
-step s1-update-2:
-  UPDATE deadlock_detection_test SET some_val = 1 WHERE user_id = 2;
- <waiting ...>
-step deadlock-checker-call: 
-  SELECT check_distributed_deadlocks();
-
-check_distributed_deadlocks
----------------------------------------------------------------------
-t
-(1 row)
-
-step s2-update-1: <... completed>
-ERROR:  canceling the transaction since it was involved in a distributed deadlock
-step s1-update-2: <... completed>
-step s1-commit:
-  COMMIT;
-
-step s2-commit:
-  COMMIT;
-
-
 starting permutation: s1-begin s2-begin s1-update-1 s2-update-2 s1-update-2 deadlock-checker-call s2-upsert-select-all deadlock-checker-call s1-commit s2-commit
 step s1-begin:
   BEGIN;

src/test/regress/input/multi_alter_table_statements.source

@@ -305,19 +305,7 @@ CREATE EVENT TRIGGER log_ddl_tag ON ddl_command_end EXECUTE PROCEDURE log_ddl_ta
 
 \c - - - :master_port
 -- The above trigger will cause failure at transaction end on one placement.
--- We'll test 2PC first, as it should handle this "best" (no divergence)
+-- Citus always uses 2PC. 2PC should handle this "best" (no divergence)
-SET citus.multi_shard_commit_protocol TO '2pc';
-BEGIN;
-CREATE INDEX single_index_2 ON single_shard_items(id);
-CREATE INDEX single_index_3 ON single_shard_items(name);
-COMMIT;
-
--- Nothing from the block should have committed
-SELECT indexname, tablename FROM pg_indexes WHERE tablename = 'single_shard_items' ORDER BY 1;
-
--- Even if 1PC is picked for multi-shard commands
--- Citus always uses 2PC for replication > 1
-SET citus.multi_shard_commit_protocol TO '1pc';
 BEGIN;
 CREATE INDEX single_index_2 ON single_shard_items(id);
 CREATE INDEX single_index_3 ON single_shard_items(name);
@@ -346,13 +334,12 @@ COMMIT;
 SELECT indexname, tablename FROM pg_indexes WHERE tablename = 'lineitem_alter';
 DROP INDEX temp_index_2;
 
---- verify that distributed ddl commands can be used with 2pc
+-- verify that distributed ddl commands are allowed without transaction block as well
-SET citus.multi_shard_commit_protocol TO '2pc';
+-- Reminder: Now Citus always uses 2PC
 CREATE INDEX temp_index_3 ON lineitem_alter(l_orderkey);
 SELECT indexname, tablename FROM pg_indexes WHERE tablename = 'lineitem_alter';
 DROP INDEX temp_index_3;
 SELECT indexname, tablename FROM pg_indexes WHERE tablename = 'lineitem_alter';
-RESET citus.multi_shard_commit_protocol;
 
 -- verify that not any of shard placements are marked as failed when a query failure occurs
 CREATE TABLE test_ab (a int, b int);

src/test/regress/output/multi_alter_table_statements.source

@@ -652,24 +652,7 @@ RESET citus.enable_metadata_sync;
 CREATE EVENT TRIGGER log_ddl_tag ON ddl_command_end EXECUTE PROCEDURE log_ddl_tag();
 \c - - - :master_port
 -- The above trigger will cause failure at transaction end on one placement.
--- We'll test 2PC first, as it should handle this "best" (no divergence)
+-- Citus always uses 2PC. 2PC should handle this "best" (no divergence)
-SET citus.multi_shard_commit_protocol TO '2pc';
-BEGIN;
-CREATE INDEX single_index_2 ON single_shard_items(id);
-CREATE INDEX single_index_3 ON single_shard_items(name);
-COMMIT;
-ERROR:  duplicate key value violates unique constraint "ddl_commands_command_key"
-DETAIL:  Key (command)=(CREATE INDEX) already exists.
-CONTEXT:  while executing command on localhost:57638
--- Nothing from the block should have committed
-SELECT indexname, tablename FROM pg_indexes WHERE tablename = 'single_shard_items' ORDER BY 1;
- indexname | tablename 
------------+-----------
-(0 rows)
-
--- Even if 1PC is picked for multi-shard commands
--- Citus always uses 2PC for replication > 1
-SET citus.multi_shard_commit_protocol TO '1pc';
 BEGIN;
 CREATE INDEX single_index_2 ON single_shard_items(id);
 CREATE INDEX single_index_3 ON single_shard_items(name);
@@ -715,8 +698,8 @@ SELECT indexname, tablename FROM pg_indexes WHERE tablename = 'lineitem_alter';
 (1 row)
 
 DROP INDEX temp_index_2;
---- verify that distributed ddl commands can be used with 2pc
+-- verify that distributed ddl commands are allowed without transaction block as well
-SET citus.multi_shard_commit_protocol TO '2pc';
+-- Reminder: Now Citus always uses 2PC
 CREATE INDEX temp_index_3 ON lineitem_alter(l_orderkey);
 SELECT indexname, tablename FROM pg_indexes WHERE tablename = 'lineitem_alter';
   indexname   |   tablename    
@@ -730,7 +713,6 @@ SELECT indexname, tablename FROM pg_indexes WHERE tablename = 'lineitem_alter';
 -----------+-----------
 (0 rows)
 
-RESET citus.multi_shard_commit_protocol;
 -- verify that not any of shard placements are marked as failed when a query failure occurs
 CREATE TABLE test_ab (a int, b int);
 SET citus.shard_count TO 8;

src/test/regress/spec/isolation_create_restore_point.spec

@@ -17,7 +17,6 @@ session "s1"
 step "s1-begin"
 {
 	BEGIN;
-	SET citus.multi_shard_commit_protocol TO '2pc';
 }
 
 step "s1-create-reference"
@@ -54,7 +53,6 @@ step "s1-modify-multiple-ref"
 
 step "s1-multi-statement-ref"
 {
-	SET citus.multi_shard_commit_protocol TO '2pc';
 	BEGIN;
 	INSERT INTO restore_ref_table VALUES (1,'hello');
 	INSERT INTO restore_ref_table VALUES (2,'hello');
@@ -63,7 +61,6 @@ step "s1-multi-statement-ref"
 
 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');

src/test/regress/spec/isolation_distributed_deadlock_detection.spec

@@ -63,11 +63,6 @@ step "s1-insert-local-10"
   INSERT INTO local_deadlock_table VALUES (10, 10);
 }
 
-step "s1-set-2pc"
-{
-	set citus.multi_shard_commit_protocol TO '2pc';
-}
-
 step "s1-update-1-rep-2"
 {
   UPDATE deadlock_detection_test_rep_2 SET some_val = 1 WHERE user_id = 1;
@@ -145,11 +140,6 @@ step "s2-insert-local-10"
   INSERT INTO local_deadlock_table VALUES (10, 10);
 }
 
-step "s2-set-2pc"
-{
-	set citus.multi_shard_commit_protocol TO '2pc';
-}
-
 step "s2-update-1-rep-2"
 {
   UPDATE deadlock_detection_test_rep_2 SET some_val = 1 WHERE user_id = 1;
@@ -315,15 +305,12 @@ step "deadlock-checker-call"
   SELECT check_distributed_deadlocks();
 }
 
-// simplest case, loop with two nodes
+// simplest case, loop with two nodes (Reminder: Citus uses 2PC)
 permutation "s1-begin" "s2-begin" "s1-update-1" "s2-update-2" "s2-update-1" "deadlock-checker-call" "s1-update-2"  "deadlock-checker-call" "s1-commit" "s2-commit"
 
 // simplest case with replication factor 2
 permutation "s1-begin" "s2-begin" "s1-update-1-rep-2" "s2-update-2-rep-2" "s2-update-1-rep-2" "deadlock-checker-call" "s1-update-2-rep-2"  "deadlock-checker-call" "s1-commit" "s2-commit"
 
-// simplest case with 2pc enabled
-permutation "s1-begin" "s2-begin" "s1-set-2pc" "s2-set-2pc" "s1-update-1" "s2-update-2" "s2-update-1" "deadlock-checker-call" "s1-update-2"  "deadlock-checker-call" "s1-commit" "s2-commit"
-
 // simplest case with multi-shard query is cancelled
 permutation "s1-begin" "s2-begin" "s1-update-1" "s2-update-2" "s1-update-2" "deadlock-checker-call" "s2-upsert-select-all"  "deadlock-checker-call" "s1-commit" "s2-commit"