From: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
---|---|
To: | Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com> |
Cc: | Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>, 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 |
Date: | 2017-09-22 07:46:40 |
Message-ID: | CAOBaU_YO9EfgKih=3evVxK-31R0HUSzoewxcY7wMfx_AKnxQ6w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hello,
On Wed, Sep 13, 2017 at 3:01 AM, Haribabu Kommi
<kommi(dot)haribabu(at)gmail(dot)com> wrote:
> I ran the latest performance tests with and without IO times, there is an
> overhead involved with IO time calculation and didn't observe any
> performance
> overhead with normal stats. May be we can enable the IO stats only in the
> development environment to find out the IO stats?
>
-1 for me to have these columns depending on a GUC *and* wether it's a
debug or assert build (unless this behaviour already exists for other
functions, but AFAIK that's not the case).
> I added the other background stats to find out how much WAL write is
> carried out by the other background processes. Now I am able to collect
> the stats for the other background processes also after the pgbench test.
> So I feel now the separate background stats may be useful.
>
> Attached latest patch, performance test results and stats details with
> separate background stats and also combine them with backend including
> the IO stats also.
>
The results seem to vary too much to have a strong opinion, but it
looks like the timing instrumentation can be too expensive, so I'd be
-1 on adding this overhead to track_io_timing.
I have some minor comments on the last patch:
+ <row>
+ <entry><structfield>backend_writes</></entry>
+ <entry><type>bigint</type></entry>
+ <entry>Number of WAL writes that are carried out by the backend
process</entry>
it should be backend processes
+#define NUM_PG_STAT_WALWRITE_ATTS 16
+
+Datum
+pg_stat_get_walwrites(PG_FUNCTION_ARGS)
+{
+ TupleDesc tupdesc;
+ Datum values[NUM_PG_STAT_WALWRITE_ATTS];
+ bool nulls[NUM_PG_STAT_WALWRITE_ATTS];
For consistency, the #define should be just before the tupdesc
declaration, and be named PG_STAT_GET_WALWRITE_COLS
+ PgStat_Counter backend_writes; /* No of writes by backend */
+ PgStat_Counter backend_dirty_writes; /* No of dirty writes by
+ * backend when WAL buffers
+ * full */
+ PgStat_Counter backend_write_blocks; /* Total no of pages
written by backend */
+ PgStat_Counter backend_total_write_time; /* Total write time in
+ * milliseconds by backend */
+ PgStat_Counter backend_total_sync_time; /* Total write time in
+ * milliseconds by backend */
these comments should all be say "backends" for consistency
+-- There will surely and maximum one record
+select count(*) > 0 as ok from pg_stat_walwrites;
The test should be count(*) = 1
From | Date | Subject | |
---|---|---|---|
Next Message | Kyotaro HORIGUCHI | 2017-09-22 07:46:54 | Re: GUC for cleanup indexes threshold. |
Previous Message | Masahiko Sawada | 2017-09-22 07:42:41 | Re: Assertion failure when the non-exclusive pg_stop_backup aborted. |