On Fri, Mar 20, 2026 at 12:14 PM shveta malik <[email protected]> wrote: > > On Thu, Mar 19, 2026 at 12:08 PM Hou, Zhijie/侯 志杰 > <[email protected]> wrote: > > > > On Wednesday, March 18, 2026 6:38 PM Ashutosh Sharma > > <[email protected]> wrote: > > > PFA patch that addresses above comments. > > > > Thanks for the patch! Overall, I think this is a valuable feature as I've > > heard > > requests from many customers for a way to avoid blocking logical replication > > when only some instances are down when using synchronized_standby_slots. > > > > I didn't find any bugs in the patch, but I have a few comments on the code > > and > > tests: > > > > 1. > > > > > /* > > > * Return true if value starts with explicit FIRST syntax: > > > * > > > * FIRST N (...) > > > * > > > * This is used to distinguish explicit FIRST from simple list syntax > > > whose > > > * first slot name may start with "first". > > > */ > > > static bool > > > IsPrioritySyncStandbySlotsSyntax(const char *value) > > > > I think adding a new function to manually parse the list isn't the most > > elegant > > approach. Instead, it would be cleaner to have a new flag that distinguishes > > when a plain name list is specified, and use that to mark the case > > appropriately like: > > > > /* syncrep_method of SyncRepConfigData */ > > #define SYNC_REP_PRIORITY 0 > > #define SYNC_REP_QUORUM 1 > > +#define SYNC_REP_IMPLICIT 2 > > > > standby_config: > > - standby_list { $$ = > > create_syncrep_config("1", $1, SYNC_REP_PRIORITY); } > > + standby_list { $$ = > > create_syncrep_config("1", $1, SYNC_REP_IMPLICIT); } > > > > I like the idea. The only thing we have to see is that the changes in > synchronous_standby_names for this look clean (converting IMPLICIT to > PRIORITY and overwriting num_sync to 1). Also, do you think > SYNC_REP_ALL is more meaningful than SYNC_REP_IMPLICIT? >
In my view, modifying the shared parser code (the syncrep parser in this case) may not be the best approach, especially when it can be avoided. The current design keeps changes localized to synchronized_standby_slots parsing within the slot handling logic, and preserves existing synchronous replication semantics. The localized approach is also comparatively safer (risk free) and easier to maintain. Additionally, the function that inspects the syncrep_parse output is fairly straightforward, it simply checks for the presence of the FIRST N syntax. -- With Regards, Ashutosh Sharma.
