The rolled-back flag on serializable xacts was pointless and redundant with

the marked-for-death flag. It was only set for a fleeting moment while a
transaction was being cleaned up at rollback. All the places that checked
for the rolled-back flag should also check the marked-for-death flag, as
both flags mean that the transaction will roll back. I also renamed the
marked-for-death into "doomed", which is a lot shorter name.

diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 141d7ff20acd082703db6923dfc8c2cf30718f0a..a12f52ecc99c85f3baa76935ef8561f11f3e7d93 100644 (file)
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -244,9 +244,9 @@
 
 #define SxactIsOnFinishedList(sxact) (!SHMQueueIsDetached(&((sxact)->finishedLink)))
 
-#define SxactIsPrepared(sxact) (((sxact)->flags & SXACT_FLAG_PREPARED) != 0)
 #define SxactIsCommitted(sxact) (((sxact)->flags & SXACT_FLAG_COMMITTED) != 0)
-#define SxactIsRolledBack(sxact) (((sxact)->flags & SXACT_FLAG_ROLLED_BACK) != 0)
+#define SxactIsPrepared(sxact) (((sxact)->flags & SXACT_FLAG_PREPARED) != 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)
 #define SxactHasSummaryConflictOut(sxact) (((sxact)->flags & SXACT_FLAG_SUMMARY_CONFLICT_OUT) != 0)
@@ -259,7 +259,6 @@
 #define SxactIsDeferrableWaiting(sxact) (((sxact)->flags & SXACT_FLAG_DEFERRABLE_WAITING) != 0)
 #define SxactIsROSafe(sxact) (((sxact)->flags & SXACT_FLAG_RO_SAFE) != 0)
 #define SxactIsROUnsafe(sxact) (((sxact)->flags & SXACT_FLAG_RO_UNSAFE) != 0)
-#define SxactIsMarkedForDeath(sxact) (((sxact)->flags & SXACT_FLAG_MARKED_FOR_DEATH) != 0)
 
 /*
  * Compute the hash code associated with a PREDICATELOCKTARGETTAG.
@@ -609,8 +608,8 @@ RWConflictExists(const SERIALIZABLEXACT *reader, const SERIALIZABLEXACT *writer)
    Assert(reader != writer);
 
    /* Check the ends of the purported conflict first. */
-   if (SxactIsRolledBack(reader)
-       || SxactIsRolledBack(writer)
+   if (SxactIsDoomed(reader)
+       || SxactIsDoomed(writer)
        || SHMQueueEmpty(&reader->outConflicts)
        || SHMQueueEmpty(&writer->inConflicts))
        return false;
@@ -3048,7 +3047,7 @@ SetNewSxactGlobalXmin(void)
 
    for (sxact = FirstPredXact(); sxact != NULL; sxact = NextPredXact(sxact))
    {
-       if (!SxactIsRolledBack(sxact)
+       if (!SxactIsDoomed(sxact)
            && !SxactIsCommitted(sxact)
            && sxact != OldCommittedSxact)
        {
@@ -3113,7 +3112,7 @@ ReleasePredicateLocks(const bool isCommit)
    }
 
    Assert(!isCommit || SxactIsPrepared(MySerializableXact));
-   Assert(!SxactIsRolledBack(MySerializableXact));
+   Assert(!isCommit || !SxactIsDoomed(MySerializableXact));
    Assert(!SxactIsCommitted(MySerializableXact));
 
    /* may not be serializable during COMMIT/ROLLBACK PREPARED */
@@ -3153,9 +3152,7 @@ ReleasePredicateLocks(const bool isCommit)
            MySerializableXact->flags |= SXACT_FLAG_READ_ONLY;
    }
    else
-   {
-       MySerializableXact->flags |= SXACT_FLAG_ROLLED_BACK;
-   }
+       MySerializableXact->flags |= SXACT_FLAG_DOOMED;
 
    if (!topLevelIsDeclaredReadOnly)
    {
@@ -3531,7 +3528,7 @@ ReleaseOneSerializableXact(SERIALIZABLEXACT *sxact, bool partial,
                nextConflict;
 
    Assert(sxact != NULL);
-   Assert(SxactIsRolledBack(sxact) || SxactIsCommitted(sxact));
+   Assert(SxactIsDoomed(sxact) || SxactIsCommitted(sxact));
    Assert(LWLockHeldByMe(SerializableFinishedListLock));
 
    /*
@@ -3736,7 +3733,8 @@ CheckForSerializableConflictOut(const bool visible, const Relation relation,
    if (!SerializationNeededForRead(relation, snapshot))
        return;
 
-   if (SxactIsMarkedForDeath(MySerializableXact))
+   /* Check if someone else has already decided that we need to die */
+   if (SxactIsDoomed(MySerializableXact))
    {
        ereport(ERROR,
                (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
@@ -3850,11 +3848,9 @@ CheckForSerializableConflictOut(const bool visible, const Relation relation,
    }
    sxact = sxid->myXact;
    Assert(TransactionIdEquals(sxact->topXid, xid));
-   if (sxact == MySerializableXact
-       || SxactIsRolledBack(sxact)
-       || SxactIsMarkedForDeath(sxact))
+   if (sxact == MySerializableXact || SxactIsDoomed(sxact))
    {
-       /* We can't conflict with our own transaction or one rolled back. */
+       /* Can't conflict with ourself or a transaction that will roll back. */
        LWLockRelease(SerializableXactHashLock);
        return;
    }
@@ -3869,7 +3865,7 @@ CheckForSerializableConflictOut(const bool visible, const Relation relation,
    {
        if (!SxactIsPrepared(sxact))
        {
-           sxact->flags |= SXACT_FLAG_MARKED_FOR_DEATH;
+           sxact->flags |= SXACT_FLAG_DOOMED;
            LWLockRelease(SerializableXactHashLock);
            return;
        }
@@ -3996,7 +3992,7 @@ CheckTargetForConflictsIn(PREDICATELOCKTARGETTAG *targettag)
                mypredlocktag = predlock->tag;
            }
        }
-       else if (!SxactIsRolledBack(sxact)
+       else if (!SxactIsDoomed(sxact)
                 && (!SxactIsCommitted(sxact)
                     || TransactionIdPrecedes(GetTransactionSnapshot()->xmin,
                                              sxact->finishedBefore))
@@ -4009,7 +4005,7 @@ CheckTargetForConflictsIn(PREDICATELOCKTARGETTAG *targettag)
             * Re-check after getting exclusive lock because the other
             * transaction may have flagged a conflict.
             */
-           if (!SxactIsRolledBack(sxact)
+           if (!SxactIsDoomed(sxact)
                && (!SxactIsCommitted(sxact)
                    || TransactionIdPrecedes(GetTransactionSnapshot()->xmin,
                                             sxact->finishedBefore))
@@ -4113,7 +4109,8 @@ CheckForSerializableConflictIn(const Relation relation, const HeapTuple tuple,
    if (!SerializationNeededForWrite(relation))
        return;
 
-   if (SxactIsMarkedForDeath(MySerializableXact))
+   /* Check if someone else has already decided that we need to die */
+   if (SxactIsDoomed(MySerializableXact))
        ereport(ERROR,
                (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
                 errmsg("could not serialize access due to read/write dependencies among transactions"),
@@ -4417,7 +4414,7 @@ OnConflict_CheckForSerializationFailure(const SERIALIZABLEXACT *reader,
        {
            SERIALIZABLEXACT *t0 = conflict->sxactOut;
 
-           if (!SxactIsRolledBack(t0)
+           if (!SxactIsDoomed(t0)
                && (!SxactIsCommitted(t0)
                    || t0->commitSeqNo >= writer->commitSeqNo)
                && (!SxactIsReadOnly(t0)
@@ -4464,7 +4461,7 @@ OnConflict_CheckForSerializationFailure(const SERIALIZABLEXACT *reader,
                     errdetail("Cancelled on conflict out to pivot %u, during read.", writer->topXid),
                     errhint("The transaction might succeed if retried.")));
        }
-       writer->flags |= SXACT_FLAG_MARKED_FOR_DEATH;
+       writer->flags |= SXACT_FLAG_DOOMED;
    }
 }
 
@@ -4496,7 +4493,8 @@ PreCommit_CheckForSerializationFailure(void)
 
    LWLockAcquire(SerializableXactHashLock, LW_EXCLUSIVE);
 
-   if (SxactIsMarkedForDeath(MySerializableXact))
+   /* Check if someone else has already decided that we need to die */
+   if (SxactIsDoomed(MySerializableXact))
    {
        LWLockRelease(SerializableXactHashLock);
        ereport(ERROR,
@@ -4513,8 +4511,7 @@ PreCommit_CheckForSerializationFailure(void)
    while (nearConflict)
    {
        if (!SxactIsCommitted(nearConflict->sxactOut)
-           && !SxactIsRolledBack(nearConflict->sxactOut)
-           && !SxactIsMarkedForDeath(nearConflict->sxactOut))
+           && !SxactIsDoomed(nearConflict->sxactOut))
        {
            RWConflict  farConflict;
 
@@ -4527,10 +4524,9 @@ PreCommit_CheckForSerializationFailure(void)
                if (farConflict->sxactOut == MySerializableXact
                    || (!SxactIsCommitted(farConflict->sxactOut)
                        && !SxactIsReadOnly(farConflict->sxactOut)
-                       && !SxactIsRolledBack(farConflict->sxactOut)
-                       && !SxactIsMarkedForDeath(farConflict->sxactOut)))
+                       && !SxactIsDoomed(farConflict->sxactOut)))
                {
-                   nearConflict->sxactOut->flags |= SXACT_FLAG_MARKED_FOR_DEATH;
+                   nearConflict->sxactOut->flags |= SXACT_FLAG_DOOMED;
                    break;
                }
                farConflict = (RWConflict)

diff --git a/src/include/storage/predicate_internals.h b/src/include/storage/predicate_internals.h
index da6e641f4e4ec0f18e11e7f1024c1a37cdd345a7..b305c6a4c797c9b61e355510f10256a5320c9492 100644 (file)
--- 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_ROLLED_BACK             0x00000001
-#define SXACT_FLAG_COMMITTED               0x00000002
+#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                0x00000004
-#define SXACT_FLAG_READ_ONLY               0x00000008
-#define SXACT_FLAG_MARKED_FOR_DEATH            0x00000010
+#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
-#define SXACT_FLAG_PREPARED                    0x00000400
 
 /*
  * The following types are used to provide an ad hoc list for holding