Horton

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

Melanie Plageman <melanieplageman@gmail.com>

View thread View on postgr.es
From: Melanie Plageman <melanieplageman@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Andres Freund <andres@anarazel.de>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Magnus Hagander <magnus@hagander.net>, Pg Hackers <pgsql-hackers@postgresql.org>, Lukas Fittl <lukas@fittl.com>, Thomas Munro <thomas.munro@gmail.com>
Date: 2021-09-24T21:58:48Z
Lists: pgsql-hackers

Attachments

On Thu, Sep 23, 2021 at 5:05 PM Melanie Plageman
<melanieplageman@gmail.com> wrote:
>
> The attached v8 patchset is rewritten to add in an additional dimension
> -- buffer type. Now, a backend keeps track of how many buffers of a
> particular type (e.g. shared, local) it has accessed in a particular way
> (e.g. alloc, write). It also changes the naming of various structures
> and the view members.
>
> Previously, stats reset did not work since it did not consider live
> backends' counters. Now, the reset message includes the current live
> backends' counters to be tracked by the stats collector and used when
> the view is queried.
>
> The reset message is one of the areas in which I still need to do some
> work -- I shoved the array of PgBufferAccesses into the existing reset
> message used for checkpointer, bgwriter, etc. Before making a new type
> of message, I would like feedback from a reviewer about the approach.
>
> There are various TODOs in the code which are actually questions for the
> reviewer. Once I have some feedback, it will be easier to address these
> items.
>
> There a few other items which may be material for other commits that
> I would also like to do:
> 1) write wrapper functions for smgr* functions which count buffer
> accesses of the appropriate type. I wasn't sure if these should
> literally just take all the parameters that the smgr* functions take +
> buffer type. Once these exist, there will be less possibility for
> regressions in which new code is added using smgr* functions without
> counting this buffer activity. Once I add these, I was going to go
> through and replace existing calls to smgr* functions and thereby start
> counting currently uncounted buffer type accesses (direct, local, etc).
>
> 2) Separate checkpointer and bgwriter into two views and add additional
> stats to the bgwriter view.
>
> 3) Consider adding a helper function to pgstatfuncs.c to help create the
> tuplestore. These functions all have quite a few lines which are exactly
> the same, and I thought it might be nice to do something about that:
>   pg_stat_get_progress_info(PG_FUNCTION_ARGS)
>   pg_stat_get_activity(PG_FUNCTION_ARGS)
>   pg_stat_get_buffers_accesses(PG_FUNCTION_ARGS)
>   pg_stat_get_slru(PG_FUNCTION_ARGS)
>   pg_stat_get_progress_info(PG_FUNCTION_ARGS)
> I can imagine a function that takes a Datums array, a nulls array, and a
> ResultSetInfo and then makes the tuplestore -- though I think that will
> use more memory. Perhaps we could make a macro which does the initial
> error checking (checking if caller supports returning a tuplestore)? I'm
> not sure if there is something meaningful here, but I thought I would
> ask.
>
> Finally, I haven't removed the test in pg_stats and haven't done a final
> pass for comment clarity, alphabetization, etc on this version.
>

I have addressed almost all of the issues mentioned above in v9.
The only remaining TODOs are described in the commit message.
most critical one is that the reset message doesn't work.

Commits

  1. Stabilize pg_stat_io writes test

  2. Fix flakey pg_stat_io test

  3. Suppress more compiler warnings in new pgstats code.

  4. Suppress compiler warnings in new pgstats code.

  5. Add tests for pg_stat_io

  6. Create regress_tblspc in test_setup

  7. Add pg_stat_io view, providing more detailed IO statistics

  8. pgstat: Track more detailed relation IO statistics

  9. pgstat: Infrastructure for more detailed IO statistics

  10. doc: Fix some issues in logical replication section

  11. Manual cleanup and pgindent of pgstat and bufmgr related code

  12. Have the planner consider Incremental Sort for DISTINCT

  13. Use actual backend IDs in pg_stat_get_backend_idset() and friends.

  14. Remove redundant call to pgstat_report_wal()

  15. Add BackendType for standalone backends

  16. Initialize backend status reporting during bootstrap.