public inbox for [email protected]
help / color / mirror / Atom feedFrom: Shlok Kyal <[email protected]>
To: Ashutosh Sharma <[email protected]>
Cc: shveta malik <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: Ajin Cherian <[email protected]>
Cc: SATYANARAYANA NARLAPURAM <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: synchronized_standby_slots behavior inconsistent with quorum-based synchronous replication
Date: Fri, 12 Jun 2026 18:33:32 +0530
Message-ID: <CANhcyEWcziK1eR=A4_TW_PbeBAgfBacck2R=sYOqAs5=5g+2=Q@mail.gmail.com> (raw)
In-Reply-To: <CAE9k0P=uMVGV5TXTVdjKzu+0gFLMKHNY2usnAzK0L2cjbmPHBg@mail.gmail.com>
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>
On Fri, 12 Jun 2026 at 15:40, Ashutosh Sharma <[email protected]> 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 <[email protected]> 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
view thread (25+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: synchronized_standby_slots behavior inconsistent with quorum-based synchronous replication
In-Reply-To: <CANhcyEWcziK1eR=A4_TW_PbeBAgfBacck2R=sYOqAs5=5g+2=Q@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox