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