Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?) - Mailing list pgsql-hackers

> Subject: [PATCH v26 3/4] Track IO operation statistics

> @@ -978,8 +979,17 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
> 
>       bufBlock = isLocalBuf ? LocalBufHdrGetBlock(bufHdr) : BufHdrGetBlock(bufHdr);
> 
> +     if (isLocalBuf)
> +             io_path = IOPATH_LOCAL;
> +     else if (strategy != NULL)
> +             io_path = IOPATH_STRATEGY;
> +     else
> +             io_path = IOPATH_SHARED;

Seems a bit ugly to have an if (isLocalBuf) just after an isLocalBuf ?.

> +                     /*
> +                      * When a strategy is in use, reused buffers from the strategy ring will
> +                      * be counted as allocations for the purposes of IO Operation statistics
> +                      * tracking.
> +                      *
> +                      * However, even when a strategy is in use, if a new buffer must be
> +                      * allocated from shared buffers and added to the ring, this is counted
> +                      * as a IOPATH_SHARED allocation.
> +                      */

There's a bit too much duplication between the paragraphs...

> @@ -628,6 +637,9 @@ pgstat_report_stat(bool force)
>       /* flush database / relation / function / ... stats */
>       partial_flush |= pgstat_flush_pending_entries(nowait);
> 
> +     /* flush IO Operations stats */
> +     partial_flush |= pgstat_flush_io_ops(nowait);

Could you either add a note to the commit message that the stats file
version needs to be increased, or just iclude that in the patch.