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 <[email protected]> > 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
