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

On Mon, Nov 14, 2016 at 12:49 PM, Kyotaro HORIGUCHI
<horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> 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>
>> 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)?

I heard about a number like that, and there is no reason to not do
tests to be sure. With that many cores we are more likely going to see
the limitation of the number of XLOG insert slots popping up as a
bottleneck, but that's just an assumption without any numbers.
Alexander (added in CC), could it be possible to get an access to this
machine?

>> @@ -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.

The point that applies here is that logging the exclusive lock
information is necessary for the *standby* recovery conflicts, not the
primary which is why it should not influence the checkpoint activity
that is happening on the primary. So marking this record with
NO_PROGRESS is actually fine to me.

> 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.

I got fond of NO_PROGRESS to be honest with the time, even if I don't
like much the negative meaning it has... Perhaps something like
XLOG_SKIP_PROGRESS would hold more meaning.
--
Michael