Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Log checkpoint request flags in checkpoint completion messages.

  1. [PATCH] Expose checkpoint reason to completion log messages.

    Soumya S Murali <soumyamurali.work@gmail.com> — 2025-12-01T11:18:56Z

    Hi all,

This patch is an update after reworking the “checkpoint reason” changes as
a standalone patch, separate from the pg_stat_checkpointer additions as
suggested [1]. I applied the patch on a clean tree and verified that the
logging changes work as expected under different workloads. I am attaching
the observations and patch in support for this.This would improve clarity
for performance debugging and help understand checkpoint behavior without
parsing WAL logs manually. Below is one representative checkpoint log entry
after a pgbench run and an explicit CHECKPOINT:

2025-12-01 15:33:30.121 IST [69178] LOG:  checkpoint complete (immediate):
wrote 3417 buffers (20.9%), wrote 3 SLRU buffers; 0 WAL file(s) added, 0
removed, 1 recycled; write=0.122 s, sync=0.022 s, total=0.166 s; sync
files=9, longest=0.005 s, average=0.003 s; distance=31304 kB,
estimate=489729 kB; lsn=0/65E92BC8, redo lsn=0/65E92B70

Regarding the pg_stat_checkpointer extensions [1], I understand the
concerns that were raised and I will follow up with a separate full patch
once I incorporate the remaining feedback.
Thank you for the guidance. It has been very helpful. Looking forward to
more further feedback.

Regards,
Soumya

Reference
[1]
https://www.postgresql.org/message-id/flat/CAMtXxw_W6w2Q1QsCvMPnoq3xCMKzH28Zjk_EmL60oP%2BsCTkXOw%40mail.gmail.com

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

    Andres Freund <andres@anarazel.de> — 2025-12-18T00:19:38Z

    Hi,

On 2025-12-01 16:48:56 +0530, Soumya S Murali wrote:
> This patch is an update after reworking the “checkpoint reason” changes as
> a standalone patch, separate from the pg_stat_checkpointer additions as
> suggested [1].

This seems to not pass the tests on any platform:
https://cirrus-ci.com/github/postgresql-cfbot/postgresql/cf%2F6306

Looks like you need to update 019_replslot_limit.pl for the changed log
format.

I suggest running the tests before submitting.

Greetings,

Andres

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

    Soumya S Murali <soumyamurali.work@gmail.com> — 2025-12-18T04:46:06Z

    Hi Andres,


On Thu, Dec 18, 2025 at 5:49 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2025-12-01 16:48:56 +0530, Soumya S Murali wrote:
> > This patch is an update after reworking the “checkpoint reason” changes as
> > a standalone patch, separate from the pg_stat_checkpointer additions as
> > suggested [1].
>
> This seems to not pass the tests on any platform:
> https://cirrus-ci.com/github/postgresql-cfbot/postgresql/cf%2F6306
>
> Looks like you need to update 019_replslot_limit.pl for the changed log
> format.
>
> I suggest running the tests before submitting.
>
> Greetings,
>
> Andres


Thank you for checking and for pointing this out. I will fix the
expected log pattern, rerun the test, and will resend an updated patch
once all tests get passed.

Regards,
Soumya

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

    Soumya S Murali <soumyamurali.work@gmail.com> — 2026-01-01T08:09:32Z

    Hi all,

On Thu, Dec 18, 2025 at 5:49 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2025-12-01 16:48:56 +0530, Soumya S Murali wrote:
> > This patch is an update after reworking the “checkpoint reason” changes as
> > a standalone patch, separate from the pg_stat_checkpointer additions as
> > suggested [1].
>
> This seems to not pass the tests on any platform:
> https://cirrus-ci.com/github/postgresql-cfbot/postgresql/cf%2F6306
>
> Looks like you need to update 019_replslot_limit.pl for the changed log
> format.
>
> I suggest running the tests before submitting.
>
> Greetings,
>
> Andres


I have updated my patch and have tested successfully. Also had rerun
the entire recovery TAP suite and every test got passed successfully.
I am attaching the updated patch herewith.


Regards,
Soumya

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-01-06T05:41:16Z

    On Thu, Jan 1, 2026 at 5:09 PM Soumya S Murali
<soumyamurali.work@gmail.com> wrote:
>
> Hi all,
>
> On Thu, Dec 18, 2025 at 5:49 AM Andres Freund <andres@anarazel.de> wrote:
> >
> > Hi,
> >
> > On 2025-12-01 16:48:56 +0530, Soumya S Murali wrote:
> > > This patch is an update after reworking the “checkpoint reason” changes as
> > > a standalone patch, separate from the pg_stat_checkpointer additions as
> > > suggested [1].

This kind of information already seems to be included in the
checkpoint starting log message, for example:

    LOG:  checkpoint starting: fast force wait

Why do you want to include essentially the same information in the
checkpoint ending log message as well?

Regards,

-- 
Fujii Masao

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

    Michael Banck <mbanck@gmx.net> — 2026-01-06T10:38:47Z

    Hi,

On Tue, Jan 06, 2026 at 02:41:16PM +0900, Fujii Masao wrote:
> On Thu, Jan 1, 2026 at 5:09 PM Soumya S Murali
> <soumyamurali.work@gmail.com> wrote:
> > On Thu, Dec 18, 2025 at 5:49 AM Andres Freund <andres@anarazel.de> wrote:
> > > On 2025-12-01 16:48:56 +0530, Soumya S Murali wrote:
> > > > This patch is an update after reworking the “checkpoint reason” changes as
> > > > a standalone patch, separate from the pg_stat_checkpointer additions as
> > > > suggested [1].
> 
> This kind of information already seems to be included in the
> checkpoint starting log message, for example:
> 
>     LOG:  checkpoint starting: fast force wait
> 
> Why do you want to include essentially the same information in the
> checkpoint ending log message as well?

I do think it is useful; the checkpoint finished message has a lot more
information than the checkpoint starting meessage (which basically only
mentions the reason) and when you extract log messages you currently
need to backtrack from the checkpoint finished message to the
corresponding checkpoint starting message to find out the type. Seeing
how the checkpoint finished message is so much more verbose anyway, I
think adding the reason to it is reasonably and helpful.


Michael

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

    VASUKI M <vasukianand0119@gmail.com> — 2026-01-06T11:10:00Z

    Yeah,the proposed idea makes sense to me,as users will almost see at the
end of log messages it will be easy to find out what is the checkpoint
reason.
+1 for it.

Regards,
Vasuki M

On Tue, Jan 6, 2026 at 4:26 PM Michael Banck <mbanck@gmx.net> wrote:

> Hi,
>
> On Tue, Jan 06, 2026 at 02:41:16PM +0900, Fujii Masao wrote:
> > On Thu, Jan 1, 2026 at 5:09 PM Soumya S Murali
> > <soumyamurali.work@gmail.com> wrote:
> > > On Thu, Dec 18, 2025 at 5:49 AM Andres Freund <andres@anarazel.de>
> wrote:
> > > > On 2025-12-01 16:48:56 +0530, Soumya S Murali wrote:
> > > > > This patch is an update after reworking the “checkpoint reason”
> changes as
> > > > > a standalone patch, separate from the pg_stat_checkpointer
> additions as
> > > > > suggested [1].
> >
> > This kind of information already seems to be included in the
> > checkpoint starting log message, for example:
> >
> >     LOG:  checkpoint starting: fast force wait
> >
> > Why do you want to include essentially the same information in the
> > checkpoint ending log message as well?
>
> I do think it is useful; the checkpoint finished message has a lot more
> information than the checkpoint starting meessage (which basically only
> mentions the reason) and when you extract log messages you currently
> need to backtrack from the checkpoint finished message to the
> corresponding checkpoint starting message to find out the type. Seeing
> how the checkpoint finished message is so much more verbose anyway, I
> think adding the reason to it is reasonably and helpful.
>
>
> Michael
>
>
>
>
>

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-01-06T13:25:39Z

    On Tue, Jan 6, 2026 at 8:09 PM VASUKI M <vasukianand0119@gmail.com> wrote:
>
> Yeah,the proposed idea makes sense to me,as users will almost see at the end of log messages it will be easy to find out what is the checkpoint reason.
> +1 for it.

If we do this, it would be better to keep the wording consistent between
the checkpoint starting and completion messages. For example,
the patch represents CHECKPOINT_FAST as "immediate",
while the checkpoint starting message uses "fast"; those should match.

I'm also wondering why this information is currently included only in
the checkpoint starting message. I tried to find any past discussion on this,
but couldn’t find one.

Regards,

-- 
Fujii Masao

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

    Soumya S Murali <soumyamurali.work@gmail.com> — 2026-01-07T07:05:08Z

    Hi all,

Thank you for the review and feedback for raising these points.
While the checkpoint starting message already includes the reason, in
practice the checkpoint completion message is what users most often
look at for performance analysis since it contains the detailed
timing, buffer, and WAL statistics to refer to. When extracting or
aggregating logs, it is currently necessary to correlate the
completion message back to the corresponding starting message to
determine the checkpoint type, which is not always convenient or
reliable in busy systems. Including the reason directly in the
completion message avoids that extra step.
I agree with your point that the checkpoint completion message should
use the same terminology as the starting message ( “fast” rather than
“immediate”) and I have updated the patch accordingly and have
attached herewith for review. This patch aligns the checkpoint
completion log wording with the existing checkpoint starting message
(using “fast” instead of “immediate”) for consistency.
And regarding why this information has historically only been present
in the starting message, I think the completion log originally focused
purely on reporting I/O and timing statistics, rather than contextual
metadata. With the increasing use of log analysis for performance
troubleshooting, it is reasonable to include the checkpoint reason
with those statistics for the detailed analysis.
Kindly review my patch and let me know the feedback.

Regards,
Soumya

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

    Michael Banck <mbanck@gmx.net> — 2026-01-07T08:07:18Z

    Hi,

On Wed, Jan 07, 2026 at 12:35:08PM +0530, Soumya S Murali wrote:
> I agree with your point that the checkpoint completion message should
> use the same terminology as the starting message ( “fast” rather than
> “immediate”) and I have updated the patch accordingly and have
> attached herewith for review. This patch aligns the checkpoint
> completion log wording with the existing checkpoint starting message
> (using “fast” instead of “immediate”) for consistency.

First off, please send complete and not incremental patches.

From what I can tell, you currently also do not have "wal" as reason?
Also, you have "timed" vs "time" etc.

I think it would make most sense to factor out the logic for the
starting checkpoint reason from xlog.c ("(errmsg("checkpoint 
starting:%s%s%s%s%s%s%s%s")") and use that both for starting and
stopping.


Michael

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

    Soumya S Murali <soumyamurali.work@gmail.com> — 2026-01-08T06:28:43Z

    Hi all,

On Wed, Jan 7, 2026 at 1:37 PM Michael Banck <mbanck@gmx.net> wrote:
>
> Hi,
>
> On Wed, Jan 07, 2026 at 12:35:08PM +0530, Soumya S Murali wrote:
> > I agree with your point that the checkpoint completion message should
> > use the same terminology as the starting message ( “fast” rather than
> > “immediate”) and I have updated the patch accordingly and have
> > attached herewith for review. This patch aligns the checkpoint
> > completion log wording with the existing checkpoint starting message
> > (using “fast” instead of “immediate”) for consistency.
>
> First off, please send complete and not incremental patches.
>
> From what I can tell, you currently also do not have "wal" as reason?
> Also, you have "timed" vs "time" etc.
>
> I think it would make most sense to factor out the logic for the
> starting checkpoint reason from xlog.c ("(errmsg("checkpoint
> starting:%s%s%s%s%s%s%s%s")") and use that both for starting and
> stopping.
>
>
> Michael

Thank you for the review and feedback.
I have reworked the patch to factor out the checkpoint reason logic to
use the same wording for both checkpoint start and completion log
(including wal, time, fast, force, and wait). The format has been
aligned with existing conventions and has been verified by running
make check and recovery TAP tests including t/019_replslot_limit.pl
successfully.
Kindly review my patch and let me know the feedback.

Regards,
Soumya

  12. 回复: [PATCH] Expose checkpoint reason to completion log messages.

    li carol <carol.li2025@outlook.com> — 2026-01-08T13:31:49Z

    > 
> Hi all,
> 
> On Wed, Jan 7, 2026 at 1:37 PM Michael Banck <mbanck@gmx.net> wrote:
> >
> > Hi,
> >
> > On Wed, Jan 07, 2026 at 12:35:08PM +0530, Soumya S Murali wrote:
> > > I agree with your point that the checkpoint completion message
> > > should use the same terminology as the starting message ( “fast”
> > > rather than
> > > “immediate”) and I have updated the patch accordingly and have
> > > attached herewith for review. This patch aligns the checkpoint
> > > completion log wording with the existing checkpoint starting message
> > > (using “fast” instead of “immediate”) for consistency.
> >
> > First off, please send complete and not incremental patches.
> >
> > From what I can tell, you currently also do not have "wal" as reason?
> > Also, you have "timed" vs "time" etc.
> >
> > I think it would make most sense to factor out the logic for the
> > starting checkpoint reason from xlog.c ("(errmsg("checkpoint
> > starting:%s%s%s%s%s%s%s%s")") and use that both for starting and
> > stopping.
> >
> >
> > Michael
> 
> Thank you for the review and feedback.
> I have reworked the patch to factor out the checkpoint reason logic to use the
> same wording for both checkpoint start and completion log (including wal, time,
> fast, force, and wait). The format has been aligned with existing conventions
> and has been verified by running make check and recovery TAP tests including
> t/019_replslot_limit.pl successfully.
> Kindly review my patch and let me know the feedback.
> 
> Regards,
> Soumya

Hi Soumya,

Thanks for the updated patch. I agree that having the checkpoint reason in the completion log is very helpful for performance analysis, as it avoids the need to manually correlate "starting" and "complete" messages in busy logs.
I’ve reviewed the latest version and have a few suggestions to make the refactoring more complete:
1. Consistency in LogCheckpointStart
Currently, the checkpoint branch uses the new CheckpointReasonString helper, but the restartpoint branch still uses the old inline triple-operator logic. It would be better to use the helper function in both places to ensure they stay in sync.

2. Missing Reason for Restartpoints 
In LogCheckpointEnd, the checkpoint reason is added to the checkpoint complete message, but the restartpoint complete message remains unchanged. I think users would find the reason for restartpoints just as useful for debugging.

3. Global Variable vs. Passing Flags: Instead of using a file-level global checkpoint_reason_str to pass data between the start and end functions, would it make sense to simply pass the flags as a new argument to LogCheckpointEnd(bool restartpoint, int flags)? 
This would allow you to call the helper function directly inside the ereport calls and avoid maintaining global state.

4. Whitespace Errors: I noticed a few whitespace warnings (space before tabs) when applying the patch. 
/home/carol/0001-Expose-checkpoint-reason-in-checkpoint-completion-lo.patch:69: space before tab in indent.
        	CheckpointReasonString(flags),
/home/carol/0001-Expose-checkpoint-reason-in-checkpoint-completion-lo.patch:70: space before tab in indent.
        	sizeof(checkpoint_reason_str));
/home/carol/0001-Expose-checkpoint-reason-in-checkpoint-completion-lo.patch:88: space before tab in indent.
        		(errmsg("checkpoint starting:%s", checkpoint_reason_str)));
warning: 3 lines add whitespace errors.

The patch can be compiled successfully and can pass make check -C src/test/recovery/

Regards,
Yuan Li(carol)

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

    Michael Banck <mbanck@gmx.net> — 2026-01-08T17:36:13Z

    Hi,

On Thu, Jan 08, 2026 at 11:58:43AM +0530, Soumya S Murali wrote:
> On Wed, Jan 7, 2026 at 1:37 PM Michael Banck <mbanck@gmx.net> wrote:
> > On Wed, Jan 07, 2026 at 12:35:08PM +0530, Soumya S Murali wrote:
> I have reworked the patch to factor out the checkpoint reason logic to
> use the same wording for both checkpoint start and completion log
> (including wal, time, fast, force, and wait). The format has been
> aligned with existing conventions and has been verified by running
> make check and recovery TAP tests including t/019_replslot_limit.pl
> successfully.
> Kindly review my patch and let me know the feedback.

Thanks for the new patch.

> index 22d0a2e8c3..9f7439f9a2 100644
> --- a/src/backend/access/transam/xlog.c
> +++ b/src/backend/access/transam/xlog.c
[...]
> +static const char *
> +CheckpointReasonString(int flags)
> +{
> +	static char buf[128];
> +
> +	buf[0] = '\0';
> +
> +	if (flags & CHECKPOINT_IS_SHUTDOWN)
> +		strcat(buf, " shutdown");
> +	if (flags & CHECKPOINT_END_OF_RECOVERY)
> +		strcat(buf, " end-of-recovery");
> +	if (flags & CHECKPOINT_FAST)
> +		strcat(buf, " fast");
> +	if (flags & CHECKPOINT_FORCE)
> +		strcat(buf, " force");
> +	if (flags & CHECKPOINT_WAIT)
> +		strcat(buf, " wait");
> +	if (flags & CHECKPOINT_CAUSE_XLOG)
> +		strcat(buf, " wal");
> +	if (flags & CHECKPOINT_CAUSE_TIME)
> +		strcat(buf, " time");
> +	if (flags & CHECKPOINT_FLUSH_UNLOGGED)
> +		strcat(buf, " flush-unlogged");
> +
> +	return buf;
> +}

The way this is coded now prepends the final string with a space
character. This is fine for the checkpoint starting message, but makes
the checkpoint finished message look slightly weird (not the "( wal)"):

|2026-01-08 18:30:33.691 CET [1666416] LOG:  checkpoint complete ( wal):
|wrote 20 buffers (0.1%), wrote 0 SLRU buffers; 0 WAL file(s) added, 4
|removed, 33 recycled; write=2.221 s, sync=0.019 s, total=2.701 s; sync
|files=3, longest=0.018 s, average=0.007 s; distance=601885 kB,
|estimate=601885 kB; lsn=0/D2D59FC8, redo lsn=0/B492B500

Also I think the translators might now complain, but I am not sure how
to best approach this.


Michael

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

    Soumya S Murali <soumyamurali.work@gmail.com> — 2026-01-12T07:23:54Z

    Hi all,

On Thu, Jan 8, 2026 at 11:06 PM Michael Banck <mbanck@gmx.net> wrote:
>
> Hi,
>
> On Thu, Jan 08, 2026 at 11:58:43AM +0530, Soumya S Murali wrote:
> > On Wed, Jan 7, 2026 at 1:37 PM Michael Banck <mbanck@gmx.net> wrote:
> > > On Wed, Jan 07, 2026 at 12:35:08PM +0530, Soumya S Murali wrote:
> > I have reworked the patch to factor out the checkpoint reason logic to
> > use the same wording for both checkpoint start and completion log
> > (including wal, time, fast, force, and wait). The format has been
> > aligned with existing conventions and has been verified by running
> > make check and recovery TAP tests including t/019_replslot_limit.pl
> > successfully.
> > Kindly review my patch and let me know the feedback.
>
> Thanks for the new patch.
>
> > index 22d0a2e8c3..9f7439f9a2 100644
> > --- a/src/backend/access/transam/xlog.c
> > +++ b/src/backend/access/transam/xlog.c
> [...]
> > +static const char *
> > +CheckpointReasonString(int flags)
> > +{
> > +     static char buf[128];
> > +
> > +     buf[0] = '\0';
> > +
> > +     if (flags & CHECKPOINT_IS_SHUTDOWN)
> > +             strcat(buf, " shutdown");
> > +     if (flags & CHECKPOINT_END_OF_RECOVERY)
> > +             strcat(buf, " end-of-recovery");
> > +     if (flags & CHECKPOINT_FAST)
> > +             strcat(buf, " fast");
> > +     if (flags & CHECKPOINT_FORCE)
> > +             strcat(buf, " force");
> > +     if (flags & CHECKPOINT_WAIT)
> > +             strcat(buf, " wait");
> > +     if (flags & CHECKPOINT_CAUSE_XLOG)
> > +             strcat(buf, " wal");
> > +     if (flags & CHECKPOINT_CAUSE_TIME)
> > +             strcat(buf, " time");
> > +     if (flags & CHECKPOINT_FLUSH_UNLOGGED)
> > +             strcat(buf, " flush-unlogged");
> > +
> > +     return buf;
> > +}
>
> The way this is coded now prepends the final string with a space
> character. This is fine for the checkpoint starting message, but makes
> the checkpoint finished message look slightly weird (not the "( wal)"):
>
> |2026-01-08 18:30:33.691 CET [1666416] LOG:  checkpoint complete ( wal):
> |wrote 20 buffers (0.1%), wrote 0 SLRU buffers; 0 WAL file(s) added, 4
> |removed, 33 recycled; write=2.221 s, sync=0.019 s, total=2.701 s; sync
> |files=3, longest=0.018 s, average=0.007 s; distance=601885 kB,
> |estimate=601885 kB; lsn=0/D2D59FC8, redo lsn=0/B492B500
>
> Also I think the translators might now complain, but I am not sure how
> to best approach this.

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

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

    Michael Banck <mbanck@gmx.net> — 2026-01-12T10:26:58Z

    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

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

    Soumya S Murali <soumyamurali.work@gmail.com> — 2026-01-13T06:29:43Z

    Hi all,

On Mon, Jan 12, 2026 at 3:57 PM Michael Banck <mbanck@gmx.net> wrote:
>
> 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.


Thank you for the detailed review and suggestions.
I have reworked the patch on a fresh tree, addressed the formatting
and indentation issues, and aligned the helper implementation with
PostgreSQL coding conventions. The updated patch has been pgindented
and validated with make check and the full recovery TAP test suite. I
am attaching the revised patch for further review.

Regards,
Soumya

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-01-13T13:09:49Z

    On Tue, Jan 13, 2026 at 3:29 PM Soumya S Murali
<soumyamurali.work@gmail.com> wrote:
> Thank you for the detailed review and suggestions.
> I have reworked the patch on a fresh tree, addressed the formatting
> and indentation issues, and aligned the helper implementation with
> PostgreSQL coding conventions. The updated patch has been pgindented
> and validated with make check and the full recovery TAP test suite. I
> am attaching the revised patch for further review.

Thanks for updating the patch!

With this patch, the checkpoint log messages look like this:

    LOG:  checkpoint starting: fast force wait
    LOG:  checkpoint complete (fast force wait): ...

The formatting of the checkpoint flags differs between the starting and
complete messages: the complete message wraps them in parentheses,
while the starting message does not. I think it would be better to log
the flags in a consistent way in both messages. For example:

    LOG:  checkpoint starting: fast force wait
    LOG:  checkpoint complete: fast force wait: ...


- if ($node_primary->log_contains("checkpoint complete: ", $logstart))
+ if ($node_primary->log_contains(qr/checkpoint complete(?:
\([^)]*\))?:/, $logstart))

If we adopt a format like the above example, this test change would
no longer be needed.


+#define APPEND_REASON(str)                          \
+ do                                              \
+ {                                               \
+ if (!first)                                 \

Wouldn't it be simpler to just build the string with snprintf() based on
the flags? For example:

---------------------------------------------------
static char buf[128];

snprintf(buf, sizeof(buf), "%s%s%s%s%s%s%s%s",
(flags & CHECKPOINT_IS_SHUTDOWN) ? " shutdown" : "",
(flags & CHECKPOINT_END_OF_RECOVERY) ? " end-of-recovery" : "",
(flags & CHECKPOINT_FAST) ? " fast" : "",
(flags & CHECKPOINT_FORCE) ? " force" : "",
(flags & CHECKPOINT_WAIT) ? " wait" : "",
(flags & CHECKPOINT_CAUSE_XLOG) ? " wal" : "",
(flags & CHECKPOINT_CAUSE_TIME) ? " time" : "",
(flags & CHECKPOINT_FLUSH_UNLOGGED) ? " flush-unlogged" : "");

return buf;
---------------------------------------------------


+ * Format checkpoint reason flags consistently for log messages.
+ * The returned string is suitable for inclusion after
+ * "checkpoint starting:" or inside "checkpoint complete (...)".
+ */
+static const char *
+CheckpointReasonString(int flags)

These flags include more than just the reason a checkpoint was triggered,
so using "reason" here seems misleading. Wouldn't just "flags" be a better term?

Regards,

-- 
Fujii Masao

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

    Soumya S Murali <soumyamurali.work@gmail.com> — 2026-01-14T10:11:36Z

    Hi all,

On Tue, Jan 13, 2026 at 6:40 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Tue, Jan 13, 2026 at 3:29 PM Soumya S Murali
> <soumyamurali.work@gmail.com> wrote:
> > Thank you for the detailed review and suggestions.
> > I have reworked the patch on a fresh tree, addressed the formatting
> > and indentation issues, and aligned the helper implementation with
> > PostgreSQL coding conventions. The updated patch has been pgindented
> > and validated with make check and the full recovery TAP test suite. I
> > am attaching the revised patch for further review.
>
> Thanks for updating the patch!
>
> With this patch, the checkpoint log messages look like this:
>
>     LOG:  checkpoint starting: fast force wait
>     LOG:  checkpoint complete (fast force wait): ...
>
> The formatting of the checkpoint flags differs between the starting and
> complete messages: the complete message wraps them in parentheses,
> while the starting message does not. I think it would be better to log
> the flags in a consistent way in both messages. For example:
>
>     LOG:  checkpoint starting: fast force wait
>     LOG:  checkpoint complete: fast force wait: ...
>
>
> - if ($node_primary->log_contains("checkpoint complete: ", $logstart))
> + if ($node_primary->log_contains(qr/checkpoint complete(?:
> \([^)]*\))?:/, $logstart))
>
> If we adopt a format like the above example, this test change would
> no longer be needed.
>
>
> +#define APPEND_REASON(str)                          \
> + do                                              \
> + {                                               \
> + if (!first)                                 \
>
> Wouldn't it be simpler to just build the string with snprintf() based on
> the flags? For example:
>
> ---------------------------------------------------
> static char buf[128];
>
> snprintf(buf, sizeof(buf), "%s%s%s%s%s%s%s%s",
> (flags & CHECKPOINT_IS_SHUTDOWN) ? " shutdown" : "",
> (flags & CHECKPOINT_END_OF_RECOVERY) ? " end-of-recovery" : "",
> (flags & CHECKPOINT_FAST) ? " fast" : "",
> (flags & CHECKPOINT_FORCE) ? " force" : "",
> (flags & CHECKPOINT_WAIT) ? " wait" : "",
> (flags & CHECKPOINT_CAUSE_XLOG) ? " wal" : "",
> (flags & CHECKPOINT_CAUSE_TIME) ? " time" : "",
> (flags & CHECKPOINT_FLUSH_UNLOGGED) ? " flush-unlogged" : "");
>
> return buf;
> ---------------------------------------------------
>
>
> + * Format checkpoint reason flags consistently for log messages.
> + * The returned string is suitable for inclusion after
> + * "checkpoint starting:" or inside "checkpoint complete (...)".
> + */
> +static const char *
> +CheckpointReasonString(int flags)
>
> These flags include more than just the reason a checkpoint was triggered,
> so using "reason" here seems misleading. Wouldn't just "flags" be a better term?
>


Thank you for the detailed review and suggestions.
Based on the feedback, I have updated the patch to make the checkpoint
and restart point log formatting fully consistent between start and
completion log messages. The completion messages now use the same
format as the start messages without parentheses. I have also replaced
the term "reason" with the term "flags". The patch has been rebuilt
and validated with make check and the full recovery TAP test suite,
and I have manually verified the resulting log output to confirm the
expected formatting. I am attaching the updated patch for further
review. Please let me know if any additional adjustments are needed.


Regards,
Soumya

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-02-17T05:55:29Z

    On Wed, Jan 14, 2026 at 7:11 PM Soumya S Murali
<soumyamurali.work@gmail.com> wrote:
> Thank you for the detailed review and suggestions.
> Based on the feedback, I have updated the patch to make the checkpoint
> and restart point log formatting fully consistent between start and
> completion log messages. The completion messages now use the same
> format as the start messages without parentheses. I have also replaced
> the term "reason" with the term "flags". The patch has been rebuilt
> and validated with make check and the full recovery TAP test suite,
> and I have manually verified the resulting log output to confirm the
> expected formatting. I am attaching the updated patch for further
> review. Please let me know if any additional adjustments are needed.

Thanks for updating the patch!

+ if (flags & CHECKPOINT_IS_SHUTDOWN)
+ APPEND_REASON("shutdown");
+ if (flags & CHECKPOINT_END_OF_RECOVERY)
+ APPEND_REASON("end-of-recovery");

I still think it would be simpler to construct the checkpoint flags string
with snprintf() rather than introducing a new macro. Please see
the attached patch.

- if ($node_primary->log_contains("checkpoint complete: ", $logstart))
+ if ($node_primary->log_contains("checkpoint complete:", $logstart))

I don't think this change is necessary.

I've also updated the commit message in the attached patch.

Regards,

-- 
Fujii Masao

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

    Soumya S Murali <soumyamurali.work@gmail.com> — 2026-02-17T10:47:48Z

    Hi all,

On Tue, Feb 17, 2026 at 11:25 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Wed, Jan 14, 2026 at 7:11 PM Soumya S Murali
> <soumyamurali.work@gmail.com> wrote:
> > Thank you for the detailed review and suggestions.
> > Based on the feedback, I have updated the patch to make the checkpoint
> > and restart point log formatting fully consistent between start and
> > completion log messages. The completion messages now use the same
> > format as the start messages without parentheses. I have also replaced
> > the term "reason" with the term "flags". The patch has been rebuilt
> > and validated with make check and the full recovery TAP test suite,
> > and I have manually verified the resulting log output to confirm the
> > expected formatting. I am attaching the updated patch for further
> > review. Please let me know if any additional adjustments are needed.
>
> Thanks for updating the patch!
>
> + if (flags & CHECKPOINT_IS_SHUTDOWN)
> + APPEND_REASON("shutdown");
> + if (flags & CHECKPOINT_END_OF_RECOVERY)
> + APPEND_REASON("end-of-recovery");
>
> I still think it would be simpler to construct the checkpoint flags string
> with snprintf() rather than introducing a new macro. Please see
> the attached patch.
>
> - if ($node_primary->log_contains("checkpoint complete: ", $logstart))
> + if ($node_primary->log_contains("checkpoint complete:", $logstart))
>
> I don't think this change is necessary.
>
> I've also updated the commit message in the attached patch.


Thank you very much for refining the implementation and simplifying
the flag construction logic using snprintf(). I have reviewed the
committed changes and verified that the formatting and behavior are
consistent and correct. The updated approach is clear and aligns well
with PostgreSQL coding style. I also appreciate the improvements made
to the commit message as it clearly explains the motivation and makes
the change easy to understand in context.
I sincerely appreciate your guidance throughout this review process.


Regards,
Soumya

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-02-19T14:57:24Z

    On Tue, Feb 17, 2026 at 7:47 PM Soumya S Murali
<soumyamurali.work@gmail.com> wrote:
> Thank you very much for refining the implementation and simplifying
> the flag construction logic using snprintf(). I have reviewed the
> committed changes and verified that the formatting and behavior are
> consistent and correct. The updated approach is clear and aligns well
> with PostgreSQL coding style. I also appreciate the improvements made
> to the commit message as it clearly explains the motivation and makes
> the change easy to understand in context.

I've pushed the patch. Thanks!

Regards,

-- 
Fujii Masao

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

    Soumya S Murali <soumyamurali.work@gmail.com> — 2026-02-20T05:25:48Z

    Hi all,

On Thu, Feb 19, 2026 at 8:27 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Tue, Feb 17, 2026 at 7:47 PM Soumya S Murali
> <soumyamurali.work@gmail.com> wrote:
> > Thank you very much for refining the implementation and simplifying
> > the flag construction logic using snprintf(). I have reviewed the
> > committed changes and verified that the formatting and behavior are
> > consistent and correct. The updated approach is clear and aligns well
> > with PostgreSQL coding style. I also appreciate the improvements made
> > to the commit message as it clearly explains the motivation and makes
> > the change easy to understand in context.
>
> I've pushed the patch. Thanks!
>

Thank you so much for pushing the patch. I truly appreciate your
guidance and the detailed review throughout this process. It has been
a valuable learning experience.
Thank you.

Regards,
Soumya