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

Attached is v40.

I have addressed the feedback from Justin [1] and Maciek [2] as well.
I took all of the suggestions regarding the docs that Maciek made,
including the following:

> + default. Future values could include those derived from
> + <symbol>XLOG_BLCKSZ</symbol>, once WAL IO is tracked in this view, and
> + constant multipliers once non-block-oriented IO (e.g. temporary file IO)
> + is tracked here.
>
>
> I know Lukas had commented that we should communicate that the goal is
> to eventually provide relatively comprehensive I/O stats in this view
> (you do that in the view description and I think that works), and this
> is sort of along those lines, but I think speculative documentation
> like this is not all that helpful. I'd drop this last sentence. Just
> my two cents.

I have removed this and added the relevant part of this as a comment to
the view generating function pg_stat_get_io().

On Mon, Dec 5, 2022 at 2:32 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> - I think it might be worth to rename IOCONTEXT_BUFFER_POOL to
> IOCONTEXT_{NORMAL, PLAIN, DEFAULT}. I'd like at some point to track WAL IO ,
> temporary file IO etc, and it doesn't seem useful to define a version of
> BUFFER_POOL for each of them. And it'd make it less confusing, because all
> the other existing contexts are also in the buffer pool (for now, can't wait
> for "bypass" or whatever to be tracked as well).

In attached v40, I've renamed IOCONTEXT_BUFFER_POOL to IOCONTEXT_NORMAL.

> - given that IOContextForStrategy() is defined in freelist.c, and that
> declaring it in pgstat.h requires including buf.h, I think it's probably
> better to move IOContextForStrategy()'s declaration to freelist.h (doesn't
> exist, but whatever the right one is)

I have moved it to buf_internals.h.

> - pgstat_backend_io_stats_assert_well_formed() doesn't seem to belong in
> pgstat.c. Why not pgstat_io_ops.c?

I put it in pgstat.c because it is only used there -- so I made it
static. I've moved it to pg_stat_io_ops.c and declared it in
pgstat_internal.h

> - Do pgstat_io_context_ops_assert_zero(), pgstat_io_op_assert_zero() have to
> be in pgstat.h?

They are used in pgstatfuncs.c, which I presume should not include
pgstat_internal.h. Or did you mean that I should not put them in a
header file at all?

- Melanie

[1] https://www.postgresql.org/message-id/20221130025113.GD24131%40telsasoft.com
[2] https://www.postgresql.org/message-id/CAOtHd0BfFdMqO7-zDOk%3DiJTatzSDgVcgYcaR1_wk0GS4NN%2BRUQ%40mail.gmail.com