On Fri, Jun 28, 2024 at 12:55 PM Peter Smith <smithpb2...@gmail.com> wrote: > > On Fri, Jun 28, 2024 at 4:18 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Fri, Jun 28, 2024 at 5:15 AM Peter Smith <smithpb2...@gmail.com> wrote: > > > > > > On Thu, Jun 27, 2024 at 3:44 PM Michael Paquier <mich...@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 <smithpb2...@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. .." > > > > Yes, as I wrote in the first post, those lines did once used to be > adjacent in logical_read_xlog_page. > > I also wondered if they still belonged together, but I opted for the > safest option of fixing only the comment instead of refactoring old > code when no problem had been reported. > > AFAICT these lines became separated due to a 2023 patch [1], and you > were one of the reviewers of that patch, so I assumed the code > separation was deemed OK at that time. Unless it was some mistake that > slipped past multiple reviewers? >
I don't know whether your assumption is correct. AFAICS, those two lines should be together. Let us ee if Bertrand remembers anything? -- With Regards, Amit Kapila.
