Re: wal stats questions

Thanks for many your suggestions!
I made the patch to handle the issues.

> 1) What is the motivation to have both prevWalUsage and pgWalUsage,
>    instead of just accumulating the stats since the last submission?
>    There doesn't seem to be any comment explaining it? Computing
>    differences to previous values, and copying to prev*, isn't free. I
>    assume this is out of a fear that the stats could get reset before
>    they're used for instrument.c purposes - but who knows?

I removed the unnecessary code copying pgWalUsage and just reset the
pgWalUsage after reporting the stats in pgstat_report_wal().


> 2) Why is there both pgstat_send_wal() and pgstat_report_wal()? With the
>    former being used by wal writer, the latter by most other processes?
>    There again don't seem to be comments explaining this.

I added the comments why two functions are separated.
(But is it better to merge them?)


> 3) Doing if (memcmp(&WalStats, &all_zeroes, sizeof(PgStat_MsgWal)) == 0)
>    just to figure out if there's been any changes isn't all that
>    cheap. This is regularly exercised in read-only workloads too. Seems
>    adding a boolean WalStats.have_pending = true or such would be
>    better.
> 4) For plain backends pgstat_report_wal() is called by
>    pgstat_report_stat() - but it is not checked as part of the "Don't
>    expend a clock check if nothing to do" check at the top.  It's
>    probably rare to have pending wal stats without also passing one of
>    the other conditions, but ...

I added the logic to check if the stats counters are updated or not in
pgstat_report_stat() using not only generated wal record but also write/sync
counters, and it can skip to call reporting function.

Although I added the condition which the write/sync counters are updated or
not, I haven't understood the following case yet...Sorry. I checked related
code and tested to insert large object, but I couldn't. I'll investigate more
deeply, but if you already know the function which leads the following case,
please let me know.

> Maybe there is the case where a backend generates no WAL records,
> but just writes them because it needs to do write-ahead-logging
> when flush the table data?

> Ugh! I was missing a very large blob.. Ok, we need additional check
> for non-pgWalUsage part. Sorry.


Regards,

-- 
Masahiro Ikeda
NTT DATA CORPORATION