From: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
---|---|
To: | Andres Freund <andres(at)2ndquadrant(dot)com> |
Cc: | Daniel Wood <dwood(at)salesforce(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: 9.3 reference constraint regression |
Date: | 2013-12-17 16:08:34 |
Message-ID: | 20131217160834.GN12902@eldon.alvh.no-ip.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Andres Freund wrote:
> On 2013-12-16 17:43:37 -0300, Alvaro Herrera wrote:
> > Alvaro Herrera wrote:
> >
> > > This POC patch changes the two places in HeapTupleSatisfiesUpdate that
> > > need to be touched for this to work. This is probably too simplistic,
> > > in that I make the involved cases return HeapTupleBeingUpdated without
> > > checking that there actually are remote lockers, which is the case of
> > > concern. I'm not yet sure if this is the final form of the fix, or
> > > instead we should expand the Multi (in the cases where there is a multi)
> > > and verify whether any lockers are transactions other than the current
> > > one. As is, nothing seems to break, but I think that's probably just
> > > chance and should not be relied upon.
> >
> > After playing with this, I think the reason this seems to work without
> > fail is that all callers of HeapTupleSatisfiesUpdate are already
> > prepared to deal with the case where HeapTupleBeingUpdated is returned
> > but there is no actual transaction that would block the operation.
> > So I think the proposed patch is okay, barring a few more comments.
>
> Are you sure? the various wait/nowait cases don't seem to handle that
> correctly.
Well, it would help if those cases weren't dead code. Neither
heap_update nor heap_delete are ever called in the "no wait" case at
all. Only heap_lock_tuple is, and I can't see any misbehavior there
either, even with HeapTupleBeingUpdated returned when there's a
non-local locker, or when there's a MultiXact as xmax, regardless of its
status.
Don't get me wrong --- it's not like this case is all that difficult to
handle. All that's required is something like this in
HeapTupleSatisfiesUpdate:
if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(tuple)))
{
...
if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask)) /* not deleter */
{
if (tuple->t_infomask & HEAP_XMAX_IS_MULTI)
{
int nmembers;
bool remote;
int i;
MultiXactMember *members;
nmembers =
GetMultiXactIdMembers(HeapTupleHeaderGetRawXmax(tuple),
&members, false);
remote = false;
for (i = 0; i < nmembers; i++)
{
if (!TransactionIdIsCurrentTransactionId(members[i].xid))
{
remote = true;
break;
}
}
if (nmembers > 0)
pfree(members);
if (remote)
return HeapTupleBeingUpdated;
else
return HeapTupleMayBeUpdated;
}
else if (!TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetRawXmax(tuple)))
return HeapTupleBeingUpdated;
return HeapTupleMayBeUpdated;
}
}
The simpler code just does away with the GetMultiXactIdMembers() and
returns HeapTupleBeingUpdated always. In absence of a test case that
misbehaves with that, it's hard to see that it is a good idea to go all
this effort there.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Stehule | 2013-12-17 16:11:08 | Re: patch: make_timestamp function |
Previous Message | Jim Nasby | 2013-12-17 16:02:12 | Re: pg_rewarm status |