Re: New statistics for tuning WAL buffer size

On 2020-10-02 10:21, Fujii Masao wrote:
> On 2020/10/01 13:35, Fujii Masao wrote:
>> 
>> 
>> On 2020/10/01 12:56, Masahiro Ikeda wrote:
>>> On 2020-10-01 11:33, Amit Kapila wrote:
>>>> On Thu, Oct 1, 2020 at 6:53 AM Kyotaro Horiguchi
>>>> <horikyota.ntt@gmail.com> wrote:
>>>>> 
>>>>> At Thu, 1 Oct 2020 09:05:19 +0900, Fujii Masao 
>>>>> <masao.fujii@oss.nttdata.com> wrote in
>>>>> >
>>>>> >
>>>>> > On 2020/09/30 20:21, Amit Kapila wrote:
>>>>> > > On Tue, Sep 29, 2020 at 9:23 PM Fujii Masao
>>>>> > > <masao.fujii@oss.nttdata.com> wrote:
>>>>> > >>
>>>>> > >> 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:
>>>>> > >>> 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
>>>>> > >>
>>>>> > > Okay, it makes sense to keep it in the current form if we have a plan
>>>>> > > to extend this view with additional stats. However, why don't we
>>>>> > > expose it with a function similar to pg_stat_get_archiver() instead of
>>>>> > > providing individual functions like pg_stat_get_wal_buffers_full() and
>>>>> > > pg_stat_get_wal_stat_reset_time?
>>>>> >
>>>>> > We can adopt either of those approaches for pg_stat_wal. I think that
>>>>> > the former is a bit more flexible because we can collect only one of
>>>>> > WAL information even when pg_stat_wal will contain many information
>>>>> > in the future, by using the function. But you thought there are some
>>>>> > reasons that the latter is better for pg_stat_wal?
>>>>> 
>>>>> FWIW I prefer to expose it by one SRF function rather than by
>>>>> subdivided functions.  One of the reasons is the less oid 
>>>>> consumption
>>>>> and/or reduction of definitions for intrinsic functions.
>>>>> 
>>>>> Another reason is at least for me subdivided functions are not 
>>>>> useful
>>>>> so much for on-the-fly examination on psql console.  I'm often 
>>>>> annoyed
>>>>> by realizing I can't recall the exact name of a function, say,
>>>>> pg_last_wal_receive_lsn or such but function names cannot be
>>>>> auto-completed on psql console. "select proname from pg_proc where
>>>>> proname like.. " is one of my friends:p On the other hand "select *
>>>>> from pg_stat_wal" requires no detailed memory.
>>>>> 
>>>>> However subdivided functions might be useful if I wanted use just 
>>>>> one
>>>>> number of wal-stats in a function, I think it is not a major usage 
>>>>> and
>>>>> we can use a SQL query on the view instead.
>>>>> 
>>>>> Another reason that I mildly want to object to subdivided functions 
>>>>> is
>>>>> I was annoyed that a stats view makes many individual calls to
>>>>> functions that internally share the same statistics entry.  That
>>>>> behavior required me to provide an entry-caching feature to my
>>>>> shared-memory statistics patch.
>>>>> 
>>>> 
>>>> All these are good reasons to expose it via one function and I think
>> 
>> Understood. +1 to expose it as one function.
>> 
>> 
>>>> that is why most of our existing views also use one function 
>>>> approach.
>>> 
>>> Thanks for your comments.
>>> I didn't notice there are the above disadvantages to provide 
>>> individual functions.
>>> 
>>> I changed the latest patch to expose it via one function.
>> 
>> Thanks for updating the patch! LGTM.
>> Barring any other objection, I will commit it.
> 
> I updated typedefs.list and pushed the patch. Thanks!

Thanks to all reviewers!

Regards,
-- 
Masahiro Ikeda
NTT DATA CORPORATION