Re: Add statistics to pg_stat_wal view for wal related parameter tuning

Thanks for your comments.

On 2020-12-22 09:39, Andres Freund wrote:
> Hi,
>
> On 2020-12-21 13:16:50 -0800, Andres Freund wrote:
>> On 2020-12-02 13:52:43 +0900, Fujii Masao wrote:
>> > Pushed. Thanks!
>>
>> Why are wal_records/fpi long, instead of uint64?
>> long wal_records; /* # of WAL records produced */
>> long wal_fpi; /* # of WAL full page images produced */
>> uint64 wal_bytes; /* size of WAL records produced */
>>
>> long is only 4 byte e.g. on windows, and it is entirely possible to
>> wrap
>> a 4 byte record counter. It's also somewhat weird that wal_bytes is
>> unsigned, but the others are signed?
>>
>> This is made doubly weird because on the SQL level you chose to make
>> wal_records, wal_fpi bigint. And wal_bytes numeric?

I'm sorry I don't know the reason.

The following thread is related to the patch and the type of wal_bytes
is changed from long to uint64 because XLogRecPtr is uint64.
https://www.postgresql.org/message-id/flat/20200402144438.GF64485%40nol#1f0127c98df430104c63426fdc285c20

I assumed that the reason why the type of wal_records/fpi is long
is BufferUsage have the members (i.e, shared_blks_hit) of the same
types.

So, I think it's better if to change the type of wal_records/fpi from
long to uint64,
to change the types of BufferUsage's members too.

> Some more things:
> - There's both PgStat_MsgWal WalStats; and static PgStat_WalStats
> walStats;
> that seems *WAY* too confusing. And the former imo shouldn't be
> global.

Sorry for the confusing name.
I referenced the following variable name.

static PgStat_MsgSLRU SLRUStats[SLRU_NUM_ELEMENTS];
static PgStat_SLRUStats slruStats[SLRU_NUM_ELEMENTS];

How about to change from "PgStat_MsgWal WalStats"
to "PgStat_MsgWal MsgWalStats"?

Is it better to change the following name too?
"PgStat_MsgBgWriter BgWriterStats;"
"static PgStat_MsgSLRU SLRUStats[SLRU_NUM_ELEMENTS];"

Since PgStat_MsgWal is called in xlog.c and pgstat.c,
I thought it's should be global.

> - AdvanceXLInsertBuffer() does WalStats.m_wal_buffers_full, but as far
> as I can tell there's nothing actually sending that?

IIUC, when pgstat_send_wal() is called by backends and so on,
it is sent to the statistic collector and it is exposed via pg_stat_wal
view.

Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION