On Mon, Mar 4, 2024 at 4:52 PM Bertrand Drouvot <bertranddrouvot...@gmail.com> wrote: > > On Sun, Mar 03, 2024 at 07:56:32AM +0000, Zhijie Hou (Fujitsu) wrote: > > Here is the V104 patch which addressed above and Peter's comments. > > Thanks! > > > 4 === > > + /* > + * Don't need to wait for the standbys to catch up if there is no > value in > + * standby_slot_names. > + */ > + if (standby_slot_names_list == NIL) > + return true; > + > + /* > + * Don't need to wait for the standbys to catch up if we are on a > standby > + * server, since we do not support syncing slots to cascading > standbys. > + */ > + if (RecoveryInProgress()) > + return true; > + > + /* > + * Don't need to wait for the standbys to catch up if they are already > + * beyond the specified WAL location. > + */ > + if (!XLogRecPtrIsInvalid(standby_slot_oldest_flush_lsn) && > + standby_slot_oldest_flush_lsn >= wait_for_lsn) > + return true; > > What about using OR conditions instead? >
I think we can use but it seems code is easier to follow this way but this is just a matter of personal choice. > 5 === > > +static bool > +NeedToWaitForStandby(XLogRecPtr target_lsn, XLogRecPtr flushed_lsn, > + uint32 *wait_event) > > Not a big deal but does it need to return a bool? (I mean it all depends of > the *wait_event value). Is it for better code readability in the caller? > Yes, I think so. Adding checks based on wait_events sounds a bit awkward. -- With Regards, Amit Kapila.
