On Tue, Dec 6, 2022 at 2:51 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Tue, Dec 6, 2022 at 5:27 AM Peter Smith <smithpb2...@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
