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

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.

regards, tom lane