Re: About to add WAL write/fsync statistics to pg_stat_wal view
Masahiro Ikeda <ikedamsh@oss.nttdata.com>
From: ikedamsh <ikedamsh@oss.nttdata.com>
To: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: "David G. Johnston" <david.g.johnston@gmail.com>,
PostgreSQL-development <pgsql-hackers@postgresql.org>,
Masahiko Sawada <sawada.mshk@gmail.com>, Li Japin <japinli@hotmail.com>,
kuroda.hayato@fujitsu.com
Date: 2021-03-22T11:25:45Z
Lists: pgsql-hackers
Attachments
- v20-0003-Makes-the-wal-receiver-report-WAL-statistics.patch (text/x-patch) patch v20-0003
On 2021/03/22 16:50, Fujii Masao wrote: > > > On 2021/03/22 9:50, ikedamsh wrote: >> Agreed. I separated the patches. >> >> If only the former is committed, my trivial concern is that there may be >> a disadvantage, but no advantage for the standby server. It may lead to >> performance degradation to the wal receiver by calling >> INSTR_TIME_SET_CURRENT(), but the stats can't visible for users until the >> latter patch is committed. > > Your concern is valid, so let's polish and commit also the 0003 patch to v14. > I'm still thinking that it's better to separate wal_xxx columns into > walreceiver's and the others. But if we count even walreceiver activity on > the existing columns, regarding 0003 patch, we need to update the document? > For example, "Number of times WAL buffers were written out to disk via > XLogWrite request." should be "Number of times WAL buffers were written > out to disk via XLogWrite request and by WAL receiver process."? Maybe > we need to append some descriptions about this into "WAL configuration" > section? Agreed. Users can know whether the stats is for walreceiver or not. The pg_stat_wal view in standby server shows for the walreceiver, and in primary server it shows for the others. So, I updated the document. (v20-0003-Makes-the-wal-receiver-report-WAL-statistics.patch) >> I followed the argument of pg_pwrite(). >> But, I think "char *" is better, so fixed it. > > Thanks for updating the patch! > > +extern int XLogWriteFile(int fd, char *buf, > + size_t nbyte, off_t offset, > + TimeLineID timelineid, XLogSegNo segno, > + bool write_all); > > write_all seems not to be necessary. You added this flag for walreceiver, > I guess. But even without the argument, walreceiver seems to work expectedly. > So, what about the attached patch? I applied some cosmetic changes to the patch. Thanks a lot. Yes, "write_all" is unnecessary. Your patch is looks good to me. Regards, -- Masahiro Ikeda NTT DATA CORPORATION
Commits
-
Send statistics collected during shutdown checkpoint to the stats collector.
- b82640df0062 14.0 landed
-
Force to send remaining WAL stats to the stats collector at walwriter exit.
- 33394ee6f243 14.0 landed
-
Track total amounts of times spent writing and syncing WAL data to disk.
- ff99918c625a 14.0 landed
-
Retry short writes when flushing WAL.
- 79ce29c734c6 9.4.0 cited