Re: uninterruptable loop: concurrent delete in progress within table

On 2014-06-02 13:27:17 -0400, Alvaro Herrera wrote:
> Andres Freund wrote:
> > On 2014-06-02 12:48:01 -0400, Alvaro Herrera wrote:
> > > Andres Freund wrote:
> > >
> > > > I do wonder if any of the other existing callers of HTSV are affected. I
> > > > don't understand predicate.c well enough to be sure, but it looks to me
> > > > like it'd could in theory lead to missed conflicts. Seems fairly
> > > > unlikely to matter in practice though.
> > >
> > > One place where the difference in the new logic would cause a change is
> > > the tupleIsAlive bit in IndexBuildHeapScan(), when building unique
> > > indexes. Right now if the tuple is inserted by a remote running
> > > transaction, and updated by it, we return DELETE_IN_PROGRESS, so we set
> > > tupleIsAlive = false; so we wouldn't block if we see a tuple with that
> > > value elsewhere. If we instead return INSERT_IN_PROGRESS we set
> > > tupleIsAlive = true, and we would block until the inserter is done.
> >
> > Hm. Maybe I am missing something, but if either INSERT or
> > DELETE_IN_PROGRESS is returned for a unique index in
> > IndexBuildHeapScan() we'll wait for the xmin/xmax respectively and
> > recheck the tuple, right? That recheck will then return a non-ephemeral
> > status.
>
> Yeah, that's how I interpret that maze of little loops using callbacks.
> I don't think waiting on xmin is the same as waiting on xmax, however;
> the xmin could stay running for a while, but the xmax could be an
> quickly aborted subtransaction, for instance.

It essentially is. If xmax aborts you still have to deal with an
'INSERT_IN_PROGRESS' tuple because, as you say, xmin is still
running. That's essentially my point. INSERT_IN_PROGRESS isn't a wrong
answer.

> > > One thing I'm now wondering while reading this code is those cases where
> > > XMAX_INVALID is not set, but the value in Xmax is InvalidXid. We have
> > > discussed these scenarios elsewhere and the conclusion seems to be that
> > > they are rare but possible and therefore we should cater for them.
> >
> > Is there really a situation where that combination is possible for
> > XMAX_INVALID? I don't see how.
>
> I vaguely remember Robert described how would this work, even though the
> precise details evidently escape me because I can't quite trace it now.
> I can't remember what thread this was on, either.

Robert: Do you remember that case?

Alvaro: In the end it'd not be very harmful - if it happens
TransactionIdDidCommit() will return false (there's special case code
for it).

> Yeah, I'm not saying this is a new bug; I was only reacting to your
> suggestion that we dump all of tqual.c and rewrite it from scratch
> (heh).

Heh. Not going to start with that today. Especially not in a patch that
needs to be backpatched :)

Greetings,

Andres Freund

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