Re: walsender.c comment with no context is hard to understand

Amit Kapila <amit.kapila16@gmail.com>

From: Amit Kapila <amit.kapila16@gmail.com>
To: Peter Smith <smithpb2250@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, vignesh C <vignesh21@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-06-28T06:18:26Z
Lists: pgsql-hackers
On Fri, Jun 28, 2024 at 5:15 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Thu, Jun 27, 2024 at 3:44 PM Michael Paquier <michael@paquier.xyz> wrote:
> >
> > On Wed, Jun 26, 2024 at 02:30:26PM +0530, vignesh C wrote:
> > > On Mon, 3 Jun 2024 at 11:21, Peter Smith <smithpb2250@gmail.com> wrote:
> > >> Perhaps the comment should say something like it used to:
> > >> /* Fail if there is not enough WAL available. This can happen during
> > >> shutdown. */
> > >
> > > Agree with this, +1 for this change.
> >
> > That would be an improvement.  Would you like to send a patch with all
> > the areas you think could stand for improvements?
> > --
>
> OK, I attached a patch equivalent of the suggestion in this thread.
>

Shouldn't the check for flushptr (if (flushptr < targetPagePtr +
reqLen)) be moved immediately after the call to WalSndWaitForWal().
The comment seems to suggests the same: "Make sure we have enough WAL
available before retrieving the current timeline. .."

-- 
With Regards,
Amit Kapila.



Commits

  1. To improve the code, move the error check in logical_read_xlog_page().

  2. Allow logical decoding on standbys

  3. Fix walsender to exit promptly if client requests shutdown.