Re: Problem while setting the fpw with SIGHUP

Hello. Thanks to Heikkit for picking this up and thanks for the
commnet to Michael.

# The attached is changed only in a comment, and rebased.

At Thu, 12 Apr 2018 05:24:14 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote in <20180411202414(dot)GA32449(at)paquier(dot)xyz>
> On Wed, Apr 11, 2018 at 02:09:48PM +0300, Heikki Linnakangas wrote:
> > I think the new behavior where the GUC only takes effect at next checkpoint
> > is OK. It seems quite intuitive.
> >
> > > [rebased patch version]
> >
> > Looks good at a quick glance. Assuming no objections from others, I'll take
> > a closer look and commit tomorrow. Thanks!
>
> Sorry for not following up closely this thread lately.
>
> + /*
> + * If full_page_writes has been turned off, issue XLOG_FPW_CHANGE before
> + * the flag actually takes effect. No lock is required since checkpointer
> + * is the only updator of shared fullPageWrites after recovery is
> + * finished. Both shared and local fullPageWrites do not change before the
> + * next reading below.
> + */
> + if (Insert->fullPageWrites && !fullPageWrites)
> + {
> + XLogBeginInsert();
> + XLogRegisterData((char *) (&fullPageWrites), sizeof(bool));
> + XLogInsert(RM_XLOG_ID, XLOG_FPW_CHANGE);
> + }
>
> This is not actually true. If a fallback_promote is used, then
> CreateCheckPoint() is called by the startup process which is in charge
> of issuing the end-of-recovery checkpoint, and not the checkpointer. So
> I still fail to see how a no-lock approach is fine except if we remove
> fallback_promote?

Checkpointer never calls CreateCheckPoint while
RecoveryInProgress() == true. In other words, checkpointer is not
an updator of shared FPW at the time StartupXLOG calls
CreateCheckPoint for fallback_promote.

The comment may be somewhat confusing that it is written
there. The point is that checkpointer and StartupXLOG are
mutually excluded on updating shared FPW by
SharedRecoveryInProgress flag.

| * If full_page_writes has been turned off, issue XLOG_FPW_CHANGE before
| * the flag actually takes effect. Checkpointer never calls this function
| * before StartupXLOG() turns off SharedRecoveryInProgress so there's no
| * window where checkpointer and startup processes - the only updators of
| * the flag - can update shared FPW simultaneously. Thus no lock is
| * required here. Both shared and local fullPageWrites do not change
| * before the next reading below.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center