Avoid uselessly looking up old LOCK_ONLY multixacts

Commit 0ac5ad5134f2 removed an optimization in multixact.c that skipped
fetching members of MultiXactId that were older than our
OldestVisibleMXactId value.  The reason this was removed is that it is
possible for multixacts that contain updates to be older than that
value.  However, if the caller is certain that the multi does not
contain an update (because the infomask bits say so), it can pass this
info down to GetMultiXactIdMembers, enabling it to use the old
optimization.

Pointed out by Andres Freund in 20131121200517.GM7240@alap2.anarazel.de

diff --git a/contrib/pgrowlocks/pgrowlocks.c b/contrib/pgrowlocks/pgrowlocks.c
index 7b8c9fbb326dfdd0bd9edb0ee79a15a2c987b491..36974f716c4602d69543c37796661a84be76fb34 100644 (file)
--- a/contrib/pgrowlocks/pgrowlocks.c
+++ b/contrib/pgrowlocks/pgrowlocks.c
@@ -165,7 +165,8 @@ pgrowlocks(PG_FUNCTION_ARGS)
 
                allow_old = !(infomask & HEAP_LOCK_MASK) &&
                    (infomask & HEAP_XMAX_LOCK_ONLY);
-               nmembers = GetMultiXactIdMembers(xmax, &members, allow_old);
+               nmembers = GetMultiXactIdMembers(xmax, &members, allow_old,
+                                                false);
                if (nmembers == -1)
                {
                    values[Atnum_xids] = "{0}";

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 0524f2efdf3b95c9d8dacd1a8b7214269dbfdae0..21e76d6a2a91edc2fcc836ff2bd4d3f42dc997ad 100644 (file)
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -4183,7 +4183,9 @@ l3:
             * the case, HeapTupleSatisfiesUpdate would have returned
             * MayBeUpdated and we wouldn't be here.
             */
-           nmembers = GetMultiXactIdMembers(xwait, &members, false);
+           nmembers =
+               GetMultiXactIdMembers(xwait, &members, false,
+                                     HEAP_XMAX_IS_LOCKED_ONLY(infomask));
 
            for (i = 0; i < nmembers; i++)
            {
@@ -4204,7 +4206,8 @@ l3:
                }
            }
 
-           pfree(members);
+           if (members)
+               pfree(members);
        }
 
        /*
@@ -4353,7 +4356,9 @@ l3:
                 * been the case, HeapTupleSatisfiesUpdate would have returned
                 * MayBeUpdated and we wouldn't be here.
                 */
-               nmembers = GetMultiXactIdMembers(xwait, &members, false);
+               nmembers =
+                   GetMultiXactIdMembers(xwait, &members, false,
+                                         HEAP_XMAX_IS_LOCKED_ONLY(infomask));
 
                if (nmembers <= 0)
                {
@@ -4834,7 +4839,7 @@ l5:
         * MultiXactIdExpand if we weren't to do this, so this check is not
         * incurring extra work anyhow.
         */
-       if (!MultiXactIdIsRunning(xmax))
+       if (!MultiXactIdIsRunning(xmax, HEAP_XMAX_IS_LOCKED_ONLY(old_infomask)))
        {
            if (HEAP_XMAX_IS_LOCKED_ONLY(old_infomask) ||
                TransactionIdDidAbort(MultiXactIdGetUpdateXid(xmax,
@@ -5175,7 +5180,8 @@ l4:
                int         i;
                MultiXactMember *members;
 
-               nmembers = GetMultiXactIdMembers(rawxmax, &members, false);
+               nmembers = GetMultiXactIdMembers(rawxmax, &members, false,
+                                    HEAP_XMAX_IS_LOCKED_ONLY(old_infomask));
                for (i = 0; i < nmembers; i++)
                {
                    HTSU_Result res;
@@ -5533,7 +5539,8 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
         */
        Assert((!(t_infomask & HEAP_LOCK_MASK) &&
                HEAP_XMAX_IS_LOCKED_ONLY(t_infomask)) ||
-              !MultiXactIdIsRunning(multi));
+              !MultiXactIdIsRunning(multi,
+                                    HEAP_XMAX_IS_LOCKED_ONLY(t_infomask)));
        if (HEAP_XMAX_IS_LOCKED_ONLY(t_infomask))
        {
            *flags |= FRM_INVALIDATE_XMAX;
@@ -5576,7 +5583,9 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
 
    allow_old = !(t_infomask & HEAP_LOCK_MASK) &&
        HEAP_XMAX_IS_LOCKED_ONLY(t_infomask);
-   nmembers = GetMultiXactIdMembers(multi, &members, allow_old);
+   nmembers =
+       GetMultiXactIdMembers(multi, &members, allow_old,
+                             HEAP_XMAX_IS_LOCKED_ONLY(t_infomask));
    if (nmembers <= 0)
    {
        /* Nothing worth keeping */
@@ -5983,7 +5992,7 @@ GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask,
     * We only use this in multis we just created, so they cannot be values
     * pre-pg_upgrade.
     */
-   nmembers = GetMultiXactIdMembers(multi, &members, false);
+   nmembers = GetMultiXactIdMembers(multi, &members, false, false);
 
    for (i = 0; i < nmembers; i++)
    {
@@ -6062,7 +6071,7 @@ MultiXactIdGetUpdateXid(TransactionId xmax, uint16 t_infomask)
     * Since we know the LOCK_ONLY bit is not set, this cannot be a multi from
     * pre-pg_upgrade.
     */
-   nmembers = GetMultiXactIdMembers(xmax, &members, false);
+   nmembers = GetMultiXactIdMembers(xmax, &members, false, false);
 
    if (nmembers > 0)
    {
@@ -6148,7 +6157,8 @@ Do_MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
    int         remain = 0;
 
    allow_old = !(infomask & HEAP_LOCK_MASK) && HEAP_XMAX_IS_LOCKED_ONLY(infomask);
-   nmembers = GetMultiXactIdMembers(multi, &members, allow_old);
+   nmembers = GetMultiXactIdMembers(multi, &members, allow_old,
+                                    HEAP_XMAX_IS_LOCKED_ONLY(infomask));
 
    if (nmembers >= 0)
    {
@@ -6294,7 +6304,8 @@ heap_tuple_needs_freeze(HeapTupleHeader tuple, TransactionId cutoff_xid,
 
            allow_old = !(tuple->t_infomask & HEAP_LOCK_MASK) &&
                HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask);
-           nmembers = GetMultiXactIdMembers(multi, &members, allow_old);
+           nmembers = GetMultiXactIdMembers(multi, &members, allow_old,
+                               HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask));
 
            for (i = 0; i < nmembers; i++)
            {

diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 9f259bb54ebb22ec6d941a4105d3c7fceb855aee..33346c76643dd91eda9096f1af666bfae6eef6f2 100644 (file)
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -428,7 +428,7 @@ MultiXactIdExpand(MultiXactId multi, TransactionId xid, MultiXactStatus status)
     * caller of this function does a check that the multixact is no longer
     * running.
     */
-   nmembers = GetMultiXactIdMembers(multi, &members, false);
+   nmembers = GetMultiXactIdMembers(multi, &members, false, false);
 
    if (nmembers < 0)
    {
@@ -517,7 +517,7 @@ MultiXactIdExpand(MultiXactId multi, TransactionId xid, MultiXactStatus status)
  * a pg_upgraded share-locked tuple.
  */
 bool
-MultiXactIdIsRunning(MultiXactId multi)
+MultiXactIdIsRunning(MultiXactId multi, bool isLockOnly)
 {
    MultiXactMember *members;
    int         nmembers;
@@ -529,7 +529,7 @@ MultiXactIdIsRunning(MultiXactId multi)
     * "false" here means we assume our callers have checked that the given
     * multi cannot possibly come from a pg_upgraded database.
     */
-   nmembers = GetMultiXactIdMembers(multi, &members, false);
+   nmembers = GetMultiXactIdMembers(multi, &members, false, isLockOnly);
 
    if (nmembers < 0)
    {
@@ -1095,10 +1095,15 @@ GetNewMultiXactId(int nmembers, MultiXactOffset *offset)
  * the value currently known as the next to assign, raise an error.  Previously
  * these also returned -1, but since this can lead to the wrong visibility
  * results, it is dangerous to do that.
+ *
+ * onlyLock must be set to true if caller is certain that the given multi
+ * is used only to lock tuples; can be false without loss of correctness,
+ * but passing a true means we can return quickly without checking for
+ * old updates.
  */
 int
 GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
-                     bool allow_old)
+                     bool allow_old, bool onlyLock)
 {
    int         pageno;
    int         prev_pageno;
@@ -1132,6 +1137,19 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
    /* Set our OldestVisibleMXactId[] entry if we didn't already */
    MultiXactIdSetOldestVisible();
 
+   /*
+    * If we know the multi is used only for locking and not for updates,
+    * then we can skip checking if the value is older than our oldest
+    * visible multi.  It cannot possibly still be running.
+    */
+   if (onlyLock &&
+       MultiXactIdPrecedes(multi, OldestVisibleMXactId[MyBackendId]))
+   {
+       debug_elog2(DEBUG2, "GetMembers: a locker-only multi is too old");
+       *members = NULL;
+       return -1;
+   }
+
    /*
     * We check known limits on MultiXact before resorting to the SLRU area.
     *
@@ -1335,7 +1353,7 @@ MultiXactHasRunningRemoteMembers(MultiXactId multi)
    int         nmembers;
    int         i;
 
-   nmembers = GetMultiXactIdMembers(multi, &members, true);
+   nmembers = GetMultiXactIdMembers(multi, &members, true, false);
    if (nmembers <= 0)
        return false;
 
@@ -2807,7 +2825,8 @@ pg_get_multixact_members(PG_FUNCTION_ARGS)
 
        multi = palloc(sizeof(mxact));
        /* no need to allow for old values here */
-       multi->nmembers = GetMultiXactIdMembers(mxid, &multi->members, false);
+       multi->nmembers = GetMultiXactIdMembers(mxid, &multi->members, false,
+                                               false);
        multi->iter = 0;
 
        tupdesc = CreateTemplateTupleDesc(2, false);

diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c
index 96874ab80a7882f5869ba514530494ed252af832..5c3f5adb4a7834c13c0ee38e6432d0efa4d8b09e 100644 (file)
--- a/src/backend/utils/time/tqual.c
+++ b/src/backend/utils/time/tqual.c
@@ -607,7 +607,7 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
             */
            if ((tuple->t_infomask & (HEAP_XMAX_EXCL_LOCK |
                                      HEAP_XMAX_KEYSHR_LOCK)) &&
-               MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple)))
+               MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), true))
                return HeapTupleBeingUpdated;
 
            SetHintBits(tuple, buffer, HEAP_XMAX_INVALID, InvalidTransactionId);
@@ -615,6 +615,11 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
        }
 
        xmax = HeapTupleGetUpdateXid(tuple);
+       if (!TransactionIdIsValid(xmax))
+       {
+           if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), false))
+               return HeapTupleBeingUpdated;
+       }
 
        /* not LOCKED_ONLY, so it has to have an xmax */
        Assert(TransactionIdIsValid(xmax));
@@ -627,7 +632,7 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
                return HeapTupleInvisible;      /* updated before scan started */
        }
 
-       if (TransactionIdIsInProgress(xmax))
+       if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), false))
            return HeapTupleBeingUpdated;
 
        if (TransactionIdDidCommit(xmax))
@@ -638,7 +643,7 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
         * what about the other members?
         */
 
-       if (!MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple)))
+       if (!MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), false))
        {
            /*
             * There's no member, even just a locker, alive anymore, so we can
@@ -1240,7 +1245,8 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin,
                 */
                if ((tuple->t_infomask & (HEAP_XMAX_EXCL_LOCK |
                                          HEAP_XMAX_KEYSHR_LOCK)) &&
-                   MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple)))
+                   MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple),
+                                        true))
                    return HEAPTUPLE_LIVE;
                SetHintBits(tuple, buffer, HEAP_XMAX_INVALID, InvalidTransactionId);
 
@@ -1267,7 +1273,7 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin,
    {
        TransactionId xmax;
 
-       if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple)))
+       if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), false))
        {
            /* already checked above */
            Assert(!HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask));

diff --git a/src/include/access/multixact.h b/src/include/access/multixact.h
index f6d2e0418b191fad2e0d098062b1e5af3827223f..8db773b1b5860c1d2521beda7ea4a376ef24e362 100644 (file)
--- a/src/include/access/multixact.h
+++ b/src/include/access/multixact.h
@@ -91,10 +91,10 @@ extern MultiXactId MultiXactIdCreateFromMembers(int nmembers,
                             MultiXactMember *members);
 
 extern MultiXactId ReadNextMultiXactId(void);
-extern bool MultiXactIdIsRunning(MultiXactId multi);
+extern bool MultiXactIdIsRunning(MultiXactId multi, bool isLockOnly);
 extern void MultiXactIdSetOldestMember(void);
 extern int GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **xids,
-                     bool allow_old);
+                     bool allow_old, bool isLockOnly);
 extern bool MultiXactHasRunningRemoteMembers(MultiXactId multi);
 extern bool MultiXactIdPrecedes(MultiXactId multi1, MultiXactId multi2);
 extern bool MultiXactIdPrecedesOrEquals(MultiXactId multi1,