On Thu, Dec 10, 2015 at 9:32 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> 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.
Who is going to take care of this?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
