On 2020/05/27 16:10, Michael Paquier wrote: > On Tue, May 26, 2020 at 07:30:54PM +0000, Bossart, Nathan wrote: >> While an assertion in UpdateControlFile() would not have helped us >> catch the problem I initially reported, it does seem worthwhile to add >> it. I have attached a patch that adds this assertion and also >> attempts to fix XLogReportParameters(). Since there is only one place >> where we feel it is safe to call UpdateControlFile() without a lock, I >> just changed it to take the lock. I don't think this adds any sort of >> significant contention risk, and IMO it is a bit cleaner than the >> boolean flag. > > Let's see what Fujii-san and Thomas think about that. I'd rather > avoid taking a lock here because we don't need it and because it makes > things IMO confusing with the beginning of StartupXLOG() where a lot > of the fields are read, even if we go without this extra assertion.
I have no strong opinion about this, but I tend to agree with Michael here.
I too don't have a strong opinion about this either, but I like Nathan's approach more, just take the lock in the startup process as well for the simplicity if that is not hurting much. I think, apart from the startup process we have to take the lock to update the control file, then having separate treatment for the startup process looks confusing to me, IMHO.
