Re: [PATCH] Expose checkpoint reason to completion log messages.

Hi,

On Mon, Jan 12, 2026 at 12:53:54PM +0530, Soumya S Murali wrote:
> Based on the feedback received, I have reworked and implemented a
> helper function so that both checkpoint start and completion messages
> use the same wording and would remain in sync, extending the same
> reason reporting to restart points for completeness and better
> observability, ensuring consistent terminology with existing log
> output, had removed the previous use of global state and instead
> passed checkpoint flags explicitly and have fixed formatting and
> whitespace issues noted during review.
> Also, I have tested and validated the logic successfully. I have
> attached the updated patch for further review.
> Kindly review the patch and let me know the feedback.
>
> Regards,
> Soumya

> From 30e3c62695a643daf96ffca3b504a79d2a761f77 Mon Sep 17 00:00:00 2001
> From: Soumya <soumyamurali(dot)work(at)gmail(dot)com>
> Date: Mon, 12 Jan 2026 12:36:29 +0530
> Subject: [PATCH] Expose checkpoint reason in completion log messages
>
> ---
> src/backend/access/transam/xlog.c | 78 ++++++++++++++++-------
> src/test/recovery/t/019_replslot_limit.pl | 2 +-
> 2 files changed, 55 insertions(+), 25 deletions(-)
>
> diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
> index 22d0a2e8c3..63d1e56ca1 100644
> --- a/src/backend/access/transam/xlog.c
> +++ b/src/backend/access/transam/xlog.c
> @@ -6714,6 +6714,49 @@ ShutdownXLOG(int code, Datum arg)
> }
> }
>
> +/*
> +* Format checkpoint reason flags consistently for log messages.
> +* The returned string is suitable for inclusion after
> +* "checkpoint starting:" or inside "checkpoint complete (...)".
> +*/

Indentation of the comment is wrong here, the asterisks should be all on
the same column.
> +static const char *
> +CheckpointReasonString(int flags)
> +{
> + static char buf[128];
> + bool first = true;

pgindent complains about the above line as well:

| {
| static char buf[128];
|- bool first = true;
|+ bool first = true;
|
| buf[0] = '\0';

> + buf[0] = '\0';
> +
> +#define APPEND_REASON(str) \
> + do { \
> + if (!first) \
> + strcat(buf, " "); \
> + strcat(buf, (str)); \
> + first = false; \
> + } while (0)

Those ending backslashes also don't seem to line up when 4-space tabs
are set. But pgindent doesn't change them so maybe it is correct as-is?

I am also not sure whether the above is current best-practise for
Postgres code, I'd have to look deeper (or maybe somebody else can
review this).

> + if (flags & CHECKPOINT_IS_SHUTDOWN)
> + APPEND_REASON("shutdown");
> + if (flags & CHECKPOINT_END_OF_RECOVERY)
> + APPEND_REASON("end-of-recovery");
> + if (flags & CHECKPOINT_FAST)
> + APPEND_REASON("fast");
> + if (flags & CHECKPOINT_FORCE)
> + APPEND_REASON("force");
> + if (flags & CHECKPOINT_WAIT)
> + APPEND_REASON("wait");
> + if (flags & CHECKPOINT_CAUSE_XLOG)
> + APPEND_REASON("wal");
> + if (flags & CHECKPOINT_CAUSE_TIME)
> + APPEND_REASON("time");
> + if (flags & CHECKPOINT_FLUSH_UNLOGGED)
> + APPEND_REASON("flush-unlogged");
> +
> +#undef APPEND_REASON
> +
> + return buf;

I am still not sure whether this will be a regression for translation of
those strings.

Otherwise, the patch looks sane to me.

Michael