Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?) - Mailing list pgsql-hackers

From Kyotaro Horiguchi
Subject Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Date
Msg-id 20230208.160328.1869330381378352117.horikyota.ntt@gmail.com
Whole thread Raw
In response to Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers
At Tue, 7 Feb 2023 22:38:14 -0800, Andres Freund <andres@anarazel.de> wrote in 
> Hi,
> 
> I did another read through the series. I do have some minor changes, but
> they're minor. I think this is ready for commit. I plan to start pushing
> tomorrow.
> 
> The changes I made are:
> - the tablespace test changes didn't quite work in isolation / needed a bit of
>   polishing
> - moved the tablespace changes to later in the series
> - split the tests out of the commit adding the view into its own commit
> - minor code formatting things (e.g. didn't like nested for()s without {})


> On 2023-01-25 16:56:17 +0900, Kyotaro Horiguchi wrote:
> > At Tue, 24 Jan 2023 14:35:12 -0800, Andres Freund <andres@anarazel.de> wrote in
> > > > +    write_chunk_s(fpout, &pgStatLocal.snapshot.io);
> > > > +    if (!read_chunk_s(fpin, &shmem->io.stats))
> > > >
> > > > The names of the functions hardly make sense alone to me. How about
> > > > write_struct()/read_struct()?  (I personally prefer to use
> > > > write_chunk() directly..)
> > >
> > > That's not related to this patch - there's several existing callers for
> > > it. And write_struct wouldn't be better imo, because it's not just for
> > > structs.
> >
> > Hmm.  Then what the "_s" stands for?
> 
> Size. It's a macro that just forwards to read_chunk()/write_chunk().

I know what the macros do.  But, I'm fine with the names as they are
there since before this patch. Sorry for the noise.

> > > > > +        Number of read operations in units of <varname>op_bytes</varname>.
> > > >
> > > > I may be the only one who see the name as umbiguous between "total
> > > > number of handled bytes" and "bytes hadled at an operation". Can't it
> > > > be op_blocksize or just block_size?
> > > >
> > > > +       b.io_object,
> > > > +       b.io_context,
> > >
> > > No, block wouldn't be helpful - we'd like to use this for something that isn't
> > > uniform blocks.
> >
> > What does the field show in that case?  The mean of operation size? Or
> > one row per opration size?  If the former, the name looks somewhat
> > wrong. If the latter, block_size seems making sense.
> 
> 1, so that it's clear that the rest are in bytes.

Thanks. Okay, I guess the documentation will be changed as necessary.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: Generating code for query jumbling through gen_node_support.pl
Next
From: Andres Freund
Date:
Subject: Re: deadlock-hard flakiness