Avoid acquiring spinlock when checking if recovery has finished, for speed.
authorHeikki Linnakangas
Fri, 22 Nov 2013 10:53:59 +0000 (12:53 +0200)
committerHeikki Linnakangas
Fri, 22 Nov 2013 11:07:23 +0000 (13:07 +0200)
RecoveryIsInProgress() can be called very frequently. During normal
operation, it just checks a backend-local variable and returns quickly,
but during hot standby, it checks a spinlock-protected shared variable.
Those spinlock acquisitions can become a point of contention on a busy
hot standby system.

Replace the spinlock acquisition with a memory barrier.

Per discussion with Andres Freund, Ants Aasma and Merlin Moncure.

src/backend/access/transam/xlog.c [changed mode: 0644->0755]

old mode 100644 (file)
new mode 100755 (executable)
index a95149b..de19d22
@@ -7367,13 +7367,13 @@ RecoveryInProgress(void)
        return false;
    else
    {
-       /* use volatile pointer to prevent code rearrangement */
+       /*
+        * use volatile pointer to make sure we make a fresh read of the
+        * shared variable.
+        */
        volatile XLogCtlData *xlogctl = XLogCtl;
 
-       /* spinlock is essential on machines with weak memory ordering! */
-       SpinLockAcquire(&xlogctl->info_lck);
        LocalRecoveryInProgress = xlogctl->SharedRecoveryInProgress;
-       SpinLockRelease(&xlogctl->info_lck);
 
        /*
         * Initialize TimeLineID and RedoRecPtr when we discover that recovery
@@ -7382,7 +7382,20 @@ RecoveryInProgress(void)
         * this, see also LocalSetXLogInsertAllowed.)
         */
        if (!LocalRecoveryInProgress)
+       {
+           /*
+            * If we just exited recovery, make sure we read TimeLineID and
+            * RedoRecPtr after SharedRecoveryInProgress (for machines with
+            * weak memory ordering).
+            */
+           pg_memory_barrier();
            InitXLOGAccess();
+       }
+       /*
+        * Note: We don't need a memory barrier when we're still in recovery.
+        * We might exit recovery immediately after return, so the caller
+        * can't rely on 'true' meaning that we're still in recovery anyway.
+        */
 
        return LocalRecoveryInProgress;
    }