From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
To: pgsql-hackers@postgresql.org
Cc: amit.kapila16@gmail.com, lchch1990@sina.cn, Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Subject: Re: Add statistics to pg_stat_wal view for wal related parameter tuning
Date: Fri, 30 Oct 2020 11:50:59 +0900
Message-ID: <04a75b35-c601-65e4-ec34-54dcd995ce11@oss.nttdata.com> (raw)
In-Reply-To: <1ddd39a1c6501239220ff55e12551586@oss.nttdata.com>
References: <06bc2d405b086a72fb14657de9befae8@oss.nttdata.com>
<20201021.180323.1892401452849353942.horikyota.ntt@gmail.com>
<729d83f87da2cfbca4343e577f2e20a1@oss.nttdata.com>
<20201022.135416.794530032338899030.horikyota.ntt@gmail.com>
<1ddd39a1c6501239220ff55e12551586@oss.nttdata.com>
On 2020/10/29 17:03, Masahiro Ikeda wrote:
> Hi,
>
> Thanks for your comments and advice. I updated the patch.
>
> On 2020-10-21 18:03, Kyotaro Horiguchi wrote:
>> At Tue, 20 Oct 2020 16:11:29 +0900, Masahiro Ikeda
>> <ikedamsh@oss.nttdata.com> wrote in
>>> On 2020-10-20 12:46, Amit Kapila wrote:
>>> > I see that we also need to add extra code to capture these stats (some
>>> > of which is in performance-critical path especially in
>>> > XLogInsertRecord) which again makes me a bit uncomfortable. It might
>>> > be that it is all fine as it is very important to collect these stats
>>> > at cluster-level in spite that the same information can be gathered at
>>> > statement-level to help customers but I don't see a very strong case
>>> > for that in your proposal.
>>
>> We should avoid that duplication as possible even if the both number
>> are important.
>>
>>> Also about performance, I thought there are few impacts because it
>>> increments stats in memory. If I can implement to reuse pgWalUsage's
>>> value which already collects these stats, there is no impact in
>>> XLogInsertRecord.
>>> For example, how about pg_stat_wal() calculates the accumulated
>>> value of wal_records, wal_fpi, and wal_bytes to use pgWalUsage's
>>> value?
>>
>> I don't think that works, but it would work that pgstat_send_wal()
>> takes the difference of that values between two successive calls.
>>
>> WalUsage prevWalUsage;
>> ...
>> pgstat_send_wal()
>> {
>> ..
>> /* fill in some values using pgWalUsage */
>> WalStats.m_wal_bytes = pgWalUsage.wal_bytes - prevWalUsage.wal_bytes;
>> WalStats.m_wal_records = pgWalUsage.wal_records - prevWalUsage.wal_records;
>> WalStats.m_wal_wal_fpi = pgWalUsage.wal_fpi - prevWalUsage.wal_fpi;
>> ...
>> pgstat_send(&WalStats, sizeof(WalStats));
>>
>> /* remember the current numbers */
>> prevWalUsage = pgWalUsage;
>
> Thanks for Horiguchi-san's advice, I changed to reuse pgWalUsage
> which is already defined and eliminates the extra overhead.
+ /* fill in some values using pgWalUsage */
+ WalStats.m_wal_bytes = pgWalUsage.wal_bytes - prevWalUsage.wal_bytes;
+ WalStats.m_wal_records = pgWalUsage.wal_records - prevWalUsage.wal_records;
+ WalStats.m_wal_fpi = pgWalUsage.wal_fpi - prevWalUsage.wal_fpi;
It's better to use WalUsageAccumDiff() here?
prevWalUsage needs to be initialized with pgWalUsage?
+ if (AmWalWriterProcess()){
+ WalStats.m_wal_write_walwriter++;
+ }
+ else
+ {
+ WalStats.m_wal_write_backend++;
+ }
I think that it's better not to separate m_wal_write_xxx into two for
walwriter and other processes. Instead, we can use one m_wal_write_xxx
counter and make pgstat_send_wal() send also the process type to
the stats collector. Then the stats collector can accumulate the counters
per process type if necessary. If we adopt this approach, we can easily
extend pg_stat_wal so that any fields can be reported per process type.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: masao.fujii@oss.nttdata.com, ikedamsh@oss.nttdata.com, horikyota.ntt@gmail.com
Subject: Re: Add statistics to pg_stat_wal view for wal related parameter tuning
In-Reply-To: <04a75b35-c601-65e4-ec34-54dcd995ce11@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox