Thread: Locks release order in LogStandbySnapshot
Hi, hackers
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?
Does there any sense to release them in reversed acquisition order in
LogStandbySnapshot like ProcArrayRemove?
--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 7db86f7885..28d1c152bf 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -1283,6 +1283,9 @@ LogStandbySnapshot(void)
*/
running = GetRunningTransactionData();
+ /* GetRunningTransactionData() acquired XidGenLock, we must release it */
+ LWLockRelease(XidGenLock);
+
/*
* GetRunningTransactionData() acquired ProcArrayLock, we must release it.
* For Hot Standby this can be done before inserting the WAL record
@@ -1304,9 +1307,6 @@ LogStandbySnapshot(void)
if (wal_level >= WAL_LEVEL_LOGICAL)
LWLockRelease(ProcArrayLock);
- /* GetRunningTransactionData() acquired XidGenLock, we must release it */
- LWLockRelease(XidGenLock);
-
return recptr;
}
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
On Wed, 09 Nov 2022 at 11:21, Andres Freund <andres@anarazel.de> wrote: > 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. > Thanks for the explanation! Got it. -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd.