| From: | Soumya S Murali <soumyamurali(dot)work(at)gmail(dot)com> |
|---|---|
| To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Cc: | Michael Banck <mbanck(at)gmx(dot)net>, VASUKI M <vasukianand0119(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>, melanieplageman(at)gmail(dot)com, juanjo(dot)santamaria(at)gmail(dot)com |
| Subject: | Re: [PATCH] Expose checkpoint reason to completion log messages. |
| Date: | 2026-02-17 10:47:48 |
| Message-ID: | CAMtXxw9GesU4M5UEfskd-d4MVvECQr3dmYi3e4CnLyvAemFK1w@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi all,
On Tue, Feb 17, 2026 at 11:25 AM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> On Wed, Jan 14, 2026 at 7:11 PM Soumya S Murali
> <soumyamurali(dot)work(at)gmail(dot)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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | VASUKI M | 2026-02-17 10:47:50 | Re: [OAuth2] Infrastructure for tracking token expiry time |
| Previous Message | VASUKI M | 2026-02-17 10:42:58 | Proposal: ANALYZE (MODIFIED_STATS) using autoanalyze thresholds |