public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Banck <[email protected]>
To: Soumya S Murali <[email protected]>
Cc: Fujii Masao <[email protected]>
Cc: VASUKI M <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Álvaro Herrera <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: [PATCH] Expose checkpoint reason to completion log messages.
Date: Mon, 12 Jan 2026 11:26:58 +0100
Message-ID: <[email protected];lightning.p46.dedyn.io> (raw)
In-Reply-To: <CAMtXxw_o3TTaUiTqqwMCEMtLNESX9-Dvd60=CkgQO37KW1AkXQ@mail.gmail.com>
References: <CAMtXxw-Pvpznd_QczTYTsVhn=19qiX5N7X6DG6p9iyZdNDs7yw@mail.gmail.com>
<CAHGQGwFzR=1EwefWGVNF5GaOMsNkWZZ9oJMXiwO3x8AhjUTqsQ@mail.gmail.com>
<[email protected]>
<CAE2r8H7yMJ2XncBM59-n0jMOGDSw3R2gLn6Cd_-eq_y-3NPw=w@mail.gmail.com>
<CAHGQGwGXoai8-83p5x2FhHnu6LD=epsapa_RTTzM3RHShju3Jw@mail.gmail.com>
<CAMtXxw-AuUyyGj8V7k0x9GgftoDd-DovMDedH0vTFv32BEpeXA@mail.gmail.com>
<[email protected]>
<CAMtXxw9qg+-G24Xe-P=a2gWmCGnE4-CcOBOZ9tdO9eAqjMgEvQ@mail.gmail.com>
<[email protected]>
<CAMtXxw_o3TTaUiTqqwMCEMtLNESX9-Dvd60=CkgQO37KW1AkXQ@mail.gmail.com>
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
view thread (13+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [PATCH] Expose checkpoint reason to completion log messages.
In-Reply-To: <[email protected];lightning.p46.dedyn.io>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox