Hi, 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! A few more random comments: 1 === + The function may be blocked if the specified slot is a failover enabled s/blocked/waiting/ ? 2 === + * specified slot when waiting for them to catch up. See + * StandbySlotsHaveCaughtup for details. s/StandbySlotsHaveCaughtup/StandbySlotsHaveCaughtup()/ ? 3 === + /* Now verify if the specified slots really exist and have correct type */ remove "really"? 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? 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? 6 === +static bool +NeedToWaitForWal(XLogRecPtr target_lsn, XLogRecPtr flushed_lsn, + uint32 *wait_event) Same questions as for NeedToWaitForStandby(). Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
