> Indeed PendingCheckpointerStats.buf_written_checkpoints needs to count > buffer writes in SlruInternalWritePage(). However, does it need to be > done immediately there? The stats will not be visible to the users > until the next pgstat_report_checkpointer(). Incrementing > buf_written_checkpoints in BufferSync() makes sense as the > pgstat_report_checkpointer() gets called in there via > CheckpointWriteDelay() and it becomes visible to the user immediately. > Have you checked if pgstat_report_checkpointer() gets called while the > checkpoint calls SlruInternalWritePage()? If not, then you can just > assign ckpt_bufs_written to buf_written_checkpoints in > LogCheckpointEnd() like its other friends > checkpoint_write_time and checkpoint_sync_time.
In case of an immediate checkpoint, the CheckpointWriteDelay() never gets called until the checkpoint is completed. So no issues in this case. CheckpointWriteDelay() comes into picture in case of non immediate checkpoints (i.e. checkpoint timeout is reached or max wal size is reached). If we remove the increment in BufferSync() and SlruInternalWritePage() and then just assign ckpt_bufs_written to buf_written_checkpoints in LogCheckpointEnd() then the update will be available after the end of each checkpoint which is not better than the existing behaviour (without patch). If we keep the increment in BufferSync() then we have to calculate the remaining buffer incremented in SlruInternalWritePage() and then increment buf_written_checkpoints with this number in LogCheckpointEnd(). This just makes it complicated and again the buffer incremented in SlruInternalWritePage() will get updated at the end of the checkpoint. In the case of checkpoint_write_time and checkpoint_sync_time, it makes sense because this information is based on the entire checkpoint operation and it should be done at the end. So I feel the patch handles it in a better way even though the pgstat_report_checkpointer() does not get called immediately but it will be called during the next increment in BufferSync() which is before the end of the checkpoint. Please share if you have any other ideas. On Wed, Dec 14, 2022 at 4:55 PM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > On Wed, Dec 14, 2022 at 1:02 PM Nitin Jadhav > <nitinjadhavpostg...@gmail.com> wrote: > > > > Hi, > > > > While working on checkpoint related stuff, I have encountered that > > there is some inconsistency while reporting checkpointer stats. When a > > checkpoint gets completed, a checkpoint complete message gets logged. > > This message has a lot of information including the buffers written > > (CheckpointStats.ckpt_bufs_written). This variable gets incremented in > > 2 cases. First is in BufferSync() and the second is in > > SlruInternalWritePage(). On the other hand the checkpointer stats > > exposed using pg_stat_bgwriter contains a lot of information including > > buffers written (PendingCheckpointerStats.buf_written_checkpoints). > > This variable gets incremented in only one place and that is in > > BufferSync(). So there is inconsistent behaviour between these two > > data. Please refer to the sample output below. > > > > In order to fix this, the > > PendingCheckpointerStats.buf_written_checkpoints should be incremented > > in SlruInternalWritePage() similar to > > CheckpointStats.ckpt_bufs_written. I have attached the patch for the > > same. Please share your thoughts. > > Indeed PendingCheckpointerStats.buf_written_checkpoints needs to count > buffer writes in SlruInternalWritePage(). However, does it need to be > done immediately there? The stats will not be visible to the users > until the next pgstat_report_checkpointer(). Incrementing > buf_written_checkpoints in BufferSync() makes sense as the > pgstat_report_checkpointer() gets called in there via > CheckpointWriteDelay() and it becomes visible to the user immediately. > Have you checked if pgstat_report_checkpointer() gets called while the > checkpoint calls SlruInternalWritePage()? If not, then you can just > assign ckpt_bufs_written to buf_written_checkpoints in > LogCheckpointEnd() like its other friends > checkpoint_write_time and checkpoint_sync_time. > > -- > Bharath Rupireddy > PostgreSQL Contributors Team > RDS Open Source Databases > Amazon Web Services: https://aws.amazon.com