Re: Synchronizing slots from primary to standby

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