From 9e182256a4c4e7485453ba83fd9276ac688a5f4d Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Wed, 3 Jun 2020 16:38:13 +0900
Subject: [PATCH v23 7/7] Add prefer mode to foreign_twophase_commit.

Co-authored-by: Masahiko Sawada, Ashutosh Bapat
---
 doc/src/sgml/config.sgml                      | 29 +++++------
 doc/src/sgml/distributed-transaction.sgml     | 10 ++--
 src/backend/access/fdwxact/fdwxact.c          | 49 ++++++++++++++++---
 src/backend/utils/misc/guc.c                  |  5 +-
 src/backend/utils/misc/postgresql.conf.sample |  2 +-
 src/include/access/fdwxact.h                  |  1 +
 .../test_fdwxact/expected/test_fdwxact.out    | 25 ++++++++++
 .../modules/test_fdwxact/sql/test_fdwxact.sql | 28 +++++++++++
 8 files changed, 119 insertions(+), 30 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 5d81ad08c3..29db09ca46 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -9087,18 +9087,19 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
         <para>
          Specifies whether distributed transaction commits ensures that all
          involved changes on foreign servers are committed or not. Valid
-         values are <literal>required</literal> and <literal>disabled</literal>.
-         The default setting is <literal>disabled</literal>. Setting to
-         <literal>disabled</literal> don't use two-phase commit protocol to
-         commit or rollback distributed transactions. When set to
-         <literal>required</literal> distributed transactions strictly requires
-         that all written servers can use two-phase commit protocol.  That is,
-         the distributed transaction cannot commit if even one server does not
-         support the prepare callback routine
+         values are <literal>required</literal>, <literal>prefer</literal> and
+         <literal>disabled</literal>. The default setting is
+         <literal>disabled</literal>. Setting to <literal>disabled</literal>
+         don't use two-phase commit protocol to commit or rollback distributed
+         transactions. When set to <literal>required</literal> distributed
+         transactions strictly requires that all written servers can use
+         two-phase commit protocol.  That is, the distributed transaction cannot
+         commit if even one server does not support the prepare callback routine
          (described in <xref linkend="fdw-callbacks-transaction-management"/>).
-         In <literal>required</literal> case, distributed transaction commit will
-         wait for all involving foreign transaction to be committed before the
-         command return a "success" indication to the client.
+         In <literal>prefer</literal> and <literal>required</literal> case,
+         distributed transaction commit will wait for all involving foreign
+         transaction to be committed before the command return a "success"
+         indication to the client.
         </para>
 
         <para>
@@ -9108,9 +9109,9 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
 
         <note>
          <para>
-          When <literal>disabled</literal> there can be risk of database
-          consistency if one or more foreign servers crashes while committing
-          the distributed transactions.
+          When <literal>disabled</literal> or <literal>prefer</literal>  there
+          can be risk of database consistency if one or more foreign servers
+          crashes while committing the distributed transactions.
          </para>
         </note>
        </listitem>
diff --git a/doc/src/sgml/distributed-transaction.sgml b/doc/src/sgml/distributed-transaction.sgml
index b4b1e26a55..845b9508be 100644
--- a/doc/src/sgml/distributed-transaction.sgml
+++ b/doc/src/sgml/distributed-transaction.sgml
@@ -48,11 +48,11 @@
        prepares all transaction on the foreign servers if two-phase commit is
        required. Two-phase commit is required when the transaction modifies
        data on two or more servers including the local server itself and
-       <xref linkend="guc-foreign-twophase-commit"/> is
-       <literal>required</literal>. If the prepare on all foreign servers is
-       successful then go to the next step.  If there is any failure in the
-       prepare phase, the server will rollback all the transactions on both
-       local and foreign servers.
+       <xref linkend="guc-foreign-twophase-commit"/> is either
+       <literal>required</literal> or <literal>prefer</literal>. If the prepare
+       on all foreign servers is successful then go to the next step.  If
+       there is any failure in the prepare phase, the server will rollback
+       all the transactions on both local and foreign servers.
       </para>
      </listitem>
      <listitem>
diff --git a/src/backend/access/fdwxact/fdwxact.c b/src/backend/access/fdwxact/fdwxact.c
index 76b973b473..9e5858bb12 100644
--- a/src/backend/access/fdwxact/fdwxact.c
+++ b/src/backend/access/fdwxact/fdwxact.c
@@ -438,7 +438,9 @@ create_fdwxact_participant(Oid serverid, Oid userid, FdwRoutine *routine)
  * When foreign twophase commit is enabled, the behavior depends on the value
  * of foreign_twophase_commit; when 'required' we strictly require for all
  * foreign servers' FDW to support two-phase commit protocol and ask them to
- * prepare foreign transactions, and when 'disabled' we ask all foreign servers
+ * prepare foreign transactions, when 'prefer' we ask only foreign servers
+ * that are capable of two-phase commit to prepare foreign transactions and ask
+ * for other servers to commit, and when 'disabled' we ask all foreign servers
  * to commit foreign transaction in one-phase. If we failed to commit any of
  * them we change to aborting.
  *
@@ -506,8 +508,9 @@ checkForeignTwophaseCommitRequired(void)
 {
 	ListCell   *lc;
 	bool		need_twophase_commit;
-	bool		have_notwophase = false;
+	bool		have_notwophase;
 	int			nserverswritten = 0;
+	int			nserverstwophase = 0;
 
 	if (!IsForeignTwophaseCommitRequested())
 		return false;
@@ -519,22 +522,51 @@ checkForeignTwophaseCommitRequired(void)
 		if (!fdw_part->modified)
 			continue;
 
-		if (!SeverSupportTwophaseCommit(fdw_part))
-			have_notwophase = true;
+		if (SeverSupportTwophaseCommit(fdw_part))
+			nserverstwophase++;
 
 		nserverswritten++;
 	}
+	Assert(nserverswritten >= nserverstwophase);
+
+	/* check if there is any servers that don't support two-phase commit */
+	have_notwophase = (nserverswritten != nserverstwophase);
 
 	/* Did we modify the local non-temporary data? */
 	if ((MyXactFlags & XACT_FLAGS_WROTENONTEMPREL) != 0)
+	{
 		nserverswritten++;
 
+		/*
+		 * We increment nserverstwophase as well for making code simple,
+		 * although we don't actually use two-phase commit for the local
+		 * transaction.
+		 */
+		nserverstwophase++;
+	}
+
 	if (nserverswritten <= 1)
 		return false;
 
-	/* We require for all modified server to support two-phase commit */
-	need_twophase_commit = (nserverswritten >= 2);
-	Assert(foreign_twophase_commit == FOREIGN_TWOPHASE_COMMIT_REQUIRED);
+	if (foreign_twophase_commit == FOREIGN_TWOPHASE_COMMIT_REQUIRED)
+	{
+		/*
+		 * In 'required' case, we require for all modified server to support
+		 * two-phase commit.
+		 */
+		need_twophase_commit = (nserverswritten >= 2);
+	}
+	else
+	{
+		Assert(foreign_twophase_commit == FOREIGN_TWOPHASE_COMMIT_PREFER);
+
+		/*
+		 * In 'prefer' case, we use two-phase commit when this transaction modified
+		 * two or more servers including the local server or servers that support
+		 * two-phase commit.
+		 */
+		need_twophase_commit = (nserverstwophase >= 2);
+	}
 
 	/*
 	 * If foreign two phase commit is required then all foreign serves must be
@@ -555,7 +587,8 @@ checkForeignTwophaseCommitRequired(void)
 					 errmsg("foreign two-phase commit is required but prepared foreign transactions are disabled"),
 					 errhint("Set max_foreign_transaction_resolvers to a nonzero value.")));
 
-		if (have_notwophase)
+		if (have_notwophase &&
+			foreign_twophase_commit == FOREIGN_TWOPHASE_COMMIT_REQUIRED)
 			ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 					 errmsg("cannot process a distributed transaction that has operated on a foreign server that does not support two-phase commit protocol"),
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 72fe0a7167..fddb172a96 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -428,11 +428,12 @@ static const struct config_enum_entry synchronous_commit_options[] = {
 };
 
 /*
- * Although only "required" and "disabled" are documented, we accept all
- * the likely variants of "on" and "off".
+ * Although only "required", "prefer", and "disabled" are documented,
+ *  we accept all the likely variants of "on" and "off".
  */
 static const struct config_enum_entry foreign_twophase_commit_options[] = {
 	{"required", FOREIGN_TWOPHASE_COMMIT_REQUIRED, false},
+	{"prefer", FOREIGN_TWOPHASE_COMMIT_PREFER, false},
 	{"disabled", FOREIGN_TWOPHASE_COMMIT_DISABLED, false},
 	{"on", FOREIGN_TWOPHASE_COMMIT_REQUIRED, false},
 	{"off", FOREIGN_TWOPHASE_COMMIT_DISABLED, false},
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 5ed8617787..7f76e2dfcc 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -358,7 +358,7 @@
 							# foreign transactions
 							# after a failed attempt
 #foreign_twophase_commit = disabled	# use two-phase commit for distributed transactions:
-					# disabled or required
+					# disabled, prefer or required
 
 #------------------------------------------------------------------------------
 # QUERY TUNING
diff --git a/src/include/access/fdwxact.h b/src/include/access/fdwxact.h
index d550ee9b87..965dbfc57f 100644
--- a/src/include/access/fdwxact.h
+++ b/src/include/access/fdwxact.h
@@ -37,6 +37,7 @@
 typedef enum
 {
 	FOREIGN_TWOPHASE_COMMIT_DISABLED,	/* disable foreign twophase commit */
+	FOREIGN_TWOPHASE_COMMIT_PREFER, /* use twophase commit where available */
 	FOREIGN_TWOPHASE_COMMIT_REQUIRED	/* all foreign servers have to support
 										 * twophase commit */
 }			ForeignTwophaseCommitLevel;
diff --git a/src/test/modules/test_fdwxact/expected/test_fdwxact.out b/src/test/modules/test_fdwxact/expected/test_fdwxact.out
index c6a91ac9f1..ce8465b52c 100644
--- a/src/test/modules/test_fdwxact/expected/test_fdwxact.out
+++ b/src/test/modules/test_fdwxact/expected/test_fdwxact.out
@@ -221,3 +221,28 @@ BEGIN;
 INSERT INTO ft_1 VALUES (1);
 PREPARE TRANSACTION 'global_x1';
 ERROR:  cannot PREPARE a distributed transaction when foreign_twophase_commit is 'disabled'
+-- Test 'prefer' mode.
+-- The cases where failed in 'required' mode should pass in 'prefer' mode.
+-- We simply commit/rollback a transaction in one-phase on a server
+-- that doesn't support two-phase commit, instead of error.
+SET foreign_twophase_commit TO 'prefer';
+BEGIN;
+INSERT INTO ft_1 VALUES (1);
+INSERT INTO ft_2pc_1 VALUES (1);
+COMMIT;
+BEGIN;
+INSERT INTO ft_no2pc_1 VALUES (1);
+INSERT INTO ft_2pc_1 VALUES (1);
+COMMIT;
+BEGIN;
+INSERT INTO ft_1 VALUES (1);
+INSERT INTO ft_2 VALUES (1);
+COMMIT;
+BEGIN;
+INSERT INTO ft_no2pc_1 VALUES (1);
+INSERT INTO ft_no2pc_2 VALUES (1);
+COMMIT;
+BEGIN;
+INSERT INTO t VALUES (1);
+INSERT INTO ft_no2pc_1 VALUES (1);
+COMMIT;
diff --git a/src/test/modules/test_fdwxact/sql/test_fdwxact.sql b/src/test/modules/test_fdwxact/sql/test_fdwxact.sql
index 8cf860e295..72a9ee6be4 100644
--- a/src/test/modules/test_fdwxact/sql/test_fdwxact.sql
+++ b/src/test/modules/test_fdwxact/sql/test_fdwxact.sql
@@ -191,3 +191,31 @@ PREPARE TRANSACTION 'global_x1';
 BEGIN;
 INSERT INTO ft_1 VALUES (1);
 PREPARE TRANSACTION 'global_x1';
+
+
+-- Test 'prefer' mode.
+-- The cases where failed in 'required' mode should pass in 'prefer' mode.
+-- We simply commit/rollback a transaction in one-phase on a server
+-- that doesn't support two-phase commit, instead of error.
+SET foreign_twophase_commit TO 'prefer';
+
+BEGIN;
+INSERT INTO ft_1 VALUES (1);
+INSERT INTO ft_2pc_1 VALUES (1);
+COMMIT;
+BEGIN;
+INSERT INTO ft_no2pc_1 VALUES (1);
+INSERT INTO ft_2pc_1 VALUES (1);
+COMMIT;
+BEGIN;
+INSERT INTO ft_1 VALUES (1);
+INSERT INTO ft_2 VALUES (1);
+COMMIT;
+BEGIN;
+INSERT INTO ft_no2pc_1 VALUES (1);
+INSERT INTO ft_no2pc_2 VALUES (1);
+COMMIT;
+BEGIN;
+INSERT INTO t VALUES (1);
+INSERT INTO ft_no2pc_1 VALUES (1);
+COMMIT;
-- 
2.23.0

