Hi, On 2022-11-09 11:03:04 +0800, Japin Li wrote: > GetRunningTransactionData requires holding both ProcArrayLock and > XidGenLock (in that order). Then LogStandbySnapshot releases those > locks in that order. However, to reduce the frequency of having to > wait for XidGenLock while holding ProcArrayLock, ProcArrayAdd releases > them in reversed acquisition order. > > The comments of LogStandbySnapshot says: > > > GetRunningTransactionData() acquired ProcArrayLock, we must release it. > > For Hot Standby this can be done before inserting the WAL record > > because ProcArrayApplyRecoveryInfo() rechecks the commit status using > > the clog. For logical decoding, though, the lock can't be released > > early because the clog might be "in the future" from the POV of the > > historic snapshot. This would allow for situations where we're waiting > > for the end of a transaction listed in the xl_running_xacts record > > which, according to the WAL, has committed before the xl_running_xacts > > record. Fortunately this routine isn't executed frequently, and it's > > only a shared lock. > > This comment is only for ProcArrayLock, not for XidGenLock. IIUC, > LogCurrentRunningXacts doesn't need holding XidGenLock, right?
I think it does. If we allow xid assignment before LogCurrentRunningXacts() is done, those new xids would not have been mentioned in the xl_running_xacts record, despite already running. Which I think result in corrupted snapshots during hot standby and logical decoding. > Does there any sense to release them in reversed acquisition order in > LogStandbySnapshot like ProcArrayRemove? I don't think it's worth optimizing for, this happens at a low frequency (whereas connection establishment can be very frequent). And due to the above, we can sometimes release ProcArrayLock earlier. Greetings, Andres Freund