Additional test for each commit in sync rep path to plug minute

possibility of race condition that would effect performance only.
Requested by Robert Haas. Re-arrange related comments.

diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
index e99b43d8e0d2c3e3084a9e5201893905da51ef63..17c255480eb47c227bf9b8391c78e39a32725f08 100644 (file)
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -114,21 +114,28 @@ SyncRepWaitForLSN(XLogRecPtr XactCommitLSN)
    /* Reset the latch before adding ourselves to the queue. */
    ResetLatch(&MyProc->waitLatch);
 
-   /*
-    * Set our waitLSN so WALSender will know when to wake us, and add
-    * ourselves to the queue.
-    */
    LWLockAcquire(SyncRepLock, LW_EXCLUSIVE);
    Assert(MyProc->syncRepState == SYNC_REP_NOT_WAITING);
-   if (!WalSndCtl->sync_standbys_defined)
+
+   /*
+    * We don't wait for sync rep if WalSndCtl->sync_standbys_defined is
+    * not set.  See SyncRepUpdateSyncStandbysDefined.
+    *
+    * Also check that the standby hasn't already replied. Unlikely
+    * race condition but we'll be fetching that cache line anyway
+    * so its likely to be a low cost check.
+    */
+   if (!WalSndCtl->sync_standbys_defined ||
+       XLByteLE(XactCommitLSN, WalSndCtl->lsn))
    {
-       /*
-        * We don't wait for sync rep if WalSndCtl->sync_standbys_defined is
-        * not set.  See SyncRepUpdateSyncStandbysDefined.
-        */
        LWLockRelease(SyncRepLock);
        return;
    }
+
+   /*
+    * Set our waitLSN so WALSender will know when to wake us, and add
+    * ourselves to the queue.
+    */
    MyProc->waitLSN = XactCommitLSN;
    MyProc->syncRepState = SYNC_REP_WAITING;
    SyncRepQueueInsert();