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


Reply via email to