From: | ikedamsh <ikedamsh(at)oss(dot)nttdata(dot)com> |
---|---|
To: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> |
Cc: | "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Li Japin <japinli(at)hotmail(dot)com>, kuroda(dot)hayato(at)fujitsu(dot)com |
Subject: | Re: About to add WAL write/fsync statistics to pg_stat_wal view |
Date: | 2021-03-22 11:25:45 |
Message-ID: | ced8789e-b2e0-3322-6b61-402c5e259eea@oss.nttdata.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
v20-0003-Makes-the-wal-receiver-report-WAL-statistics.patch | text/x-patch | 3.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2021-03-22 12:13:11 | Re: proposal - psql - use pager for \watch command |
Previous Message | Yugo NAGATA | 2021-03-22 10:29:36 | Re: Columns correlation and adaptive query optimization |