On Thu, Dec 10, 2015 at 9:04 AM, Andres Freund <andres@anarazel.de> wrote:
> On 2015-12-10 08:55:54 -0500, Robert Haas wrote:
>> Maybe. But I think we could use a little more vigorous discussion of
>> that issue, since Andres doesn't seem to be very convinced by your
>> analysis, and I don't currently understand what you've fixed because I
>> can't, as mentioned several times, follow your patch stack.
>
> The issue at hand is that the following block
> oldestOffsetKnown =
> find_multixact_start(oldestMultiXactId, &oldestOffset);
>
> ...
> else if (prevOldestOffsetKnown)
> {
> /*
> * If we failed to get the oldest offset this time, but we have a
> * value from a previous pass through this function, use the old value
> * rather than automatically forcing it.
> */
> oldestOffset = prevOldestOffset;
> oldestOffsetKnown = true;
> }
> in SetOffsetVacuumLimit() fails to restore offsetStopLimit, which then
> is set in shared memory:
> /* Install the computed values */
> LWLockAcquire(MultiXactGenLock, LW_EXCLUSIVE);
> MultiXactState->oldestOffset = oldestOffset;
> MultiXactState->oldestOffsetKnown = oldestOffsetKnown;
> MultiXactState->offsetStopLimit = offsetStopLimit;
> LWLockRelease(MultiXactGenLock);
>
> so, if find_multixact_start() failed - a "should never happen" occurance
> - we install a wrong stop limit. It does get 'repaired' upon the next
> suceeding find_multixact_start() in SetOffsetVacuumLimit() or a restart
> though.
>
> Adding a 'prevOffsetStopLimit' and using it seems like a ~5 line patch.
So let's do that, then.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company