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
signature.asc
Description: PGP signature