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

From: Melanie Plageman <melanieplageman(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Justin Pryzby <pryzby(at)telsasoft(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Lukas Fittl <lukas(at)fittl(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Date: 2021-12-30 20:30:50
Message-ID: CAAKRu_Y2t8uOhMBU461foxJd_C0gTyND-wZzvE8d-FLFtJOksw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Dec 21, 2021 at 8:32 PM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
> On Thu, Dec 16, 2021 at 3:18 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > > > > From 9f22da9041e1e1fbc0ef003f5f78f4e72274d438 Mon Sep 17 00:00:00 2001
> > > > > From: Melanie Plageman <melanieplageman(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2021-12-30 20:35:45 Re: Why is src/test/modules/committs/t/002_standby.pl flaky?
Previous Message Alvaro Herrera 2021-12-30 20:21:28 Re: Column Filtering in Logical Replication