Re: SyncRepLock acquired exclusively in default configuration

On Mon, Apr 6, 2020 at 2:14 PM Andres Freund <andres(at)anarazel(dot)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.

I wasn't fully thinking there, as I got distracted by if lock will be
required or not for reading the same. If lock was required then checking
for guc first would have been better, but seems not required.