On 2020-Nov-16, Alvaro Herrera wrote:
> On 2020-Nov-09, Tom Lane wrote:
>
> > Michael Paquier <michael@paquier.xyz> writes:
> > >> + LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
> > >> + MyProc->vacuumFlags |= PROC_IN_SAFE_IC;
> > >> + ProcGlobal->vacuumFlags[MyProc->pgxactoff] = MyProc->vacuumFlags;
> > >> + LWLockRelease(ProcArrayLock);
> >
> > > I can't help noticing that you are repeating the same code pattern
> > > eight times. I think that this should be in its own routine, and that
> > > we had better document that this should be called just after starting
> > > a transaction, with an assertion enforcing that.
> >
> > Do we really need exclusive lock on the ProcArray to make this flag
> > change? That seems pretty bad from a concurrency standpoint.
>
> BTW I now know that the reason for taking ProcArrayLock is not the
> vacuumFlags itself, but rather MyProc->pgxactoff, which can move.
... ah, but I realize now that this means that we can use shared lock
here, not exclusive, which is already an enormous improvement. That's
because ->pgxactoff can only be changed with exclusive lock held; so as
long as we hold shared, the array item cannot move.