Advance the stop point for multixact offset creation only at checkpoint.

Commit b69bf30b9bfacafc733a9ba77c9587cf54d06c0c advanced the stop point
at vacuum time, but this has subsequently been shown to be unsafe as a
result of analysis by myself and Thomas Munro and testing by Thomas
Munro.  The crux of the problem is that the SLRU deletion logic may
get confused about what to remove if, at exactly the right time during
the checkpoint process, the head of the SLRU crosses what used to be
the tail.

This patch, by me, fixes the problem by advancing the stop point only
following a checkpoint.  This has the additional advantage of making
the removal logic work during recovery more like the way it works during
normal running, which is probably good.

At least one of the calls to DetermineSafeOldestOffset which this patch
removes was already dead, because MultiXactAdvanceOldest is called only
during recovery and DetermineSafeOldestOffset was set up to do nothing
during recovery.  That, however, is inconsistent with the principle that
recovery and normal running should work similarly, and was confusing to
boot.

Along the way, fix some comments that previous patches in this area
neglected to update.  It's not clear to me whether there's any
concrete basis for the decision to use only half of the multixact ID
space, but it's neither necessary nor sufficient to prevent multixact
member wraparound, so the comments should not say otherwise.

diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 7ec7f525d1e7a1c24281c567c191b668fa00cf14..9a26f072fe48778a88e271c4d92c11a99f40027f 100644 (file)
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -2062,8 +2062,6 @@ TrimMultiXact(void)
        }
 
        LWLockRelease(MultiXactMemberControlLock);
-
-       DetermineSafeOldestOffset(MultiXactState->oldestMultiXactId);
 }
 
 /*
@@ -2167,13 +2165,11 @@ SetMultiXactIdLimit(MultiXactId oldest_datminmxid, Oid oldest_datoid)
        Assert(MultiXactIdIsValid(oldest_datminmxid));
 
        /*
-        * Since multixacts wrap differently from transaction IDs, this logic is
-        * not entirely correct: in some scenarios we could go for longer than 2
-        * billion multixacts without seeing any data loss, and in some others we
-        * could get in trouble before that if the new pg_multixact/members data
-        * stomps on the previous cycle's data.  For lack of a better mechanism we
-        * use the same logic as for transaction IDs, that is, start taking action
-        * halfway around the oldest potentially-existing multixact.
+        * We pretend that a wrap will happen halfway through the multixact ID
+        * space, but that's not really true, because multixacts wrap differently
+        * from transaction IDs.  Note that, separately from any concern about
+        * multixact IDs wrapping, we must ensure that multixact members do not
+        * wrap.  Limits for that are set in DetermineSafeOldestOffset, not here.
         */
        multiWrapLimit = oldest_datminmxid + (MaxMultiXactId >> 1);
        if (multiWrapLimit < FirstMultiXactId)
@@ -2228,8 +2224,6 @@ SetMultiXactIdLimit(MultiXactId oldest_datminmxid, Oid oldest_datoid)
        curMulti = MultiXactState->nextMXact;
        LWLockRelease(MultiXactGenLock);
 
-       DetermineSafeOldestOffset(oldest_datminmxid);
-
        /* Log the info */
        ereport(DEBUG1,
         (errmsg("MultiXactId wrap limit is %u, limited by database with OID %u",
@@ -2324,8 +2318,6 @@ MultiXactAdvanceOldest(MultiXactId oldestMulti, Oid oldestMultiDB)
 {
        if (MultiXactIdPrecedes(MultiXactState->oldestMultiXactId, oldestMulti))
                SetMultiXactIdLimit(oldestMulti, oldestMultiDB);
-       else
-               DetermineSafeOldestOffset(oldestMulti);
 }
 
 /*
@@ -2503,19 +2495,11 @@ DetermineSafeOldestOffset(MultiXactId oldestMXact)
        MultiXactOffset oldestOffset;
 
        /*
-        * Can't do this while initdb'ing or in the startup process while
-        * replaying WAL: the segment file to read might have not yet been
-        * created, or already been removed.
-        */
-       if (IsBootstrapProcessingMode() || InRecovery)
-               return;
-
-       /*
-        * Determine the offset of the oldest multixact.  Normally, we can read
-        * the offset from the multixact itself, but there's an important special
-        * case: if there are no multixacts in existence at all, oldestMXact
-        * obviously can't point to one.  It will instead point to the multixact
-        * ID that will be assigned the next time one is needed.
+        * We determine the safe upper bound for offsets of new xacts by reading
+        * the offset of the oldest multixact, and going back one segment.  This
+        * way, the sequence of multixact member segments will always have a
+        * one-segment hole at a minimum.  We start spewing warnings a few
+        * complete segments before that.
         */
        LWLockAcquire(MultiXactGenLock, LW_SHARED);
        if (MultiXactState->nextMXact == oldestMXact)
@@ -2852,6 +2836,13 @@ TruncateMultiXact(void)
        SimpleLruTruncate(MultiXactOffsetCtl,
                                          MultiXactIdToOffsetPage(oldestMXact));
 
+       
+       /*
+        * Now, and only now, we can advance the stop point for multixact members.
+        * If we did it any sooner, the segments we deleted above might already
+        * have been overwritten with new members.  That would be bad.
+        */
+       DetermineSafeOldestOffset(oldestMXact);
 }
 
 /*