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: Justin Pryzby <pryzby@telsasoft.com>, 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: 2022-07-20T17:40:40Z
Lists: pgsql-hackers 
On Wed, Jul 20, 2022 at 12:50 PM Andres Freund <andres@anarazel.de> wrote:

>
> On 2022-07-14 18:44:48 -0400, Melanie Plageman wrote:
>
> > @@ -1427,8 +1445,10 @@ pgstat_read_statsfile(void)
> >       FILE       *fpin;
> >       int32           format_id;
> >       bool            found;
> > +     PgStat_BackendIOPathOps io_stats;
> >       const char *statfile = PGSTAT_STAT_PERMANENT_FILENAME;
> >       PgStat_ShmemControl *shmem = pgStatLocal.shmem;
> > +     PgStatShared_BackendIOPathOps *io_stats_shmem = &shmem->io_ops;
> >
> >       /* shouldn't be called from postmaster */
> >       Assert(IsUnderPostmaster || !IsPostmasterEnvironment);
> > @@ -1486,6 +1506,22 @@ pgstat_read_statsfile(void)
> >       if (!read_chunk_s(fpin, &shmem->checkpointer.stats))
> >               goto error;
> >
> > +     /*
> > +      * Read IO Operations stats struct
> > +      */
> > +     if (!read_chunk_s(fpin, &io_stats))
> > +             goto error;
> > +
> > +     io_stats_shmem->stat_reset_timestamp =
> io_stats.stat_reset_timestamp;
> > +
> > +     for (int i = 0; i < BACKEND_NUM_TYPES; i++)
> > +     {
> > +             PgStat_IOPathOps *stats = &io_stats.stats[i];
> > +             PgStatShared_IOPathOps *stats_shmem =
> &io_stats_shmem->stats[i];
> > +
> > +             memcpy(stats_shmem->data, stats->data,
> sizeof(stats->data));
> > +     }
>
> Why can't the data be read directly into shared memory?
>
>
It is not the same lock. Each PgStatShared_IOPathOps has a lock so that
they can be accessed individually (per BackendType in
PgStatShared_BackendIOPathOps). It is optimized for the more common
operation of flushing at the expense of the snapshot operation (which
should be less common) and reset operation.


> > +void
> > +pgstat_io_ops_snapshot_cb(void)
> > +{
> > +     PgStatShared_BackendIOPathOps *all_backend_stats_shmem =
> &pgStatLocal.shmem->io_ops;
> > +     PgStat_BackendIOPathOps *all_backend_stats_snap =
> &pgStatLocal.snapshot.io_ops;
> > +
> > +     for (int i = 0; i < BACKEND_NUM_TYPES; i++)
> > +     {
> > +             PgStatShared_IOPathOps *stats_shmem =
> &all_backend_stats_shmem->stats[i];
> > +             PgStat_IOPathOps *stats_snap =
> &all_backend_stats_snap->stats[i];
> > +
> > +             LWLockAcquire(&stats_shmem->lock, LW_EXCLUSIVE);
>
> Why acquire the same lock repeatedly for each type, rather than once for
> the whole?
>
>
This is also because of having a LWLock in each PgStatShared_IOPathOps.
Because I don't want a lock in the backend local stats, I have two data
structures PgStatShared_IOPathOps and PgStat_IOPathOps. I thought it was
odd to write out the lock to the file, so when persisting the stats, I
write out the relevant data only and when reading it back in to shared
memory,  I read in the data member of PgStatShared_IOPathOps.

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.