On Fri, Sep 22, 2017 at 5:46 PM, Julien Rouhaud <rjuju...@gmail.com> wrote:
> Hello, > > On Wed, Sep 13, 2017 at 3:01 AM, Haribabu Kommi > <kommi.harib...@gmail.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 adding 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 > process</entry> > > it should be backend processes > Changed. > +#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 > Changed. > + 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 > Changed. > +-- There will surely and maximum one record > +select count(*) > 0 as ok from pg_stat_walwrites; > > The test should be count(*) = 1 > Changed. Updated patch attached. Regards, Hari Babu Fujitsu Australia
pg_stat_walwrites-statistics-view_v9.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers