Re: Introduce a new view for checkpointer related stats

On Mon, Feb 13, 2023 at 11:31:03AM +0530, Bharath Rupireddy wrote:
> Needed a rebase. Please review the attached v8 patch set.

I was looking at this patch, and got a few comments.

FWIW, I kind of agree with the feeling of Bertrand upthread that using
"checkpoint_" in the attribute names for the new view is globally
inconsistent.  After 0003, we get: 
=# select attname from pg_attribute
     where attrelid = 'pg_stat_checkpointer'::regclass
     and attnum > 0;
          attname
-----------------------------
 timed_checkpoints
 requested_checkpoints
 checkpoint_write_time
 checkpoint_sync_time
 buffers_written_checkpoints
 stats_reset
(6 rows)
=# select attname from pg_attribute
     where attrelid = 'pg_stat_bgwriter'::regclass and
     attnum > 0;
     attname
------------------
 buffers_clean
 maxwritten_clean
 buffers_alloc
 stats_reset
(4 rows)

The view for the bgwriter does not do that.  I'd suggest to use
functions that are named as pg_stat_get_checkpoint_$att with shorter
$atts.  It is true that "timed" is a bit confusing, because it refers
to a number of checkpoints, and that can be read as a time value (?).
So how about num_timed?  And for the others num_requested and
buffers_written?

+ * Unlike the checkpoint fields, reqquests related fields are protected by 

s/reqquests/requests/.

 SlruSyncFileTag(SlruCtl ctl, const FileTag *ftag, char *path)
 {
+	SlruShared	shared = ctl->shared;
 	int			fd;
 	int			save_errno;
 	int			result;
 
+	/* update the stats counter of flushes */
+	pgstat_count_slru_flush(shared->slru_stats_idx);

Why is that in 0002?  Isn't that something we should treat as a bug
fix of its own, even backpatching it to make sure that the flush
requests for individual commit_ts, multixact and clog files are
counted in the stats?

Saying that, I am OK with moving ahead with 0001 and 0002 to remove
buffers_backend_fsync and buffers_backend from pg_stat_bgwriter, but
it is better to merge them into a single commit.  It helps with 0003
and this would encourage the use of pg_stat_io that does a better job
overall with more field granularity.
--
Michael