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

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: andres(at)anarazel(dot)de
Cc: melanieplageman(at)gmail(dot)com, vignesh21(at)gmail(dot)com, pryzby(at)telsasoft(dot)com, lukas(at)fittl(dot)com, alvherre(at)alvh(dot)no-ip(dot)org, magnus(at)hagander(dot)net, pgsql-hackers(at)postgresql(dot)org, thomas(dot)munro(at)gmail(dot)com, m(dot)sakrejda(at)gmail(dot)com
Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Date: 2023-02-08 07:03:28
Message-ID: 20230208.160328.1869330381378352117.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Tue, 7 Feb 2023 22:38:14 -0800, Andres Freund <andres(at)anarazel(dot)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(at)anarazel(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2023-02-08 07:06:03 Re: deadlock-hard flakiness
Previous Message Andres Freund 2023-02-08 07:01:03 Re: Generating code for query jumbling through gen_node_support.pl