Re: Online base backup from the hot-standby

>
> > > > ERROR: full_page_writes on master is set invalid at least once since
> > > > latest checkpoint
> > > >
> > > > I think this error should be rewritten as
> > > > ERROR: full_page_writes on master has been off at some point since
> > > > latest checkpoint
> > > >
> > > > We should be using 'off' instead of 'invalid' since that is what is what
> > > > the user sets it to.
> > >
> > > Sure.
> >
> > What about the following message? It sounds more precise to me.
> >
> > ERROR: WAL generated with full_page_writes=off was replayed since last
> > restartpoint
>
> Okay, I changes the patch to this messages.
> If someone says there is a idea better than it, I will consider again.
>
>
> > > I updated to patch corresponded above-comments.
> >
> > Thanks for updating the patch! Here are the comments:
> >
> > * don't yet have the insert lock, forcePageWrites could change under us,
> > * but we'll recheck it once we have the lock.
> > */
> > - doPageWrites = fullPageWrites || Insert->forcePageWrites;
> > + doPageWrites = Insert->fullPageWrites || Insert->forcePageWrites;
> >
> > The source comment needs to be modified.
> >
> > * just turned off, we could recompute the record without full pages, but
> > * we choose not to bother.)
> > */
> > - if (Insert->forcePageWrites && !doPageWrites)
> > + if ((Insert->fullPageWrites || Insert->forcePageWrites) && !doPageWrites)
> >
> > Same as above.
>
> Sure.
>
>
> > XLogReportParameters() should skip writing WAL if full_page_writes has not been
> > changed by SIGHUP.
> >
> > XLogReportParameters() should skip updating pg_control if any parameter related
> > to hot standby has not been changed.
>
> YES.
>
>
> > In checkpoint, XLogReportParameters() is called only when wal_level is
> > hot_standby.
> > OTOH, in walwriter, it's always called even when wal_level is not hot_standby.
> > Can't we skip calling XLogReportParameters() whenever wal_level is not
> > hot_standby?
>
> Yes, It is possible.
>
>
> > In do_pg_start_backup() and do_pg_stop_backup(), the spinlock must be held to
> > see XLogCtl->lastFpwDisabledLSN.
>
> Yes.
>
>
> > What about changing the error message to:
> > ERROR: WAL generated with full_page_writes=off was replayed during online backup
>
> Okay, too.

> Sorry.
> I was not previously able to answer fujii's all comments.
> This is the remaining answers.
>
>
> > + LWLockAcquire(WALInsertLock, LW_EXCLUSIVE);
> > + XLogCtl->Insert.fullPageWrites = fullPageWrites;
> > + LWLockRelease(WALInsertLock);
> >
> > I don't think WALInsertLock needs to be hold here because there is no
> > concurrently running process which can access Insert.fullPageWrites.
> > For example, Insert->currpos and Insert->LogwrtResult are also changed
> > without the lock there.
> >
>
> Yes.
>
> > The source comment of XLogReportParameters() needs to be modified.
>
> Yes, too.

Done.
I updated to patch corresponded above-comments.

Regards.

--------------------------------------------
Jun Ishizuka
NTT Software Corporation
TEL：045-317-7018
E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
--------------------------------------------