Thomas Munro wrote:
> > Doesn't this heap_lock_tuple() need to check for a WouldBlock result?
> > Not sure that this is the same case that you were trying to test in
> > heap_lock_updated_tuple; I think this requires an update chain (so that
> > EPQFetch is invoked) and some tuple later in the chain is locked by a
> > third transaction.
>
> You're right, that case was clearly lacking. The new patch, attached,
> releases the buffer and returns NULL in that case. But I have no idea
> how to reach it in an isolation test! skip-locked-4.spec gets pretty
> close, it creates the right kind of update chain to get into
> EvalPlanQualFetch and then enter the conditional block beginning:
>
> if (TransactionIdIsValid(SnapshotDirty.xmax))
>
> But to reach the case you mentioned, it would need to get past that
> (xmax is not a valid transaction) but then the tuple would need to be
> locked by another session before heap_lock_tuple is called a few lines
> below. That's a race scenario that I don't believe we can create
> using advisory lock tricks in an isolation test.
Hm, are you able to reproduce it using GDB?
Craig Ringer was saying elsewhere that there are other cases that are
impossible to test reliably and was proposing addings hooks or
something to block backends at convenient times. Not an easy problem ...
> > I attach some additional minor suggestions to your patch. Please feel
> > free to reword comments differently if you think my wording isn't an
> > improvements (or I've maked an english mistakes).
>
> Thanks, these are incorporated in the new version (also rebased).
Great, thanks; I'll look at it again soon to commit, as I think we're
done now.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services