|From:||Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>|
|To:||Julien Rouhaud <rjuju123(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|
|Views:||Raw Message | Whole Thread | Download mbox|
On Fri, Sep 22, 2017 at 5:46 PM, Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
> 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.
Thanks for the review.
I removed the time related columns from the view. As these columns are
an overhead and GUC controlled behavior is different to the other views.
Apart from above time columns, I removed walwriter_dirty_writes, as the
walwriter writers cannot be treated as dirty writes.
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
> it should be backend processes
> +#define NUM_PG_STAT_WALWRITE_ATTS 16
> + 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 */
> 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
Updated patch attached.
|Next Message||Albe Laurenz||2017-09-27 09:05:45||Re: Enhancements to passwordcheck|
|Previous Message||Michael Paquier||2017-09-27 08:45:19||Re: A design for amcheck heapam verification|