Re: race condition when writing pg_control

On Fri, May 29, 2020 at 12:54 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
wrote:

>
>
> 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.

Regards,
Amul