From: | Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com> |
---|---|
To: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, masao(dot)fujii(at)oss(dot)nttdata(dot)com, Andres Freund <andres(at)anarazel(dot)de> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: wal stats questions |
Date: | 2021-03-26 07:20:04 |
Message-ID: | ed560f19-597b-b37f-8ff5-12885a6487af@oss.nttdata.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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().
> 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?)
> 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.
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.
> Maybe there is the case where a backend generates no WAL records,
> but just writes them because it needs to do write-ahead-logging
> when flush the table data?
> Ugh! I was missing a very large blob.. Ok, we need additional check
> for non-pgWalUsage part. Sorry.
Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION
Attachment | Content-Type | Size |
---|---|---|
v1-0001-performance-improvements-of-reporting-wal-stats.patch | text/x-patch | 7.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Masahiro Ikeda | 2021-03-26 07:46:14 | Re: make the stats collector shutdown without writing the statsfiles if the immediate shutdown is requested. |
Previous Message | Joel Jacobson | 2021-03-26 06:53:04 | Re: [PATCH] pg_permissions |