On Wed, Jan 11, 2023 at 5:33 PM Andres Freund <and...@anarazel.de> wrote:
> Hi, > > On 2023-01-11 10:29:06 +0100, Anthonin Bonnefoy wrote: > > Currently, the Checkpointer process only reports SLRU statistics at > server > > shutdown, leading to delayed statistics for SLRU flushes. This patch > adds a > > flush of SLRU stats to the end of checkpoints. > > Hm. I wonder if we should do this even earlier, by the > pgstat_report_checkpointer() calls in CheckpointWriteDelay(). > > I'm inclined to move the pgstat_report_wal() and pgstat_report_slru() calls > into pgstat_report_checkpointer() to avoid needing to care about all the > individual places. > That would make sense. I've created a new patch with everything moved in pgstat_report_checkpointer(). I did split the checkpointer flush in a pgstat_flush_checkpointer() function as it seemed more readable. Thought? > > @@ -505,6 +505,7 @@ CheckpointerMain(void) > > /* Report pending statistics to the cumulative stats > system */ > > pgstat_report_checkpointer(); > > pgstat_report_wal(true); > > + pgstat_report_slru(true); > > Why do we need a force parameter if all callers use it? Good point. I've written the same signature as pgstat_report_wal but there's no need for the nowait parameter.
flush-slru-counters-v2.patch
Description: Binary data