While testing the fix for this one, I found another bug. Patches for
both are attached.
The first patch addresses this bug by re-adding SXACT_FLAG_ROLLED_BACK,
in a more limited form than its previous incarnation.
We need to be able to distinguish transactions that have already
called ReleasePredicateLocks and are thus eligible for cleanup from
those that have been merely marked for abort by other
backends. Transactions that are ROLLED_BACK are excluded from
SxactGlobalXmin calculations, but those that are merely DOOMED need to
be included.
Also update a couple of assertions to ensure we only try to clean up
ROLLED_BACK transactions.
The second patch fixes a bug in PreCommit_CheckForSerializationFailure.
This function checks whether there's a dangerous structure of the form
far ---> near ---> me
where neither the "far" or "near" transactions have committed. If so,
it aborts the "near" transaction by marking it as DOOMED. However, that
transaction might already be PREPARED. We need to check whether that's
the case and, if so, abort the transaction that's trying to commit
instead.
One of the prepared_xacts regression tests actually hits this bug.
I removed the anomaly from the duplicate-gids test so that it fails in
the intended way, and added a new test to check serialization failures
with a prepared transaction.
Dan
--
Dan R. K. Ports MIT CSAIL http://drkp.net/
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 6c55211..3678878 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -246,7 +246,6 @@
#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)
#define SxactIsDoomed(sxact) (((sxact)->flags & SXACT_FLAG_DOOMED) != 0)
#define SxactIsReadOnly(sxact) (((sxact)->flags & SXACT_FLAG_READ_ONLY) != 0)
#define SxactHasSummaryConflictIn(sxact) (((sxact)->flags & SXACT_FLAG_SUMMARY_CONFLICT_IN) != 0)
@@ -3047,7 +3046,7 @@ SetNewSxactGlobalXmin(void)
for (sxact = FirstPredXact(); sxact != NULL; sxact = NextPredXact(sxact))
{
- if (!SxactIsRolledBack(sxact)
+ if (!SxactIsDoomed(sxact)
&& !SxactIsCommitted(sxact)
&& sxact != OldCommittedSxact)
{
@@ -3114,7 +3113,6 @@ ReleasePredicateLocks(const bool isCommit)
Assert(!isCommit || SxactIsPrepared(MySerializableXact));
Assert(!isCommit || !SxactIsDoomed(MySerializableXact));
Assert(!SxactIsCommitted(MySerializableXact));
- Assert(!SxactIsRolledBack(MySerializableXact));
/* may not be serializable during COMMIT/ROLLBACK PREPARED */
if (MySerializableXact->pid != 0)
@@ -3153,22 +3151,7 @@ ReleasePredicateLocks(const bool isCommit)
MySerializableXact->flags |= SXACT_FLAG_READ_ONLY;
}
else
- {
- /*
- * The DOOMED flag indicates that we intend to roll back this
- * transaction and so it should not cause serialization
- * failures for other transactions that conflict with
- * it. Note that this flag might already be set, if another
- * backend marked this transaction for abort.
- *
- * The ROLLED_BACK flag further indicates that
- * ReleasePredicateLocks has been called, and so the
- * SerializableXact is eligible for cleanup. This means it
- * should not be considered when calculating SxactGlobalXmin.
- */
MySerializableXact->flags |= SXACT_FLAG_DOOMED;
- MySerializableXact->flags |= SXACT_FLAG_ROLLED_BACK;
- }
if (!topLevelIsDeclaredReadOnly)
{
@@ -3544,7 +3527,7 @@ ReleaseOneSerializableXact(SERIALIZABLEXACT *sxact, bool partial,
nextConflict;
Assert(sxact != NULL);
- Assert(SxactIsRolledBack(sxact) || SxactIsCommitted(sxact));
+ Assert(SxactIsDoomed(sxact) || SxactIsCommitted(sxact));
Assert(LWLockHeldByMe(SerializableFinishedListLock));
/*
diff --git a/src/include/storage/predicate_internals.h b/src/include/storage/predicate_internals.h
index 34c661d..495983f 100644
--- a/src/include/storage/predicate_internals.h
+++ b/src/include/storage/predicate_internals.h
@@ -90,22 +90,21 @@ typedef struct SERIALIZABLEXACT
int pid; /* pid of associated process */
} SERIALIZABLEXACT;
-#define SXACT_FLAG_COMMITTED 0x00000001 /* already committed */
-#define SXACT_FLAG_PREPARED 0x00000002 /* about to commit */
-#define SXACT_FLAG_ROLLED_BACK 0x00000004 /* already rolled back */
-#define SXACT_FLAG_DOOMED 0x00000008 /* will roll back */
+#define SXACT_FLAG_COMMITTED 0x00000001 /* already committed */
+#define SXACT_FLAG_PREPARED 0x00000002 /* about to commit */
+#define SXACT_FLAG_DOOMED 0x00000004 /* will roll back */
/*
* The following flag actually means that the flagged transaction has a
* conflict out *to a transaction which committed ahead of it*. It's hard
* to get that into a name of a reasonable length.
*/
-#define SXACT_FLAG_CONFLICT_OUT 0x00000010
-#define SXACT_FLAG_READ_ONLY 0x00000020
-#define SXACT_FLAG_DEFERRABLE_WAITING 0x00000040
-#define SXACT_FLAG_RO_SAFE 0x00000080
-#define SXACT_FLAG_RO_UNSAFE 0x00000100
-#define SXACT_FLAG_SUMMARY_CONFLICT_IN 0x00000200
-#define SXACT_FLAG_SUMMARY_CONFLICT_OUT 0x00000400
+#define SXACT_FLAG_CONFLICT_OUT 0x00000008
+#define SXACT_FLAG_READ_ONLY 0x00000010
+#define SXACT_FLAG_DEFERRABLE_WAITING 0x00000020
+#define SXACT_FLAG_RO_SAFE 0x00000040
+#define SXACT_FLAG_RO_UNSAFE 0x00000080
+#define SXACT_FLAG_SUMMARY_CONFLICT_IN 0x00000100
+#define SXACT_FLAG_SUMMARY_CONFLICT_OUT 0x00000200
/*
* The following types are used to provide an ad hoc list for holding
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 35a25e0..6c55211 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -4542,16 +4542,6 @@ PreCommit_CheckForSerializationFailure(void)
&& !SxactIsReadOnly(farConflict->sxactOut)
&& !SxactIsDoomed(farConflict->sxactOut)))
{
- if (SxactIsPrepared(nearConflict->sxactOut))
- {
- LWLockRelease(SerializableXactHashLock);
- ereport(ERROR,
- (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
- errmsg("could not serialize access due to read/write dependencies among transactions"),
- errdetail("Cancelled on commit attempt with conflict in from prepared pivot."),
- errhint("The transaction might succeed if retried.")));
-
- }
nearConflict->sxactOut->flags |= SXACT_FLAG_DOOMED;
break;
}
diff --git a/src/test/regress/expected/prepared_xacts.out b/src/test/regress/expected/prepared_xacts.out
index 328cd74..1a6b4ce 100644
--- a/src/test/regress/expected/prepared_xacts.out
+++ b/src/test/regress/expected/prepared_xacts.out
@@ -88,17 +88,17 @@ SELECT gid FROM pg_prepared_xacts;
BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
INSERT INTO pxtest1 VALUES ('fff');
--- This should fail, because the gid foo3 is already in use
-PREPARE TRANSACTION 'foo3';
-ERROR: transaction identifier "foo3" is already in use
SELECT * FROM pxtest1;
foobar
--------
aaa
ddd
-(2 rows)
+ fff
+(3 rows)
-ROLLBACK PREPARED 'foo3';
+-- This should fail, because the gid foo3 is already in use
+PREPARE TRANSACTION 'foo3';
+ERROR: transaction identifier "foo3" is already in use
SELECT * FROM pxtest1;
foobar
--------
@@ -106,24 +106,7 @@ SELECT * FROM pxtest1;
ddd
(2 rows)
--- Test serialization failure (SSI)
-BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
-UPDATE pxtest1 SET foobar = 'eee' WHERE foobar = 'ddd';
-SELECT * FROM pxtest1;
- foobar
---------
- aaa
- eee
-(2 rows)
-
-PREPARE TRANSACTION 'foo4';
-SELECT gid FROM pg_prepared_xacts;
- gid
-------
- foo4
-(1 row)
-
-BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
+ROLLBACK PREPARED 'foo3';
SELECT * FROM pxtest1;
foobar
--------
@@ -131,24 +114,6 @@ 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';
-ERROR: could not serialize access due to read/write dependencies among transactions
-DETAIL: Cancelled on commit attempt with conflict in from prepared pivot.
-HINT: The transaction might succeed if retried.
-SELECT gid FROM pg_prepared_xacts;
- gid
-------
- foo4
-(1 row)
-
-ROLLBACK PREPARED 'foo4';
-SELECT gid FROM pg_prepared_xacts;
- gid
------
-(0 rows)
-
-- Clean up
DROP TABLE pxtest1;
-- Test subtransactions
diff --git a/src/test/regress/sql/prepared_xacts.sql b/src/test/regress/sql/prepared_xacts.sql
index e06c9d4..2bdbb0d 100644
--- a/src/test/regress/sql/prepared_xacts.sql
+++ b/src/test/regress/sql/prepared_xacts.sql
@@ -54,6 +54,7 @@ SELECT gid FROM pg_prepared_xacts;
BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
INSERT INTO pxtest1 VALUES ('fff');
+SELECT * FROM pxtest1;
-- This should fail, because the gid foo3 is already in use
PREPARE TRANSACTION 'foo3';
@@ -64,27 +65,6 @@ ROLLBACK PREPARED 'foo3';
SELECT * FROM pxtest1;
--- Test serialization failure (SSI)
-BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
-UPDATE pxtest1 SET foobar = 'eee' WHERE foobar = 'ddd';
-SELECT * FROM pxtest1;
-PREPARE TRANSACTION 'foo4';
-
-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
-PREPARE TRANSACTION 'foo5';
-
-SELECT gid FROM pg_prepared_xacts;
-
-ROLLBACK PREPARED 'foo4';
-
-SELECT gid FROM pg_prepared_xacts;
-
-- Clean up
DROP TABLE pxtest1;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
