Horton

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

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: melanieplageman@gmail.com, vignesh21@gmail.com, pryzby@telsasoft.com, lukas@fittl.com, alvherre@alvh.no-ip.org, magnus@hagander.net, pgsql-hackers@postgresql.org, thomas.munro@gmail.com, m.sakrejda@gmail.com
Date: 2023-02-08T06:38:14Z
Lists: pgsql-hackers 
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().



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

Commits

  1. Stabilize pg_stat_io writes test

  2. Fix flakey pg_stat_io test

  3. Suppress more compiler warnings in new pgstats code.

  4. Suppress compiler warnings in new pgstats code.

  5. Add tests for pg_stat_io

  6. Create regress_tblspc in test_setup

  7. Add pg_stat_io view, providing more detailed IO statistics

  8. pgstat: Track more detailed relation IO statistics

  9. pgstat: Infrastructure for more detailed IO statistics

  10. doc: Fix some issues in logical replication section

  11. Manual cleanup and pgindent of pgstat and bufmgr related code

  12. Have the planner consider Incremental Sort for DISTINCT

  13. Use actual backend IDs in pg_stat_get_backend_idset() and friends.

  14. Remove redundant call to pgstat_report_wal()

  15. Add BackendType for standalone backends

  16. Initialize backend status reporting during bootstrap.