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.work@gmail.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