| 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 |
| 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 |