On Sat, Dec 12, 2015 at 12:02 PM, Andres Freund <andres@anarazel.de> wrote:
> Noah, Robert, All
>
> On 2015-12-11 11:20:21 -0500, Robert Haas wrote:
>> On Thu, Dec 10, 2015 at 9:32 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>> >> 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?
>
> Here's two patches:
>
> 1) The fix to SetOffsetVacuumLimit().
>
> I've tested this by introducing a probabilistic "return false;" to
> find_multixact_start(), and torturing postgres by burning through
> billions of multixactids of various sizes. Behaves about as
> bad^Wgood as without the induced failures; before the patch there
> were moments of spurious warnings/errors when ->offsetStopLimit was
> out of whack.
I find the commit message you wrote a little difficult to read, and
propose the following version instead, which reads better to me:
Previously, if find_multixact_start() failed, SetOffsetVacuumLimit()
would install 0 into MultiXactState->offsetStopLimit. Luckily, there
are no known cases where find_multixact_start() will return an error
in 9.5 and above. But if it were to happen, for example due to
filesystem permission issues, it'd be somewhat bad:
GetNewMultiXactId() could continue allocating mxids even if close to a
wraparound, or it could erroneously stop allocating mxids, even if no
wraparound is looming. The wrong value would be corrected the next
time SetOffsetVacuumLimit() is called, or by a restart.
(I have no comments on the substance of either patch and have reviewed
the first one to a negligible degree - it doesn't look obviously wrong
- and the second one not at all.)
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company