Fix a bug with SSI and prepared transactions:

If there's a dangerous structure T0 ---> T1 ---> T2, and T2 commits first,
we need to abort something. If T2 commits before both conflicts appear,
then it should be caught by OnConflict_CheckForSerializationFailure. If
both conflicts appear before T2 commits, it should be caught by
PreCommit_CheckForSerializationFailure. But that is actually run when
T2 *prepares*. Fix that in OnConflict_CheckForSerializationFailure, by
treating a prepared T2 as if it committed already.

This is mostly a problem for prepared transactions, which are in prepared
state for some time, but also for regular transactions because they also go
through the prepared state in the SSI code for a short moment when they're
committed.

Kevin Grittner and Dan Ports

diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 1707a746646f519c314b65e2d034570bd9b480f2..f0c8ee48c340403fdf075b8ee55b44066971845e 100644 (file)
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -244,6 +244,14 @@
 
 #define SxactIsOnFinishedList(sxact) (!SHMQueueIsDetached(&((sxact)->finishedLink)))
 
+/*
+ * Note that a sxact is marked "prepared" once it has passed
+ * PreCommit_CheckForSerializationFailure, even if it isn't using
+ * 2PC. This is the point at which it can no longer be aborted.
+ *
+ * The PREPARED flag remains set after commit, so SxactIsCommitted
+ * implies SxactIsPrepared.
+ */
 #define SxactIsCommitted(sxact) (((sxact)->flags & SXACT_FLAG_COMMITTED) != 0)
 #define SxactIsPrepared(sxact) (((sxact)->flags & SXACT_FLAG_PREPARED) != 0)
 #define SxactIsRolledBack(sxact) (((sxact)->flags & SXACT_FLAG_ROLLED_BACK) != 0)
@@ -3165,6 +3173,13 @@ ReleasePredicateLocks(bool isCommit)
                 */
                MySerializableXact->flags |= SXACT_FLAG_DOOMED;
                MySerializableXact->flags |= SXACT_FLAG_ROLLED_BACK;
+               /*
+                * If the transaction was previously prepared, but is now failing due
+                * to a ROLLBACK PREPARED or (hopefully very rare) error after the
+                * prepare, clear the prepared flag.  This simplifies conflict
+                * checking.
+                */
+               MySerializableXact->flags &= ~SXACT_FLAG_PREPARED;
        }
 
        if (!topLevelIsDeclaredReadOnly)
@@ -4363,6 +4378,11 @@ OnConflict_CheckForSerializationFailure(const SERIALIZABLEXACT *reader,
         * - the writer committed before T2
         * - the reader is a READ ONLY transaction and the reader was concurrent
         *       with T2 (= reader acquired its snapshot before T2 committed)
+        *
+        * We also handle the case that T2 is prepared but not yet committed
+        * here. In that case T2 has already checked for conflicts, so if it
+        * commits first, making the above conflict real, it's too late for it
+        * to abort.
         *------------------------------------------------------------------------
         */
        if (!failure)
@@ -4381,7 +4401,12 @@ OnConflict_CheckForSerializationFailure(const SERIALIZABLEXACT *reader,
                {
                        SERIALIZABLEXACT *t2 = conflict->sxactIn;
 
-                       if (SxactIsCommitted(t2)
+                       /*
+                        * Note that if T2 is merely prepared but not yet committed, we
+                        * rely on t->commitSeqNo being InvalidSerCommitSeqNo, which is
+                        * larger than any valid commit sequence number.
+                        */
+                       if (SxactIsPrepared(t2)
                                && (!SxactIsCommitted(reader)
                                        || t2->commitSeqNo <= reader->commitSeqNo)
                                && (!SxactIsCommitted(writer)
@@ -4400,7 +4425,8 @@ OnConflict_CheckForSerializationFailure(const SERIALIZABLEXACT *reader,
        }
 
        /*------------------------------------------------------------------------
-        * Check whether the reader has become a pivot with a committed writer:
+        * Check whether the reader has become a pivot with a writer
+        * that's committed (or prepared):
         *
         *              T0 ------> R ------> W
         *                       rw                rw
@@ -4411,7 +4437,7 @@ OnConflict_CheckForSerializationFailure(const SERIALIZABLEXACT *reader,
         * - T0 is READ ONLY, and overlaps the writer
         *------------------------------------------------------------------------
         */
-       if (!failure && SxactIsCommitted(writer) && !SxactIsReadOnly(reader))
+       if (!failure && SxactIsPrepared(writer) && !SxactIsReadOnly(reader))
        {
                if (SxactHasSummaryConflictIn(reader))
                {
@@ -4427,6 +4453,12 @@ OnConflict_CheckForSerializationFailure(const SERIALIZABLEXACT *reader,
                {
                        SERIALIZABLEXACT *t0 = conflict->sxactOut;
 
+                       /*
+                        * Note that if the writer is merely prepared but not yet
+                        * committed, we rely on writer->commitSeqNo being
+                        * InvalidSerCommitSeqNo, which is larger than any valid commit
+                        * sequence number.
+                        */
                        if (!SxactIsDoomed(t0)
                                && (!SxactIsCommitted(t0)
                                        || t0->commitSeqNo >= writer->commitSeqNo)

diff --git a/src/test/regress/expected/prepared_xacts.out b/src/test/regress/expected/prepared_xacts.out
index e094476cb4ad7a7ca8035b934c9b00dbc4dcf588..9697ace16a516effb7671e056e60ab7a66a216d5 100644 (file)
--- a/src/test/regress/expected/prepared_xacts.out
+++ b/src/test/regress/expected/prepared_xacts.out
@@ -131,12 +131,12 @@ SELECT * FROM pxtest1;
  ddd
 (2 rows)
 
-INSERT INTO pxtest1 VALUES ('fff');
 -- This should fail, because the two transactions have a write-skew anomaly
-PREPARE TRANSACTION 'foo5';
+INSERT INTO pxtest1 VALUES ('fff');
 ERROR:  could not serialize access due to read/write dependencies among transactions
-DETAIL:  Canceled on commit attempt with conflict in from prepared pivot.
+DETAIL:  Canceled on identification as a pivot, during write.
 HINT:  The transaction might succeed if retried.
+PREPARE TRANSACTION 'foo5';
 SELECT gid FROM pg_prepared_xacts;
  gid  
 ------

diff --git a/src/test/regress/expected/prepared_xacts_1.out b/src/test/regress/expected/prepared_xacts_1.out
index acd90467328c8f0dcb3915d82fb8253bef5af83b..898f278c11ed8aab20ea1a6bc45a2fcabda04dbc 100644 (file)
--- a/src/test/regress/expected/prepared_xacts_1.out
+++ b/src/test/regress/expected/prepared_xacts_1.out
@@ -134,8 +134,8 @@ SELECT * FROM pxtest1;
  aaa
 (1 row)
 
-INSERT INTO pxtest1 VALUES ('fff');
 -- This should fail, because the two transactions have a write-skew anomaly
+INSERT INTO pxtest1 VALUES ('fff');
 PREPARE TRANSACTION 'foo5';
 ERROR:  prepared transactions are disabled
 HINT:  Set max_prepared_transactions to a nonzero value.

diff --git a/src/test/regress/sql/prepared_xacts.sql b/src/test/regress/sql/prepared_xacts.sql
index e06c9d47c132bdaecabb464629496562e2c052fb..7902152775c70751ff8ceaf67114559f5736ac04 100644 (file)
--- a/src/test/regress/sql/prepared_xacts.sql
+++ b/src/test/regress/sql/prepared_xacts.sql
@@ -74,9 +74,9 @@ SELECT gid FROM pg_prepared_xacts;
 
 BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
 SELECT * FROM pxtest1;
-INSERT INTO pxtest1 VALUES ('fff');
 
 -- This should fail, because the two transactions have a write-skew anomaly
+INSERT INTO pxtest1 VALUES ('fff');
 PREPARE TRANSACTION 'foo5';
 
 SELECT gid FROM pg_prepared_xacts;