Small comment fixes and enhancements.

diff --git a/src/backend/storage/lmgr/README-SSI b/src/backend/storage/lmgr/README-SSI
index c079e38b90327bd87c8c5f783ea7c56a3f0eb1ff..c685beef4a4166b2f1cdeca04511d6ce9ac4783e 100644 (file)
--- a/src/backend/storage/lmgr/README-SSI
+++ b/src/backend/storage/lmgr/README-SSI
@@ -389,7 +389,7 @@ locks.
 
     * Because PostgreSQL does not have the same concept of an "oldest
 transaction ID" for all serializable transactions as assumed in the
-Cahill these, we track the oldest snapshot xmin among serializable
+Cahill thesis, we track the oldest snapshot xmin among serializable
 transactions, and a count of how many active transactions use that
 xmin. When the count hits zero we find the new oldest xmin and run a
 clean-up based on that.

diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 8b1a3336ca3dcf196f08831d2253fb6ae32d0ccf..d5cb7da1b0d41f27729612fd0f1e87e8aab0fba8 100644 (file)
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -703,8 +703,8 @@ OldSerXidInit(void)
         * Set up SLRU management of the pg_serial data.
         */
        OldSerXidSlruCtl->PagePrecedes = OldSerXidPagePrecedesLogically;
-       SimpleLruInit(OldSerXidSlruCtl, "OldSerXid SLRU Ctl", NUM_OLDSERXID_BUFFERS, 0,
-                                 OldSerXidLock, "pg_serial");
+       SimpleLruInit(OldSerXidSlruCtl, "OldSerXid SLRU Ctl",
+                                 NUM_OLDSERXID_BUFFERS, 0, OldSerXidLock, "pg_serial");
        /* Override default assumption that writes should be fsync'd */
        OldSerXidSlruCtl->do_fsync = false;
 
@@ -2954,7 +2954,8 @@ PredicateLockPageCombine(const Relation relation, const BlockNumber oldblkno,
 }
 
 /*
- * Walk the hash table and find the new xmin.
+ * Walk the list of in-progress serializable transactions and find the new
+ * xmin.
  */
 static void
 SetNewSxactGlobalXmin(void)
@@ -3126,6 +3127,10 @@ ReleasePredicateLocks(const bool isCommit)
                && !SxactIsReadOnly(MySerializableXact)
                && SxactHasSummaryConflictOut(MySerializableXact))
        {
+               /*
+                * we don't know which old committed transaction we conflicted with,
+                * so be conservative and use FirstNormalSerCommitSeqNo here
+                */
                MySerializableXact->SeqNo.earliestOutConflictCommit =
                        FirstNormalSerCommitSeqNo;
                MySerializableXact->flags |= SXACT_FLAG_CONFLICT_OUT;
@@ -3320,7 +3325,8 @@ ReleasePredicateLocksIfROSafe(void)
 }
 
 /*
- * Clear old predicate locks.
+ * Clear old predicate locks, belonging to committed transactions that are no
+ * longer interesting to any in-progress transaction.
  */
 static void
 ClearOldPredicateLocks(void)
@@ -3328,6 +3334,10 @@ ClearOldPredicateLocks(void)
        SERIALIZABLEXACT *finishedSxact;
        PREDICATELOCK *predlock;
 
+       /*
+        * Loop through finished transactions. They are in commit order, so we can
+        * stop as soon as we find one that's still interesting.
+        */
        LWLockAcquire(SerializableFinishedListLock, LW_EXCLUSIVE);
        finishedSxact = (SERIALIZABLEXACT *)
                SHMQueueNext(FinishedSerializableTransactions,
@@ -3346,6 +3356,10 @@ ClearOldPredicateLocks(void)
                        || TransactionIdPrecedesOrEquals(finishedSxact->finishedBefore,
                                                                                         PredXact->SxactGlobalXmin))
                {
+                       /*
+                        * This transaction committed before any in-progress transaction
+                        * took its snapshot. It's no longer interesting.
+                        */
                        LWLockRelease(SerializableXactHashLock);
                        SHMQueueDelete(&(finishedSxact->finishedLink));
                        ReleaseOneSerializableXact(finishedSxact, false, false);
@@ -3362,7 +3376,10 @@ ClearOldPredicateLocks(void)
                        LWLockAcquire(SerializableXactHashLock, LW_SHARED);
                }
                else
+               {
+                       /* Still interesting. */
                        break;
+               }
                finishedSxact = nextSxact;
        }
        LWLockRelease(SerializableXactHashLock);
@@ -3391,17 +3408,19 @@ ClearOldPredicateLocks(void)
                canDoPartialCleanup = (predlock->commitSeqNo <= PredXact->CanPartialClearThrough);
                LWLockRelease(SerializableXactHashLock);
 
+               /*
+                * If this lock originally belonged to an old enough transaction, we
+                * can release it.
+                */
                if (canDoPartialCleanup)
                {
                        PREDICATELOCKTAG tag;
-                       SHM_QUEUE  *targetLink;
                        PREDICATELOCKTARGET *target;
                        PREDICATELOCKTARGETTAG targettag;
                        uint32          targettaghash;
                        LWLockId        partitionLock;
 
                        tag = predlock->tag;
-                       targetLink = &(predlock->targetLink);
                        target = tag.myTarget;
                        targettag = target->tag;
                        targettaghash = PredicateLockTargetTagHashCode(&targettag);
@@ -3409,7 +3428,7 @@ ClearOldPredicateLocks(void)
 
                        LWLockAcquire(partitionLock, LW_EXCLUSIVE);
 
-                       SHMQueueDelete(targetLink);
+                       SHMQueueDelete(&(predlock->targetLink));
                        SHMQueueDelete(&(predlock->xactLink));
 
                        hash_search_with_hash_value(PredicateLockHash, &tag,
@@ -3437,9 +3456,9 @@ ClearOldPredicateLocks(void)
  * delete the transaction.
  *
  * When the partial flag is set, we can release all predicate locks and
- * out-conflict information -- we've established that there are no longer
+ * in-conflict information -- we've established that there are no longer
  * any overlapping read write transactions for which this transaction could
- * matter.
+ * matter -- but keep the transaction entry itself and any outConflicts.
  *
  * When the summarize flag is set, we've run short of room for sxact data
  * and must summarize to the SLRU.     Predicate locks are transferred to a
@@ -3460,6 +3479,10 @@ ReleaseOneSerializableXact(SERIALIZABLEXACT *sxact, bool partial,
        Assert(SxactIsRolledBack(sxact) || SxactIsCommitted(sxact));
        Assert(LWLockHeldByMe(SerializableFinishedListLock));
 
+       /*
+        * First release all the predicate locks held by this xact (or transfer
+        * them to OldCommittedSxact if summarize is true)
+        */
        LWLockAcquire(SerializablePredicateLockListLock, LW_SHARED);
        predlock = (PREDICATELOCK *)
                SHMQueueNext(&(sxact->predicateLocks),
@@ -3545,9 +3568,9 @@ ReleaseOneSerializableXact(SERIALIZABLEXACT *sxact, bool partial,
        sxidtag.xid = sxact->topXid;
        LWLockAcquire(SerializableXactHashLock, LW_EXCLUSIVE);
 
+       /* Release all outConflicts (unless 'partial' is true) */
        if (!partial)
        {
-               /* Release all outConflicts. */
                conflict = (RWConflict)
                        SHMQueueNext(&sxact->outConflicts,
                                                 &sxact->outConflicts,
@@ -3582,9 +3605,9 @@ ReleaseOneSerializableXact(SERIALIZABLEXACT *sxact, bool partial,
                conflict = nextConflict;
        }
 
+       /* Finally, get rid of the xid and the record of the transaction itself. */
        if (!partial)
        {
-               /* Get rid of the xid and the record of the transaction itself. */
                if (sxidtag.xid != InvalidTransactionId)
                        hash_search(SerializableXidHash, &sxidtag, HASH_REMOVE, NULL);
                ReleasePredXact(sxact);
@@ -3844,7 +3867,8 @@ CheckForSerializableConflictOut(const bool visible, const Relation relation,
 }
 
 /*
- * Check a particular target for rw-dependency conflict in.
+ * Check a particular target for rw-dependency conflict in. A subroutine of
+ * CheckForSerializableConflictIn().
  */
 static void
 CheckTargetForConflictsIn(PREDICATELOCKTARGETTAG *targettag)

diff --git a/src/include/storage/predicate_internals.h b/src/include/storage/predicate_internals.h
index 56a01f0b916df50569e617d56d9680509d3de8c6..e73159554b380e0becf7265b06febf26ff83a013 100644 (file)
--- a/src/include/storage/predicate_internals.h
+++ b/src/include/storage/predicate_internals.h
@@ -134,7 +134,8 @@ typedef struct PredXactListData
        /*
         * These global variables are maintained when registering and cleaning up
         * serializable transactions.  They must be global across all backends,
-        * but are not needed outside the predicate.c source file.
+        * but are not needed outside the predicate.c source file. Protected
+        * by SerializableXactHashLock.
         */
        TransactionId SxactGlobalXmin;          /* global xmin for active serializable
                                                                                 * transactions */