Re: New statistics for tuning WAL buffer size

On 2020/09/29 11:51, Masahiro Ikeda wrote:
> On 2020-09-29 11:43, Amit Kapila wrote:
>> On Tue, Sep 29, 2020 at 7:39 AM Masahiro Ikeda <ikedamsh@oss.nttdata.com> wrote:
>>>
>>> On 2020-09-28 12:43, Amit Kapila wrote:
>>> > On Mon, Sep 28, 2020 at 8:24 AM Kyotaro Horiguchi
>>> > <horikyota.ntt@gmail.com> wrote:
>>> >>
>>> >> At Mon, 28 Sep 2020 08:11:23 +0530, Amit Kapila
>>> >> <amit.kapila16@gmail.com> wrote in
>>> >> > One other thing that occurred to me today is can't we keep this as
>>> >> > part of PgStat_GlobalStats? We can use pg_stat_reset_shared('wal'); to
>>> >> > reset it. It seems to me this is a cluster-wide stats and somewhat
>>> >> > similar to some of the other stats we maintain there.
>>> >>
>>> >> I like that direction, but PgStat_GlobalStats is actually
>>> >> PgStat_BgWriterStats and cleard by a RESET_BGWRITER message.
>>> >>
>>> >
>>> > Yeah, I think if we want to pursue this direction then we probably
>>> > need to have a separate message to set/reset WAL-related stuff. I
>>> > guess we probably need to have a separate reset timestamp for WAL. I
>>> > think the difference would be that we can have one structure to refer
>>> > to global_stats instead of referring to multiple structures and we
>>> > don't need to issue separate read/write calls but OTOH I don't see
>>> > many disadvantages of the current approach as well.
>>>
>>> IIUC, if we keep wal stats as part of PgStat_GlobalStats,
>>> don't we need to add PgStat_ArchiverStats and PgStat_SLRUStats
>>> to PgStat_GlobalStats too?
>>>
>>
>> I have given the idea for wal_stats because there is just one counter
>> in that. I think you can just try to evaluate the merits of each
>> approach and choose whichever you feel is good. This is just a
>> suggestion, if you don't like it feel free to proceed with the current
>> approach.
> 
> Thanks for your suggestion.
> I understood that the point is that WAL-related stats have just one counter now.
> 
> Since we may add some WAL-related stats like pgWalUsage.(bytes, records, fpi),
> I think that the current approach is good.

+1

I marked this patch as ready for committer.
Barring any objection, I will commit the patch.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION