Re: synchronized_standby_slots behavior inconsistent with quorum-based synchronous replication

On Fri, 12 Jun 2026 at 15:40, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
>
> Hi Shveta,
>
> Thanks again for your review comments and suggestions. Please see my
> comments inline below:
>
> On Wed, Jun 10, 2026 at 12:16 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > Please find a few comments on June8 version fo patches:
> >
> > 1)
> > patch001:
> >
> > SYNC_REP_DEFAULT: do we need to give one-line comment for this
> > somewhere as unlike PRIORITY and QUORUM, it is not self-explanatory.
> >
>
> Yes, it does makes sense to include a one-line comment. I've added it
> in the attached patch.
>
> >
> > patch002:
> > 2)
> > It is better to avoid mentioning it as 'synchronized standby slots'.
> > We can make it as 'synchronized_standby_slots' in below comments:
> >
> > + /* Parse the synchronized standby slots configuration */
> >
> > + * Report problem states for synchronized standby slots that prevented the
> >
>
> Good point. I've made the suggested change in the attached patch
>
> > 3)
> > For these error-messages too, we need to mention GUC name to give
> > better clarity.
> >
> > + GUC_check_errmsg("number of synchronized standby slots (%d) must not
> > exceed the number of unique listed slots (%d)",
> > + syncrep_parse_result->num_sync,
> > + syncrep_parse_result->nmembers);
> >
> >
> > How about:
> > GUC_check_errcode(ERRCODE_INVALID_PARAMETER_VALUE);
> > GUC_check_errmsg("invalid value for parameter \"%s\: synchronization
> > requirement (%d) exceeds the number of unique listed slots (%d)",
> > "synchronized_standby_slots",
> > syncrep_parse_result->num_sync,
> > syncrep_parse_result->nmembers);
> >
>
> Updated as suggested in the attached patch.
>
> > 3)
> > + GUC_check_errmsg("number of synchronized standby slots (%d) must be
> > greater than zero",
> > + syncrep_parse_result->num_sync)
> >
> > How about:
> > GUC_check_errcode(ERRCODE_INVALID_PARAMETER_VALUE);
> > GUC_check_errmsg("invalid value for parameter \"%s\: required number
> > of synchronized standby slots (%d) must be greater than zero",
> > "synchronized_standby_slots",
> > syncrep_parse_result->num_sync);
> >
> >
> >
>
> Updated as suggested in the attached patch.
>
> > 4)
> > +ReportUnavailableSyncStandbySlots(SyncStandbySlotsStateInfo * slot_states
> >
> > We can get rid of space before slot_states. I think pgindent might
> > have put it in my patch. Sorry for the trouble.
> >
> > 5)
> > 003:
> > - wait_for_all = (required == synchronized_standby_slots_config->nslotnames);
> > + wait_for_all =
> > + (synchronized_standby_slots_config->syncrep_method == SYNC_REP_DEFAULT);
> >
> > This change can be moved to 002, right?
>
> Done.
>
> PFA patch containing all the above changes.
>
Hi Ashutosh,

I have reviewed the patches. Here are some comments:

1. Should we update the doc for function "pg_logical_slot_get_changes". It says:
```
If the specified slot is a logical failover slot then the function will
not return until all physical slots specified in
<link linkend="guc-synchronized-standby-slots"><varname>synchronized_standby_slots</varname></link>
have confirmed WAL receipt.
```
This line "return until all physical slots specified in" seems wrong
after introduction of "FIRST/ANY"

2. Similarly, should we update the doc for function
"pg_replication_slot_advance"? It also says:
```
If the specified slot is a
logical failover slot then the function will not return until all
physical slots specified in
<link linkend="guc-synchronized-standby-slots"><varname>synchronized_standby_slots</varname></link>
have confirmed WAL receipt.
```

3. Comment in syncrep_gram.y states:
* syncrep_gram.y - Parser for synchronous_standby_names

Since synchronosed_standby_slots is reusing this, should we update this comment?

4. Similarly for syncrep_scanner.l, we have comment:
* syncrep_scanner.l
* a lexical scanner for synchronous_standby_names

Should we update it?

5. enum "SyncStandbySlotsState" and stuct "SyncStandbySlotsStateInfo" should be
mentioned in typedefs.list, so that pgindent works properly.

Thanks,
Shlok Kyal