|From:||Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>|
|To:||Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>|
|Cc:||Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: pg_stat_wal_write statistics view|
|Views:||Raw Message | Whole Thread | Download mbox|
On Tue, Sep 12, 2017 at 9:06 AM, Haribabu Kommi
> On Tue, Sep 12, 2017 at 2:04 AM, Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>
>> On Wed, Sep 6, 2017 at 9:16 AM, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
>> > Attached the latest patch and performance report.
>> While looking into the patch, I realized that a normal backend has to
>> check almost 10 if conditions at worst case inside XLogWrite(6 in
>> am_background_process method, 1 for write, 1 for blocks, 2 for
>> fsyncs), just to decide whether it is a normal backend or not. The
>> count is more for walwriter process. Although I've not done any
>> performance tests, IMHO, it might add further overhead on the
>> XLogWrite Lock.
>> I was thinking whether it is possible to collect all the stats in
>> XLogWrite() irrespective of the type of backend and update the shared
>> counters at once at the end of the function. Thoughts?
> Thanks for the review.
> Yes, I agree with you that the stats update can be done at the end
> of the function to avoid some overhead. Updated patch is attached.
Thanks for the patch.
+ * Check whether the current process is a normal backend or not.
+ * This function checks for the background processes that does
+ * some WAL write activity only and other background processes
+ * are not considered. It considers all the background workers
+ * as WAL write activity workers.
+ * Returns false - when the current process is a normal backend
+ * true - when the current process a background process/worker
+ /* check whether current process is a background process/worker? */
+ if (!AmBackgroundWriterProcess() ||
+ !AmCheckpointerProcess() ||
+ !AmStartupProcess() ||
+ !IsBackgroundWorker ||
+ !am_walsender ||
+ return false;
+ return true;
I think you've to do AND operation here instead of OR. Isn't it?
Another point is that, the function description could start with
'Check whether the current process is a background process/worker.'
> There is an overhead with IO time calculation. Is the view is good
> enough without IO columns?
I'm not sure how IO columns are useful for tuning the WAL write/fsync
performance from an user's perspective. But, it's definitely useful
for developing/improving stuffs in XLogWrite.
> And also during my tests, I didn't observe any other background
> processes performing the xlogwrite operation, the values are always
> zero. Is it fine to merge them with backend columns?
Apart from wal writer process, I don't see any reason why you should
track other background processes separately from normal backends.
However, I may be missing some important point.
Thanks & Regards,
|Next Message||Amit Khandekar||2017-09-12 05:45:13||Re: UPDATE of partition key|
|Previous Message||Jing Wang||2017-09-12 05:07:03||Re: Support to COMMENT ON DATABASE CURRENT_DATABASE|