Avoid unnecessary acquisition of SyncRepLock in transaction commit time.

MasaoFujii · MasaoFujii · commit be9788e9989a · 2020-09-02T10:55:55.000+09:00
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 48c9f49, 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
diff --git 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);
 
