On Wed, Dec 21, 2022 at 5:15 PM Nitin Jadhav <nitinjadhavpostg...@gmail.com> wrote: > > I have modified the code accordingly and attached the new version of > patches. patch 0001 fixes the inconsistency in checkpointer stats and > patch 0002 separates main buffer and SLRU buffer count from checkpoint > complete log message.
IMO, there's no need for 2 separate patches for these changes. + (errmsg("restartpoint complete: wrote %d buffers (%.1f%%), " + "wrote %d slru buffers (%.1f%%); %d WAL file(s) added, " + "%d removed, %d recycled; write=%ld.%03d s, " + "sync=%ld.%03d s, total=%ld.%03d s; sync files=%d, " + "longest=%ld.%03d s, average=%ld.%03d s; distance=%d kB, " + "estimate=%d kB; lsn=%X/%X, redo lsn=%X/%X", Hm, restartpoint /checkpoint complete message is already too long to read and adding slru buffers to it make it further longer. Note that we don't need to add every checkpoint stat to the log message but to pg_stat_bgwriter. Isn't it enough to show SLRU buffers information in pg_stat_bgwriter alone? Can't one look at pg_stat_slru's blks_written (pgstat_count_slru_page_written()) to really track the SLRUs written? Or is it that one may want to track SLRUs during a checkpoint separately? Is there a real use-case/customer reported issue driving this change? After looking at pg_stat_slru.blks_written, I think the best way is to just leave things as-is and let CheckpointStats count slru buffers too unless there's really a reported issue that says pg_stat_slru.blks_written doesn't serve the purpose. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com