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

Hi,

On Wed, Feb 28, 2024 at 02:23:27AM +0000, Zhijie Hou (Fujitsu) wrote:
> Attach the V100 patch set which addressed above comments.

Thanks!

A few random comments:

1 ===

+       if (!ok)
+       {
+               GUC_check_errdetail("List syntax is invalid.");
+       }

What about to get rid of the brackets here?

2 ===

+
+       /*
+        * If the replication slots' data have been initialized, verify if the
+        * specified slots exist and are logical slots.
+        */

remove the empty line above the comment?

3 ===

+check_standby_slot_names(char **newval, void **extra, GucSource source)
+{
+       if ((*newval)[0] == '\0')
+               return true;

I think "**newval == '\0'" is easier to read but that's a matter of taste and
check_synchronous_standby_names() is already using the same so it's a nit.

4 ===

Regarding the test, what about adding one to test the "new" behavior discussed
up-thread? (logical replication will wait if slot mentioned in standby_slot_names
is dropped and/or does not exist when the engine starts?)

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com