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

On Sun, Jan 26, 2020 at 1:44 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2020-01-25 15:43:41 +0100, Magnus Hagander wrote:
> > On Fri, Jan 24, 2020 at 8:52 PM Andres Freund <andres@anarazel.de> wrote:
> > > Additionally pg_stat_bgwriter.buffers_backend also counts writes done by
> > > autovacuum et al.
>
> > > I think it'd make sense to at least split buffers_backend into
> > > buffers_backend_extend,
> > > buffers_backend_write,
> > > buffers_backend_write_strat
> > >
> > > but it could also be worthwhile to expand it into
> > > buffers_backend_extend,
> > > buffers_{backend,checkpoint,bgwriter,autovacuum}_write
> > > buffers_{backend,autovacuum}_write_stat
> >
> > Given that these are individual global counters, I don't really see
> > any reason not to expand it to the bigger set of counters. It's easy
> > enough to add them up together later if needed.
>
> Are you agreeing to
> buffers_{backend,checkpoint,bgwriter,autovacuum}_write
> or are you suggesting further ones?

The former.


> > > I think we also count things as writes that aren't writes: mdtruncate()
> > > is AFAICT counted as one backend write for each segment. Which seems
> > > weird to me.
> >
> > It's at least slightly weird :) Might it be worth counting truncate
> > events separately?
>
> Is that really something interesting? Feels like it'd have to be done at
> a higher level to be useful. E.g. the truncate done by TRUNCATE (when in
> same xact as creation) and VACUUM are quite different. I think it'd be
> better to just not include it.

Yeah, you're probably right. it certainly makes very little sense
where it is now.


> > > Lastly, I don't understand what the point of sending fixed size stats,
> > > like the stuff underlying pg_stat_bgwriter, through pgstats IPC. While
> > > I don't like it's architecture, we obviously need something like pgstat
> > > to handle variable amounts of stats (database, table level etc
> > > stats). But that doesn't at all apply to these types of global stats.
> >
> > That part has annoyed me as well a few times. +1 for just moving that
> > into a global shared memory. Given that we don't really care about
> > things being in sync between those different counters *or* if we loose
> > a bit of data (which the stats collector is designed to do), we could
> > even do that without a lock?
>
> I don't think we'd quite want to do it without any (single counter)
> synchronization - high concurrency setups would be pretty likely to
> loose values that way. I suspect the best would be to have a struct in
> shared memory that contains the potential counters for each potential
> process. And then sum them up when actually wanting the concrete
> value. That way we avoid unnecessary contention, in contrast to having a
> single shared memory value for each(which would just pingpong between
> different sockets and store buffers).  There's a few details like how
> exactly to implement resetting the counters, but ...

Right. Each process gets to do their own write, but still in shared
memory. But do you need to lock them when reading them (for the
summary)? That's the part where I figured you could just read and
summarize them, and accept the possible loss.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/