On 2020-Nov-16, Alvaro Herrera wrote: > On 2020-Nov-09, Tom Lane wrote: > > > Michael Paquier <mich...@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.
