From be9788e9989a0744ba60ab100153340fd123b786 Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Wed, 2 Sep 2020 10:55:55 +0900 Subject: [PATCH] Avoid unnecessary acquisition of SyncRepLock in transaction commit time. In SyncRepWaitForLSN() routine called in transaction commit time, SyncRepLock is necessary to atomically both check the shared sync_standbys_defined flag and operate the sync replication wait-queue. On the other hand, when the flag is false, the lock is not necessary because the wait-queue is not touched. But due to the changes by commit 48c9f49265, previously the lock was taken whatever the flag was. This could cause unnecessary performance overhead in every transaction commit time. Therefore this commit avoids that unnecessary aquisition of SyncRepLock. Author: Fujii Masao Reviewed-by: Asim Praveen, Masahiko Sawada, Discussion: https://postgr.es/m/20200406050332.nsscfqjzk2d57zyx@alap3.anarazel.de --- src/backend/replication/syncrep.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c index df1e341c76..6e8c76537a 100644 --- a/src/backend/replication/syncrep.c +++ b/src/backend/replication/syncrep.c @@ -158,18 +158,30 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit) */ Assert(InterruptHoldoffCount > 0); + /* + * Fast exit if user has not requested sync replication, or there are no + * sync replication standby names defined. + * + * Since this routine gets called every commit time, it's important to + * exit quickly if sync replication is not requested. So we check + * WalSndCtl->sync_standbys_defined flag without the lock and exit + * immediately if it's false. If it's true, we need to check it again later + * while holding the lock, to check the flag and operate the sync rep + * queue atomically. This is necessary to avoid the race condition + * described in SyncRepUpdateSyncStandbysDefined(). On the other + * hand, if it's false, the lock is not necessary because we don't touch + * the queue. + */ + if (!SyncRepRequested() || + !((volatile WalSndCtlData *) WalSndCtl)->sync_standbys_defined) + return; + /* Cap the level for anything other than commit to remote flush only. */ if (commit) mode = SyncRepWaitMode; else mode = Min(SyncRepWaitMode, SYNC_REP_WAIT_FLUSH); - /* - * Fast exit if user has not requested sync replication. - */ - if (!SyncRepRequested()) - return; - Assert(SHMQueueIsDetached(&(MyProc->syncRepLinks))); Assert(WalSndCtl != NULL);