>
> Hi all,
>
> On Wed, Jan 7, 2026 at 1:37 PM Michael Banck <[email protected]> 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)
