On Tue, Dec 6, 2022 at 2:51 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, Dec 6, 2022 at 5:27 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > Here are my review comments for patch v55-0002
> >
> ...
> > 4.
> >
> > /*
> > + * Replay the spooled messages in the parallel apply worker if the leader apply
> > + * worker has finished serializing changes to the file.
> > + */
> > +static void
> > +pa_spooled_messages(void)
> >
> > I'm not 100% sure of the logic, so IMO maybe the comment should say a
> > bit more about how this works:
> >
> > Specifically, let's say there was some timeout and the LA needed to
> > write the spool file, then let's say the PA timed out and found itself
> > inside this function. Now, let's say the LA is still busy writing the
> > file -- so what happens next?
> >
> > Does this function simply return, then the main PA loop waits again,
> > then the times out again, then PA finds itself back inside this
> > function again... and that keeps happening over and over until
> > eventually the spool file is found FS_READY? Some explanatory comments
> > might help.
> >
>
> No, PA will simply wait for LA to finish. See the code handling for
> FS_BUSY state. We might want to slightly improve part of the current
> comment to: "If the leader apply worker is busy serializing the
> partial changes then acquire the stream lock now and wait for the
> leader worker to finish serializing the changes".
>
Sure, "PA will simply wait for LA to finish".
Except I think it's not quite that simple because IIUC when LA *does*
finish, the PA (this function) will continue and just drop out the
bottom -- it cannot apply those spooled messages yet until it cycles
all the way back around the main loop and times out again and gets
back into pa_spooled_messages function again to get to the FS_READY
block of code where it can finally call the
'apply_spooled_messages'...
If my understanding is correct, then It's that extra looping that I
thought maybe warrants some mention in a comment here.
------
Kind Regards,
Peter Smith.
Fujitsu Australia
