Re: Introduce a new view for checkpointer related stats

On Thu, Feb 9, 2023 at 12:33 PM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,

Thanks for looking at this.

> On 2023-02-09 12:21:51 +0530, Bharath Rupireddy wrote:
> > @@ -1105,18 +1105,22 @@ CREATE VIEW pg_stat_archiver AS
> >
> >  CREATE VIEW pg_stat_bgwriter AS
> >      SELECT
> > -        pg_stat_get_bgwriter_timed_checkpoints() AS checkpoints_timed,
> > -        pg_stat_get_bgwriter_requested_checkpoints() AS checkpoints_req,
> > -        pg_stat_get_checkpoint_write_time() AS checkpoint_write_time,
> > -        pg_stat_get_checkpoint_sync_time() AS checkpoint_sync_time,
> > -        pg_stat_get_bgwriter_buf_written_checkpoints() AS buffers_checkpoint,
> >          pg_stat_get_bgwriter_buf_written_clean() AS buffers_clean,
> >          pg_stat_get_bgwriter_maxwritten_clean() AS maxwritten_clean,
> > -        pg_stat_get_buf_written_backend() AS buffers_backend,
> > -        pg_stat_get_buf_fsync_backend() AS buffers_backend_fsync,
> >          pg_stat_get_buf_alloc() AS buffers_alloc,
> >          pg_stat_get_bgwriter_stat_reset_time() AS stats_reset;
> >
> > +CREATE VIEW pg_stat_checkpointer AS
> > +    SELECT
> > +        pg_stat_get_timed_checkpoints() AS timed_checkpoints,
> > +        pg_stat_get_requested_checkpoints() AS requested_checkpoints,
> > +        pg_stat_get_checkpoint_write_time() AS checkpoint_write_time,
> > +        pg_stat_get_checkpoint_sync_time() AS checkpoint_sync_time,
> > +        pg_stat_get_buf_written_checkpoints() AS buffers_written_checkpoints,
> > +        pg_stat_get_buf_written_backend() AS buffers_written_backend,
> > +        pg_stat_get_buf_fsync_backend() AS buffers_fsync_backend,
> > +        pg_stat_get_checkpointer_stat_reset_time() AS stats_reset;
> > +
>
> I don't think the backend written stats belong more accurately in
> pg_stat_checkpointer than pg_stat_bgwriter.

We accumulate buffers_written_backend and buffers_fsync_backend of all
backends under checkpointer stats to show the aggregated results to
the users. I think this is correct because the checkpointer is the one
that processes fsync requests (of course, backends themselves can
fsync when needed, that's what the buffers_fsync_backend shows),
whereas bgwriter doesn't perform IIUC.

> I continue to be worried about breaking just about any postgres monitoring
> setup.

Hm. Yes, it requires minimal and straightforward changes in monitoring
scripts. Please note that we separated out bgwriter and checkpointer
in v9.2 12 years ago but we haven't had a chance to separate the stats
so far. We might do it at some point of time, IMHO this is that time.

We did away with promote_trigger_file (cd4329d) very recently. The
agreement was that the changes required to move on to other mechanisms
of promotion are minimal, hence we didn't want it to be first
deprecated and then removed.

From the discussion upthread, it looks like Robert, Amit, Bertrand,
Greg and myself are in favour of not having a deprecated version but
moving them to the new pg_stat_checkpointer view.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com