Re: SyncRepLock acquired exclusively in default configuration

[Fujii Masao]

On 2020/04/08 3:01, Ashwin Agrawal wrote:

On Mon, Apr 6, 2020 at 2:14 PM Andres Freund <and...@anarazel.de 
<mailto:and...@anarazel.de>> wrote:

     > How about we change it to this ?

    Hm. Better. But I think it might need at least a compiler barrier /
    volatile memory load?  Unlikely here, but otherwise the compiler could
    theoretically just stash the variable somewhere locally (it's not likely
    to be a problem because it'd not be long ago that we acquired an lwlock,
    which is a full barrier).


That's the part, I am not fully sure about. But reading the comment above 
SyncRepUpdateSyncStandbysDefined(), it seems fine.

     > Bring back the check which existed based on GUC but instead of just 
blindly
     > returning based on just GUC not being set, check
     > WalSndCtl->sync_standbys_defined. Thoughts?

    Hm. Is there any reason not to just check
    WalSndCtl->sync_standbys_defined? rather than both !SyncStandbysDefined()
    and WalSndCtl->sync_standbys_defined?


Agree, just checking for WalSndCtl->sync_standbys_defined seems fine.

So the consensus is something like the following? Patch attached.

        /*
-        * Fast exit if user has not requested sync replication.
+        * Fast exit if user has not requested sync replication, or there are no
+        * sync replication standby names defined.
         */
-       if (!SyncRepRequested())
+       if (!SyncRepRequested() ||
+               !((volatile WalSndCtlData *) WalSndCtl)->sync_standbys_defined)
                return;

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

diff --git a/src/backend/replication/syncrep.c 
b/src/backend/replication/syncrep.c
index ffd5b31eb2..c6df3f4da6 100644
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -163,9 +163,11 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit)
                mode = Min(SyncRepWaitMode, SYNC_REP_WAIT_FLUSH);
 
        /*
-        * Fast exit if user has not requested sync replication.
+        * Fast exit if user has not requested sync replication, or there are no
+        * sync replication standby names defined.
         */
-       if (!SyncRepRequested())
+       if (!SyncRepRequested() ||
+               !((volatile WalSndCtlData *) WalSndCtl)->sync_standbys_defined)
                return;
 
        Assert(SHMQueueIsDetached(&(MyProc->syncRepLinks)));