Re: Synchronizing slots from primary to standby - Mailing list pgsql-hackers

From Bertrand Drouvot
Subject Re: Synchronizing slots from primary to standby
Date
Msg-id ZeWu6JY0n2oficvf@ip-10-97-1-34.eu-west-3.compute.internal
Whole thread Raw
In response to RE: Synchronizing slots from primary to standby  ("Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>)
Responses Re: Synchronizing slots from primary to standby
RE: Synchronizing slots from primary to standby
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: Andy Fan
Date:
Subject: Re: a wrong index choose when statistics is out of date
Next
From: Bharath Rupireddy
Date:
Subject: Re: PostgreSQL Contributors Updates