Re: PoC: history of recent vacuum/checkpoint runs (using new hooks)

On Thu, Dec 26, 2024 at 06:58:11PM +0100, Tomas Vondra wrote:
> If 128MB is insufficient, why would 256MB be OK? A factor of 2x does not
> make a fundamental difference ...
>
> Anyway, the 128MB value is rather arbitrary. I don't mind increasing the
> limit, or possibly removing it entirely (and accepting anything the
> system can handle).

+ DefineCustomIntVariable("stats_history.size",
+ "Sets the amount of memory available for past events.",

How about some time-based retention? Data size can be hard to think
about for the end user, while there would be a set of users that would
want to retain data for the past week, month, etc? If both size and
time upper-bound are define, then entries that match one condition or
the other are removed.

+ checkpoint_log_hook(
+ CheckpointStats.ckpt_start_t, /* start_time */
+ CheckpointStats.ckpt_end_t, /* end_time */
+ (flags & CHECKPOINT_IS_SHUTDOWN), /* is_shutdown */
+ (flags & CHECKPOINT_END_OF_RECOVERY), /* is_end_of_recovery */
+ (flags & CHECKPOINT_IMMEDIATE), /* is_immediate */
+ (flags & CHECKPOINT_FORCE), /* is_force */
+ (flags & CHECKPOINT_WAIT), /* is_wait */
+ (flags & CHECKPOINT_CAUSE_XLOG), /* is_wal */
+ (flags & CHECKPOINT_CAUSE_TIME), /* is_time */
+ (flags & CHECKPOINT_FLUSH_ALL), /* is_flush_all */
+ CheckpointStats.ckpt_bufs_written, /* buffers_written */
+ CheckpointStats.ckpt_slru_written, /* slru_written */
+ CheckpointStats.ckpt_segs_added, /* segs_added */
+ CheckpointStats.ckpt_segs_removed, /* segs_removed */
+ CheckpointStats.ckpt_segs_recycled, /* segs_recycled */

That's a lot of arguments. CheckpointStatsData and the various
CHECKPOINT_* flags are exposed, why not just send these values to the
hook?

For v1-0001 as well, I'd suggest some grouping with existing
structures, or expose these structures so as they can be reused for
out-of-core code via the proposed hook. More arguments lead to more
mistakes that could be easily avoided.
--
Michael