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

Hello, thank you for the comment.

At Sat, 12 Nov 2016 10:28:56 +0530, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote in <CAA4eK1K0gGQTBxCyKqi6QnqOWGzEoVVPHCgPJ_RkOBoLPejCTA(at)mail(dot)gmail(dot)com>
> On Tue, Nov 8, 2016 at 5:18 PM, Kyotaro HORIGUCHI
> <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> > Hello,
> >
> >> on something else than LW_EXCLUSIVE compared to now. To keep things
> >> more simple I' would still favor using WALInsertLocks for this patch,
> >> that looks more consistent, and also because there is no noticeable
> >> difference.
> >
> > Ok, the patch looks fine. So there's nothing for me but to accept
> > the current shape since the discussion about performance seems
> > not to be settled with out performance measurement with machines
> > with many cores.
> >
>
> I think it is good to check the performance impact of this patch on
> many core m/c. Is it possible for you to once check with Alexander
> Korotkov to see if he can provide you access to his powerful m/c which
> has 70 cores (if I remember correctly)?
>
>
> @@ -1035,6 +1038,7 @@ LogAccessExclusiveLocks(int nlocks,
> xl_standby_lock *locks)
> XLogBeginInsert();
> XLogRegisterData((char *) &xlrec, offsetof(xl_standby_locks, locks));
> XLogRegisterData((char *) locks, nlocks * sizeof(xl_standby_lock));
> + XLogSetFlags(XLOG_NO_PROGRESS);
>
>
> Is it right to set XLOG_NO_PROGRESS flag in LogAccessExclusiveLocks?
> This function is called not only in LogStandbySnapshot(), but during
> DDL operations as well where lockmode >= AccessExclusiveLock.

This does not remove any record from WAL. So theoretically any
kind of record can be NO_PROGRESS, but practically as long as
checkpoints are not unreasonably suppressed. Any explicit
database operation must be accompanied with at least commit
record that triggers checkpoint. NO_PROGRESSing there doesn't
seem to me to harm database durability for this reason.

The objective of this patch is skipping WALs on completely-idle
state and the NO_PROGRESSing is necessary to do its work. Of
course we can distinguish exclusive lock with PROGRESS and
without PROGRESS but it is unnecessary complexity.

But rethinking about the above, the namging of "XLOG_NO_PROGRESS"
might be inappropriate. "XLOG_NO_CKPT_TRIGGER" or any sainer name
would be needed.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center