public inbox for [email protected]  

help / color / mirror / Atom feed

From: Andres Freund <[email protected]>
To: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Date: Tue, 7 Feb 2023 22:38:14 -0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CAAKRu_af0YcV9CXG9M5R06tYRUzk13+H7qOsU6etmS4zE7RAUQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>

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 <[email protected]> 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().



> > > > +        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.

Greetings,

Andres Freund

view thread (36+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox