Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)

I've attached v27 of the patch.

I've renamed IOPATH to IOCONTEXT. I also have added assertions to
confirm that unexpected statistics are not being accumulated.

There are also assorted other cleanups and changes.

It would be good to confirm that the rows being skipped and cells that
are NULL in the view are the correct ones.
The startup process will never use a BufferAccessStrategy, right?

On Wed, Jul 20, 2022 at 12:50 PM Andres Freund <andres(at)anarazel(dot)de> wrote:

>
> > 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 ?.
>

Changed this.

>
>
> > + /*
> > + * 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...
>

I actually think the two paragraphs are making separate points. I've
edited this, so see if you like it better now.

>
> > @@ -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.
>
>
Bumped the stats file version in attached patchset.

- Melanie