Re: Fix checkpoint skip logic on idle systems by tracking LSN progress

On Fri, Dec 2, 2016 at 9:50 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Wed, Nov 30, 2016 at 7:53 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>> + * Switch segment only when WAL has done some progress since the
>> + * > last time a segment has switched because of a timeout.
>>
>>> + if (GetProgressRecPtr() > last_switch_lsn)
>>
>> Either the above comment is wrong or the code after it has a problem.
>> last_switch_lsn aka XLogCtl->lastSegSwitchLSN is updated not only for
>> a timeout but also when there is a lot of WAL activity which makes WAL
>> Write to cross a segment boundary.
>
> Right, this should be reworded a bit better to mention both. Done as attached.
>

+ * Switch segment only when WAL has done some progress since the
+ * last time a segment has switched because of a timeout or because
+ * of some WAL activity.

I think it could be better written as below, but it is up to you to
retain your version or use below one.

Switch segment only when WAL has done some progress since the last
time a segment has switched due to timeout or WAL activity. Apart
from that patch looks good to me.

Note to Committer: As discussed above [1], this patch skips logging
for LogAccessExclusiveLocks which can be called from multiple places,
so for clarity purpose either we should document it or skip it only
when absolutely necessary.

[1] - https://www.postgresql.org/message-id/CAA4eK1KJAXA3PdxH4T1QJKBNOvyUK8UKm_GCvTuT%2BFC5jpjmjg%40mail.gmail.com

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com