Re: [PATCH] Expose checkpoint timestamp and duration in pg_stat_checkpointer
Michael Banck <mbanck@gmx.net>
From: Michael Banck <mbanck@gmx.net>
To: Soumya S Murali <soumyamurali.work@gmail.com>
Cc: Álvaro Herrera <alvherre@kurilemu.de>, pgsql-hackers@lists.postgresql.org, melanieplageman@gmail.com, juanjo.santamaria@gmail.com
Date: 2025-11-28T08:08:50Z
Lists: pgsql-hackers
Hi, On Fri, Nov 28, 2025 at 10:23:54AM +0530, Soumya S Murali wrote: > I have updated the code based on the feedback received to my earlier > mails and prepared a patch for further review. I think the logging change and the pg_stat_checkpointer changes are different enough that they should be separate patches. If not just because the logging change seems to not have had any non-positive feedback. > In this patch, I have renamed the checkpoint_total_time to > last_checkpoint_duration, stats_reset has been kept as the last column > following the usual pattern, last_checkpoint_duration and > last_checkpoint_time will now be overwritten per checkpoint and also > have removed unnecessary lines as per the usual format. I had > successfully verified the checkpointer duration with different write > loads and I am attaching the observations for further reference. I am still not convinced of the usefulness of those changes to pg_stat_checkpointer, but some feedback on the patch: diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 9217508917..4a45f4f708 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -6778,7 +6778,7 @@ LogCheckpointEnd(bool restartpoint, int flags) /* Store in PendingCheckpointerStats */ - PendingCheckpointerStats.checkpoint_total_time += (double) total_msecs; + PendingCheckpointerStats.last_checkpoint_duration = (double) total_msecs; PendingCheckpointerStats.last_checkpoint_time = CheckpointStats.ckpt_end_t; [...] diff --git a/src/include/pgstat.h b/src/include/pgstat.h index a8eb1f8add..73688041c8 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -263,7 +263,7 @@ typedef struct PgStat_CheckpointerStats PgStat_Counter sync_time; PgStat_Counter buffers_written; PgStat_Counter slru_written; - PgStat_Counter checkpoint_total_time; /* new: total ms of last checkpoint */ + PgStat_Counter last_checkpoint_duration; /* new: total ms of last checkpoint */ TimestampTz last_checkpoint_time; /* new: end time of last checkpoint */ TimestampTz stat_reset_timestamp; } PgStat_CheckpointerStats; This looks like an incremental patch based on your original one? It is customary to send the full, updated, patch again. Michael