Re: Fix comments to reference xlogrecovery.c

From: Imran Zaheer <imran(dot)zhir(at)gmail(dot)com>
To: Álvaro Herrera <alvherre(at)kurilemu(dot)de>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix comments to reference xlogrecovery.c
Date: 2026-06-04 17:29:26
Message-ID: CA+UBfanPr6qD5TQ4m_eEYE_BuoneRTzKXX=PJm_YKtwc_sqY6g@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jun 4, 2026 at 6:32 PM Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:

> I think it would be better if the comments mentioned the function name
> where the operation in question takes place rather than the file name;
> for instance
>
> @@ -6223,7 +6223,7 @@ xact_redo_commit(xl_xact_parsed_commit *parsed,
> * If a transaction completion record arrives that has as-yet
> * unobserved subtransactions then this will not have been fully
> * handled by the call to RecordKnownAssignedTransactionIds() in the
> - * main recovery loop in xlog.c. So we need to do bookkeeping again to
> + * main recovery loop in PerformWalRecovery(). So we need to do bookkeeping again to
>
> and so on. I think this is more helpful for a reader because they have
> a precise point to jump to instead of having to scroll through the file
> looking for the place. Also this doesn't get outdated if the function
> is moved to a different file (not that this happens very often.)
>

Yes I agree.

>
> > @@ -118,7 +118,7 @@ StartupProcShutdownHandler(SIGNAL_ARGS)
> > /*
> > * Re-read the config file.
> > *
> > - * If one of the critical walreceiver options has changed, flag xlog.c
> > + * If one of the critical walreceiver options has changed, flag xlogrecovery.c
> > * to restart it.
> > */
> > static void
>
> This could be, maybe "If ... has changed, make
> StartupRequestWalReceiverRestart() aware of that." or something like
> that.

I am not sure whether we should be mentioning the function name which
is just being called a few lines below. So maybe it's better to change
the wording here.

*
- * If one of the critical walreceiver options has changed, flag xlog.c
- * to restart it.
+ * If one of the critical walreceiver options has changed, request the startup
+ * process to restart the walreceiver.
*/

Let me know if this sounds ok.

I am attaching the new version.

Thanks
Imran Zaheer

Attachment Content-Type Size
v2-0001-Update-comments-to-refer-to-the-correct-functions.patch application/octet-stream 4.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Denis Rodionov 2026-06-04 18:27:05 Re: [PATCH] Remove obsolete tupDesc assignment in extended statistics
Previous Message Jeff Davis 2026-06-04 17:17:57 Re: Avoid orphaned objects dependencies, take 3