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
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 |
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 |