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: Andres Freund <andres@anarazel.de>
Cc: Justin Pryzby <pryzby@telsasoft.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, 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-12-30T20:30:50Z
Lists: pgsql-hackers 
On Tue, Dec 21, 2021 at 8:32 PM Melanie Plageman
<melanieplageman@gmail.com> wrote:
> On Thu, Dec 16, 2021 at 3:18 PM Andres Freund <andres@anarazel.de> wrote:
> > > > > From 9f22da9041e1e1fbc0ef003f5f78f4e72274d438 Mon Sep 17 00:00:00 2001
> > > > > From: Melanie Plageman <melanieplageman@gmail.com>
> > > > > Date: Wed, 24 Nov 2021 12:20:10 -0500
> > > > > Subject: [PATCH v17 6/7] Remove superfluous bgwriter stats
> > > > >
> > > > > Remove stats from pg_stat_bgwriter which are now more clearly expressed
> > > > > in pg_stat_buffers.
> > > > >
> > > > > TODO:
> > > > > - make pg_stat_checkpointer view and move relevant stats into it
> > > > > - add additional stats to pg_stat_bgwriter
> > > >
> > > > When do you think it makes sense to tackle these wrt committing some of the
> > > > patches?
> > >
> > > Well, the new stats are a superset of the old stats (no stats have been
> > > removed that are not represented in the new or old views). So, I don't
> > > see that as a blocker for committing these patches.
> >
> > > Since it is weird that pg_stat_bgwriter had mostly checkpointer stats,
> > > I've edited this commit to rename that view to pg_stat_checkpointer.
> >
> > > I have not made a separate view just for maxwritten_clean (presumably
> > > called pg_stat_bgwriter), but I would not be opposed to doing this if
> > > you thought having a view with a single column isn't a problem (in the
> > > event that we don't get around to adding more bgwriter stats right
> > > away).
> >
> > How about keeping old bgwriter values in place in the view , but generated
> > from the new stats stuff?
>
> I tried this, but I actually don't think it is the right way to go. In
> order to maintain the old view with the new source code, I had to add
> new code to maintain a separate resets array just for the bgwriter view.
> It adds some fiddly code that will be annoying to maintain (the reset
> logic is confusing enough as is).
> And, besides the implementation complexity, if a user resets
> pg_stat_bgwriter and not pg_stat_buffers (or vice versa), they will
> see totally different numbers for "buffers_backend" in pg_stat_bgwriter
> than shared buffers written by B_BACKEND in pg_stat_buffers. I would
> find that confusing.

In a quick chat off-list, Andres suggested it might be okay to have a
single reset target for both the pg_stat_buffers view and legacy
pg_stat_bgwriter view. So, I am planning to share a new patchset which
has only the new "buffers" target which will also reset the legacy
pg_stat_bgwriter view.

I'll also remove the bgwriter stats I proposed and the
pg_stat_checkpointer view to keep things simple for now.

- Melanie

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.