Hi, On 2020-11-17 12:55:01 -0300, Alvaro Herrera wrote: > ... 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.
Uh, wait a second. The acquisition of this lock hasn't been affected by the snapshot scalability changes, and therefore are unrelated to ->pgxactoff changing or not. In 13 this is: LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); MyPgXact->vacuumFlags |= PROC_IN_VACUUM; if (params->is_wraparound) MyPgXact->vacuumFlags |= PROC_VACUUM_FOR_WRAPAROUND; LWLockRelease(ProcArrayLock); Lowering this to a shared lock doesn't seem right, at least without a detailed comment explaining why it's safe. Because GetSnapshotData() etc look at all procs with just an LW_SHARED ProcArrayLock, changing vacuumFlags without a lock means that two concurrent horizon computations could come to a different result. I'm not saying it's definitely wrong to relax things here, but I'm not sure we've evaluated it sufficiently. Greetings, Andres Freund