On Fri, Mar 20, 2026 at 12:14 PM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Thu, Mar 19, 2026 at 12:08 PM Hou, Zhijie/侯 志杰
> <houzj.fnst@fujitsu.com> wrote:
> >
> > On Wednesday, March 18, 2026 6:38 PM Ashutosh Sharma <ashu.coek88@gmail.com> 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.
