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

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Noah Misch <noah(at)leadboat(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pgsql: Fix a couple of bugs in MultiXactId freezing
Date: 2013-12-13 20:08:46
Message-ID: 20131213200845.GG12902@eldon.alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

Andres Freund wrote:
> On 2013-12-12 18:24:34 -0300, Alvaro Herrera wrote:

> > + else if (TransactionIdIsValid(update_xid) && !has_lockers)
> > + {
> > + /*
> > + * If there's a single member and it's an update, pass it back alone
> > + * without creating a new Multi. (XXX we could do this when there's a
> > + * single remaining locker, too, but that would complicate the API too
> > + * much; moreover, the case with the single updater is more
> > + * interesting, because those are longer-lived.)
> > + */
> > + Assert(nnewmembers == 1);
> > + *flags |= FRM_RETURN_IS_XID;
> > + if (update_committed)
> > + *flags |= FRM_MARK_COMMITTED;
> > + xid = update_xid;
> > + }
>
> Afaics this will cause HEAP_KEYS_UPDATED to be reset, is that
> problematic? I don't really remember what it's needed for TBH...

So we do reset HEAP_KEYS_UPDATED, and in general that bit seems critical
for several things. So it should be kept when a Xmax is carried over
from the pre-frozen version of the tuple. But while reading through
that, I realize that we should also be keeping HEAP_HOT_UPDATED in that
case. And particularly we should never clear HEAP_ONLY_TUPLE.

So I think heap_execute_freeze_tuple() is wrong, because it's resetting
the whole infomask to zero, and then setting it to only the bits that
heap_prepare_freeze_tuple decided that it needed set. That seems bogus
to me. heap_execute_freeze_tuple() should only clear a certain number
of bits, and then possibly set some of the same bits; but the remaining
flags should remain untouched. So HEAP_KEYS_UPDATED, HEAP_UPDATED and
HEAP_HOT_UPDATED should be untouched by heap_execute_freeze_tuple;
heap_prepare_freeze_tuple needn't worry about querying those bits at
all.

Only when FreezeMultiXactId returns FRM_INVALIDATE_XMAX, and when the
Xmax is not a multi and it gets removed, should those three flags be
removed completely.

HEAP_ONLY_TUPLE should be untouched by the freezing protocol.

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

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Alvaro Herrera 2013-12-13 20:23:53 pgsql: Rework MultiXactId cache code
Previous Message Tom Lane 2013-12-13 19:06:11 pgsql: Add HOLD/RESUME_INTERRUPTS in HandleCatchupInterrupt/HandleNotif

Browse pgsql-hackers by date

  From Date Subject
Next Message Merlin Moncure 2013-12-13 21:49:45 Re: "stuck spinlock"
Previous Message Jim Nasby 2013-12-13 20:04:11 Re: patch: make_timestamp function