> > > > > 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....@po.ntts.co.jp --------------------------------------------
standby_online_backup_09base-04fpw.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
