On Thu, Jun 4, 2026 at 6:32 PM Álvaro Herrera <alvherre@kurilemu.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
