From: | Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Cc: | masao(dot)fujii(at)oss(dot)nttdata(dot)com, Andres Freund <andres(at)anarazel(dot)de>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
Subject: | Re: wal stats questions |
Date: | 2021-03-30 00:41:24 |
Message-ID: | 5a22c695-04f8-b1e8-f83a-8f76e4a7a9ac@oss.nttdata.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I update the patch since there were my misunderstanding points.
On 2021/03/26 16:20, Masahiro Ikeda wrote:
> Thanks for many your suggestions!
> I made the patch to handle the issues.
>
>> 1) What is the motivation to have both prevWalUsage and pgWalUsage,
>> instead of just accumulating the stats since the last submission?
>> There doesn't seem to be any comment explaining it? Computing
>> differences to previous values, and copying to prev*, isn't free. I
>> assume this is out of a fear that the stats could get reset before
>> they're used for instrument.c purposes - but who knows?
>
> I removed the unnecessary code copying pgWalUsage and just reset the
> pgWalUsage after reporting the stats in pgstat_report_wal().
I didn't change this.
>> 2) Why is there both pgstat_send_wal() and pgstat_report_wal()? With the
>> former being used by wal writer, the latter by most other processes?
>> There again don't seem to be comments explaining this.
>
> I added the comments why two functions are separated.
> (But is it better to merge them?)
I updated the comments for following reasons.
>> 3) Doing if (memcmp(&WalStats, &all_zeroes, sizeof(PgStat_MsgWal)) == 0)
>> just to figure out if there's been any changes isn't all that
>> cheap. This is regularly exercised in read-only workloads too. Seems
>> adding a boolean WalStats.have_pending = true or such would be
>> better.
>> 4) For plain backends pgstat_report_wal() is called by
>> pgstat_report_stat() - but it is not checked as part of the "Don't
>> expend a clock check if nothing to do" check at the top. It's
>> probably rare to have pending wal stats without also passing one of
>> the other conditions, but ...
>
> I added the logic to check if the stats counters are updated or not in
> pgstat_report_stat() using not only generated wal record but also write/sync
> counters, and it can skip to call reporting function.
I removed the checking code whether the wal stats counters were updated or not
in pgstat_report_stat() since I couldn't understand the valid case yet.
pgstat_report_stat() is called by backends when the transaction is finished.
This means that the condition seems always pass.
I thought to implement if the WAL stats counters were not updated, skip to
send all statistics including the counters for databases and so on. But I
think it's not good because it may take more time to be reflected the
generated stats by read-only transaction.
> Although I added the condition which the write/sync counters are updated or
> not, I haven't understood the following case yet...Sorry. I checked related
> code and tested to insert large object, but I couldn't. I'll investigate more
> deeply, but if you already know the function which leads the following case,
> please let me know.
I understood the above case (Fujii-san, thanks for your advice in person).
When to flush buffers, for example, to select buffer replacement victim,
it requires a WAL flush if the buffer is dirty. So, to check the WAL stats
counters are updated or not, I check the number of generated wal record and
the counter of syncing in pgstat_report_wal().
The reason why not to check the counter of writing is that if to write is
happened, to sync is happened too in the above case. I added the comments in
the patch.
Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION
Attachment | Content-Type | Size |
---|---|---|
v2-0001-performance-improvements-of-reporting-wal-stats.patch | text/x-patch | 6.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2021-03-30 00:51:37 | Re: Merging statistics from children instead of re-sampling everything |
Previous Message | Tomas Vondra | 2021-03-30 00:32:53 | Re: Merging statistics from children instead of re-sampling everything |