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

Andres Freund wrote:
> On 2013-12-11 22:08:41 -0300, Alvaro Herrera wrote:
> > Andres Freund wrote:

> > > I worry that this MultiXactIdSetOldestMember() will be problematic in
> > > longrunning vacuums like a anti-wraparound vacuum of a huge
> > > table. There's no real need to set MultiXactIdSetOldestMember() here,
> > > since we will not become the member of a multi. So I think you should
> > > either move the Assert() in MultiXactIdCreateFromMembers() to it's other
> > > callers, or add a parameter to skip it.
> >
> > I would like to have the Assert() work automatically, that is, check the
> > PROC_IN_VACUUM flag in MyProc->vacuumflags ... but this probably won't
> > work with CLUSTER. That said, I think we *should* call SetOldestMember
> > in CLUSTER. So maybe both things should be conditional on
> > PROC_IN_VACUUM.
>
> Why should it be dependent on cluster? SetOldestMember() defines the
> oldest multi we can be a member of. Even in cluster, the freezing will
> not make us a member of a multi. If the transaction does something else
> requiring SetOldestMember(), that will do it?

One last thing (I hope). It's not real easy to disable this check,
because it actually lives in GetNewMultiXactId. It would uglify the API
a lot if we were to pass a flag down two layers of routines; and moving
it to higher-level routines doesn't seem all that appropriate either.
I'm thinking we can have a new flag in MyPgXact->vacuumFlags, so
heap_prepare_freeze_tuple does this:

PG_TRY();
{
/* set flag to let multixact.c know what we're doing */
MyPgXact->vacuumFlags |= PROC_FREEZING_MULTI;
newxmax = FreezeMultiXactId(xid, tuple->t_infomask,
cutoff_xid, cutoff_multi, &flags);
}
PG_CATCH();
{
MyPgXact->vacuumFlags &= ~PROC_FREEZING_MULTI;
PG_RE_THROW();
}
PG_END_TRY();
MyPgXact->vacuumFlags &= ~PROC_FREEZING_MULTI;

and GetNewMultiXactId tests it to avoid the assert:

/*
* MultiXactIdSetOldestMember() must have been called already, but don't
* check while freezing MultiXactIds.
*/
Assert((MyPggXact->vacuumFlags & PROC_FREEZING_MULTI) ||
MultiXactIdIsValid(OldestMemberMXactId[MyBackendId]));

This avoids the API uglification issues, but introduces a setjmp call
for every tuple to be frozen. I don't think this is an excessive cost
to pay; after all, this is going to happen only for tuples for which
heap_tuple_needs_freeze already returned true; and for those we're
already going to do a lot of other work.

Attached is the whole series of patches for 9.3. (master is the same,
only with an additional patch that removes the legacy WAL record.)

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services