On Fri, Apr 6, 2018 at 1:50 PM, Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> wrote: > Hello. > > At Wed, 04 Apr 2018 17:26:46 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI > <horiguchi.kyot...@lab.ntt.co.jp> wrote in > <20180404.172646.238325981.horiguchi.kyot...@lab.ntt.co.jp> >> > In general, I was wondering why in the first place this variable >> > (full_page_writes) is a SIGHUP variable? I think if the user tries to >> > switch it to 'on' from 'off', it won't guarantee the recovery from >> > torn pages. Yeah, one can turn it to 'off' from 'on' without any >> > problem, but as the reverse doesn't guarantee anything, it can confuse >> > users. What do you think? >> >> I tend to agree with you. It works as expected after the next >> checkpoint. So define the variable as "it can be changed any time >> but has an effect at the next checkpoint time", then remove >> XLOG_FPW_CHANGE record. And that eliminates the problem of >> concurrent calls since the checkpointer becomes the only modifier >> of the variable. And the problematic function >> UpdateFullPageWrites also no longer needs to write a WAL >> record. The information is conveyed only by checkpoint records. > > I noticed that XLOG_FPW_CHANGE(fpw=false) is still required by > pg_start/stop_backup to know FPW's turning-off without waiting > for the next checkpoint record. But XLOG_FPW_CHANGE(true) is not > required since no one uses the information. It seems even harmful > when it is written at the incorrect place. > > In the attached patch, shared fullPageWrites is updated only at > REDO point >
I am not completely sure if that is the right option because this would mean that we are defining the new scope for a GUC variable. I guess we should take the input of others as well. I am not sure what is the right way to do that, but maybe we can start a new thread with a proper subject and description rather than discussing this under some related bug fix patch discussion. I guess we can try that after CF unless some other people pitch in and share their feedback. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
