Horton

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

Melanie Plageman <melanieplageman@gmail.com>

View thread View on postgr.es
From: Melanie Plageman <melanieplageman@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Magnus Hagander <magnus@hagander.net>, Pg Hackers <pgsql-hackers@postgresql.org>, Lukas Fittl <lukas@fittl.com>, Thomas Munro <thomas.munro@gmail.com>
Date: 2021-11-24T21:19:20Z
Lists: pgsql-hackers

Attachments

On Fri, Nov 19, 2021 at 11:49 AM Andres Freund <andres@anarazel.de> wrote:
> > +/*
> > + * On modern systems this is really just *counter++.  On some older systems
> > + * there might be more to it, due to inability to read and write 64 bit values
> > + * atomically.
> > + */
> > +static inline void inc_counter(pg_atomic_uint64 *counter)
> > +{
> > +     pg_atomic_write_u64(counter, pg_atomic_read_u64(counter) + 1);
> > +}
> > +
> >  #undef INSIDE_ATOMICS_H
>
> Why is this using a completely different naming scheme from the rest of the
> file?

It was what Thomas originally named it. Also, I noticed all the other
pg_atomic* in this file were wrappers around the same impl function, so
I thought maybe naming it this way would be confusing. I renamed it to
pg_atomic_inc_counter(), though maybe pg_atomic_readonly_write() would
be better?

>
> >  doc/src/sgml/monitoring.sgml                | 116 +++++++++++++-
> >  src/backend/catalog/system_views.sql        |  11 ++
> >  src/backend/postmaster/checkpointer.c       |   3 +-
> >  src/backend/postmaster/pgstat.c             | 161 +++++++++++++++++++-
> >  src/backend/storage/buffer/bufmgr.c         |  46 ++++--
> >  src/backend/storage/buffer/freelist.c       |  23 ++-
> >  src/backend/storage/buffer/localbuf.c       |   3 +
> >  src/backend/storage/sync/sync.c             |   1 +
> >  src/backend/utils/activity/backend_status.c |  60 +++++++-
> >  src/backend/utils/adt/pgstatfuncs.c         | 152 ++++++++++++++++++
> >  src/include/catalog/pg_proc.dat             |   9 ++
> >  src/include/miscadmin.h                     |   2 +
> >  src/include/pgstat.h                        |  53 +++++++
> >  src/include/storage/buf_internals.h         |   4 +-
> >  src/include/utils/backend_status.h          |  80 ++++++++++
> >  src/test/regress/expected/rules.out         |   8 +
> >  16 files changed, 701 insertions(+), 31 deletions(-)
>
> This is a pretty large change, I wonder if there's a way to make it a bit more
> granular.
>

I have done this. See v15 patch set attached.

- Melanie

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.