MIME-Version: 1.0
References: 
 <CAJpy0uBT8JbEGE0xvm-Wxh1g_VVgC=whKqChZo-uB+VOa_YCTw@mail.gmail.com>
 <CAE9k0Pkk6q72X3Rc3MUo7PxU46UcCzLfMhM02PGDUmAue9cDGg@mail.gmail.com>
 <TY4PR01MB17718104B91F2945BE727467694102@TY4PR01MB17718.jpnprd01.prod.outlook.com>
 <CAE9k0P=dgCEaKE6+vSCQp8TgrYOi_RqQkDTScdWzFSECsPQn9w@mail.gmail.com>
 <TY4PR01MB17718F364B2CB8108B34FA5FC94112@TY4PR01MB17718.jpnprd01.prod.outlook.com>
 <CAJpy0uCnasi4MSQB=nrjPSv4U_0rb2Z-cg_wazUGQ-P_VnRZeA@mail.gmail.com>
 <CAA4eK1JLwL_aauy+nrrobC3Sftv369N3N6DaidBpNJdiOwXG+g@mail.gmail.com>
 <CAJpy0uBR-8-d__LgfqqUbFvLBvzL6bY_kjUmBWAVsLLu1QZEPA@mail.gmail.com>
 <CAE9k0P=uMVGV5TXTVdjKzu+0gFLMKHNY2usnAzK0L2cjbmPHBg@mail.gmail.com>
In-Reply-To: 
 <CAE9k0P=uMVGV5TXTVdjKzu+0gFLMKHNY2usnAzK0L2cjbmPHBg@mail.gmail.com>
From: Shlok Kyal <shlok.kyal.oss@gmail.com>
Date: Fri, 12 Jun 2026 18:33:32 +0530
Message-ID: 
 <CANhcyEWcziK1eR=A4_TW_PbeBAgfBacck2R=sYOqAs5=5g+2=Q@mail.gmail.com>
Subject: Re: synchronized_standby_slots behavior inconsistent with
 quorum-based synchronous replication
To: Ashutosh Sharma <ashu.coek88@gmail.com>
Cc: shveta malik <shveta.malik@gmail.com>,
 Amit Kapila <amit.kapila16@gmail.com>,
	"Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>,
 Ajin Cherian <itsajin@gmail.com>,
	SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CANhcyEWcziK1eR%3DA4_TW_PbeBAgfBacck2R%3DsYOqAs5%3D5g%2B2%3DQ%40mail.gmail.com>
Precedence: bulk

On Fri, 12 Jun 2026 at 15:40, Ashutosh Sharma <ashu.coek88@gmail.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=E2=80=AFPM shveta malik <shveta.malik@gmail=
.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_sta=
tes
> >
> > 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 =3D (required =3D=3D synchronized_standby_slots_config->=
nslotnames);
> > + wait_for_all =3D
> > + (synchronized_standby_slots_config->syncrep_method =3D=3D SYNC_REP_DE=
FAULT);
> >
> > 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=3D"guc-synchronized-standby-slots"><varname>synchronized_stan=
dby_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=3D"guc-synchronized-standby-slots"><varname>synchronized_stan=
dby_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 com=
ment?

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" shoul=
d be
mentioned in typedefs.list, so that pgindent works properly.

Thanks,
Shlok Kyal