Re: pgsql: Fix race condition in snapshot caching when 2PC is used.

Hi,

On 2020-08-18 23:31:04 -0400, Tom Lane wrote:
> David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> >> On 2020-08-18 19:55:50 -0400, Tom Lane wrote:
> >>> On Wed, 19 Aug 2020 at 12:37, Andres Freund <andres(at)anarazel(dot)de> wrote:
> >>>> I'm inclined to just make ClearTransaction take an exclusive lock - the
> >>>> rest of the 2PC operations are so heavyweight that I can't imagine
> >>>> making a difference. When I tested the locking changes in
> >>>> ProcArrayAdd()/Remove() the more heavyweight locking wasn't at all
> >>>> visible.
>
> >>> I was wondering if it'd be sensible to convert that counter into an
> >>> atomic variable. That's not real clear, but worth thinking about.
>
> > Couldn't it be done by creating two inline functions, one to call to
> > atomically increment and the other to just increment? Can backup that
> > the correct version of the function is being called with an
> > Assert(LWLockHeldByMeInMode(ProcArrayLock, ...));
>
> On reflection I agree with Andres' thought that just taking the lock
> exclusively in ProcArrayClearTransaction is the right solution.
> It's silly to imagine that a 2PC commit (plus all the other stuff that
> needs to happen around that) is fast enough that that'll be a serious
> performance hit. Keeping things simple for the other code paths is
> a more useful goal.

Cool, pushed that way.

Greetings,

Andres Freund