Re: pg_stat_wal_write statistics view

On Thu, Mar 16, 2017 at 9:55 AM, Julien Rouhaud <julien(dot)rouhaud(at)dalibo(dot)com>
wrote:

> On Wed, Feb 15, 2017 at 02:53:44PM +1100, Haribabu Kommi wrote:
> > Here I attached patch that implements the view.
> > I will add this patch to next commitfest.
>
> Hello,
>
> I just reviewed the patch.
>

Thanks for the review.

First, there are some whitespace issues that make git-apply complaining (at
> least lines 218 and 396).
>

Removed.

Patch is rather straightforward and works as expected, doc compiles without
> issue.
>
> I only found some minor issues:
>
> + <entry>One row only, showing statistics about the
> + wal writing activity. See
>
> + <entry>Number of wal writes that are carried out by the backend
> process</entry>
>
>
> WAL should be uppercase (and for some more occurences).
>

Fixed.

> + <entry>
> + Number of wal writes that are carried out by background workers
> such as checkpointer,
> + writer and walwriter.
>
> I guess you meant backgroung processes?
>

Yes, it is background processes. Updated.

> >+ This field data will be populated only when the track_io_timing
> GUC is enabled
> (and similar occurences)
>
> track_io_timing should link to <xref linkend="guc-track-io-timing">
> instead of
> mentionning GUC.
>

Updated accordingly.

I think you also need to update the track_io_timing description in
> sgml/config.sgml to mention this new view.
>

Added the reference of pg_stat_walwrites in the GUC description.

>
> + else
> + {
> + LocalWalWritesStats.m_wal_total_write_time = 0;
> + }
> (and similar ones)
>
> The brackets seem unnecessary.
>

Corrected.

Updated patch attached.

Regards,
Hari Babu
Fujitsu Australia