Re: pgsql: Fix a couple of bugs in MultiXactId freezing

On 2013-12-03 09:16:18 -0500, Noah Misch wrote:
> On Tue, Dec 03, 2013 at 11:56:07AM +0100, Andres Freund wrote:
> > On 2013-12-03 00:47:07 -0500, Noah Misch wrote:
> > > On Sat, Nov 30, 2013 at 01:06:09AM +0000, Alvaro Herrera wrote:
> > Any idea how to cheat our way out of that one given the current way
> > heap_freeze_tuple() works (running on both primary and standby)? My only
> > idea was to MultiXactIdWait() if !InRecovery but that's extremly grotty.
> > We can't even realistically create a new multixact with fewer members
> > with the current format of xl_heap_freeze.
>
> Perhaps set HEAP_XMAX_LOCK_ONLY on the tuple? We'd then ensure all update XID
> consumers check HEAP_XMAX_IS_LOCKED_ONLY() first, much like xmax consumers are
> already expected to check HEAP_XMAX_INVALID first. Seems doable, albeit yet
> another injection of complexity.

I think its pretty much checked that way already, but the problem seems
to be how to avoid checks on xid commit/abort in that case. I've
complained in 20131121200517(dot)GM7240(at)alap2(dot)anarazel(dot)de that the old
pre-condition that multixacts aren't checked when they can't be relevant
(via OldestVisibleM*) isn't observed anymore.
So, if we re-introduce that condition again, we should be on the safe
side with that, right?

> > > The test spec additionally
> > > covers a (probably-related) assertion failure, new in 9.3.2.
> >
> > Too bad it's too late to do anthing about it for 9.3.2. :(. At least the
> > last seems actually unrelated, I am not sure why it's 9.3.2
> > only. Alvaro, are you looking?
>
> (For clarity, the other problem demonstrated by the test spec is also a 9.3.2
> regression.)

Yea, I just don't see why yet... Looking now.

> heap_freeze_tuple() of 9.2 had an XXX comment about the possibility of getting
> spurious lock contention due to wraparound of the multixact space. The
> comment is gone, and that mechanism no longer poses a threat. However, a
> non-wrapped multixact containing wrapped locker XIDs (we don't freeze locker
> XIDs, just updater XIDs) may cause similar spurious contention.

Yea, I noticed that that comment was missing as well. I think what we
should do now is to rework freezing in HEAD to make all this more
reasonable.

> Numerous comments in the vicinity (e.g. ones at MultiXactStateData) reflect a
> pre-9.3 world. Most or all of that isn't new with the patch at hand, but it
> does complicate study.

Yea, Alvaro sent a patch for that somewhere, it seems a patch in the
series got lost when foreign key locks were originally applied.

I think we seriously need to put a good amount of work into the
multixact.c stuff in the next months. Otherwise it will be a maintenance
nightmore for a fair bit more time.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services