From: | Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com> |
---|---|
To: | "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> |
Cc: | 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-02-15 02:59:48 |
Message-ID: | 23a9f71817f616135551e2fde5ef362e@oss.nttdata.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2021-02-10 00:51, David G. Johnston wrote:
> On Thu, Feb 4, 2021 at 4:45 PM Masahiro Ikeda
> <ikedamsh(at)oss(dot)nttdata(dot)com> wrote:
>
>> I pgindented the patches.
>
> ... <function>XLogWrite</function>, which is invoked during an
> <function>XLogFlush</function> request (see ...). This is also
> incremented by the WAL receiver during replication.
>
> ("which normally called" should be "which is normally called" or
> "which normally is called" if you want to keep true to the original)
> You missed the adding the space before an opening parenthesis here and
> elsewhere (probably copy-paste)
>
> is ether -> is either
> "This parameter is off by default as it will repeatedly query the
> operating system..."
> ", because" -> "as"
Thanks, I fixed them.
> wal_write_time and the sync items also need the note: "This is also
> incremented by the WAL receiver during replication."
I skipped changing it since I separated the stats for the WAL receiver
in pg_stat_wal_receiver.
> "The number of times it happened..." -> " (the tally of this event is
> reported in wal_buffers_full in....) This is undesirable because ..."
Thanks, I fixed it.
> I notice that the patch for WAL receiver doesn't require explicitly
> computing the sync statistics but does require computing the write
> statistics. This is because of the presence of issue_xlog_fsync but
> absence of an equivalent pg_xlog_pwrite. Additionally, I observe that
> the XLogWrite code path calls pgstat_report_wait_*() while the WAL
> receiver path does not. It seems technically straight-forward to
> refactor here to avoid the almost-duplicated logic in the two places,
> though I suspect there may be a trade-off for not adding another
> function call to the stack given the importance of WAL processing
> (though that seems marginalized compared to the cost of actually
> writing the WAL). Or, as Fujii noted, go the other way and don't have
> any shared code between the two but instead implement the WAL receiver
> one to use pg_stat_wal_receiver instead. In either case, this
> half-and-half implementation seems undesirable.
OK, as Fujii-san mentioned, I separated the WAL receiver stats.
(v10-0002-Makes-the-wal-receiver-report-WAL-statistics.patch)
I added the infrastructure code to communicate the WAL receiver stats
messages between the WAL receiver and the stats collector, and
the stats for WAL receiver is counted in pg_stat_wal_receiver.
What do you think?
Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION
Attachment | Content-Type | Size |
---|---|---|
v10-0001-Add-statistics-related-to-write-sync-wal-records.patch | text/x-diff | 19.0 KB |
v10-0002-Makes-the-wal-receiver-report-WAL-statistics.patch | text/x-diff | 26.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2021-02-15 03:11:37 | Re: Some regular-expression performance hacking |
Previous Message | Michael Paquier | 2021-02-15 02:58:02 | Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly |