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: Tue, 16 Jun 2026 12:09:47 +0530
Message-ID: <CANhcyEXPYAsLRJOwY6dqtLn82yhyPGv8kCtkc_HcwkbADr_j-Q@mail.gmail.com> (raw)
In-Reply-To: <CAE9k0PkxvD_Rbz1qm3ZHOX_n-JXaOiPV2wusvb6KVwQqifpt6w@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>
<CANhcyEWcziK1eR=A4_TW_PbeBAgfBacck2R=sYOqAs5=5g+2=Q@mail.gmail.com>
<CAE9k0PkxvD_Rbz1qm3ZHOX_n-JXaOiPV2wusvb6KVwQqifpt6w@mail.gmail.com>
On Mon, 15 Jun 2026 at 16:09, Ashutosh Sharma <[email protected]> wrote:
>
> Hi Shlok,
>
> Thanks for reviewing the patch and sharing your feedback - all your
> comments are well noted. Please find my responses below.
>
> On Fri, Jun 12, 2026 at 6:33 PM Shlok Kyal <[email protected]> wrote:
> >
> > 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"
> >
>
> Agreed, it indeed needs correction, it has been addressed in the attached patch.
>
> > 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.
> > ```
>
> Same as comment 1, this has been corrected in the attached patch as well
>
> >
> > 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?
> >
>
> Agreed, update was due. This has been taken care of in the attached patch.
>
> > 4. Similarly for syncrep_scanner.l, we have comment:
> > * syncrep_scanner.l
> > * a lexical scanner for synchronous_standby_names
> >
> > Should we update it?
>
> Agreed and updated in the attached patch.
>
> >
> > 5. enum "SyncStandbySlotsState" and stuct "SyncStandbySlotsStateInfo" should be
> > mentioned in typedefs.list, so that pgindent works properly.
> >
>
> These have been added to the typdefs.list
>
> PFA attached patches with all these changes.
>
Hi Ashutosh,
I checked the CFbot and saw the test was failing [1].
It is failing for the test:
not ok 4 - guc_privs 171 ms
diff -U3 /__w/postgresql/postgresql/src/test/modules/unsafe_tests/expected/guc_privs.out
/__w/postgresql/postgresql/build/testrun/unsafe_tests/regress/results/guc_privs.out
--- /__w/postgresql/postgresql/src/test/modules/unsafe_tests/expected/guc_privs.out
2026-06-15 11:08:26.979460240 +0000
+++ /__w/postgresql/postgresql/build/testrun/unsafe_tests/regress/results/guc_privs.out
2026-06-15 11:20:01.829423596 +0000
@@ -590,8 +590,7 @@
-- Cannot set synchronized_standby_slots to an invalid slot name
ALTER SYSTEM SET synchronized_standby_slots='invalid*';
ERROR: invalid value for parameter "synchronized_standby_slots": "invalid*"
-DETAIL: replication slot name "invalid*" contains invalid character
-HINT: Replication slot names may only contain lower case letters,
numbers, and the underscore character.
+DETAIL: syntax error at or near "*"
-- Can set synchronized_standby_slots to a non-existent slot name
ALTER SYSTEM SET synchronized_standby_slots='missing';
-- Reset the GUC
In HEAD, validate_sync_standby_slots() called SplitIdentifierString()
and then ReplicationSlotValidateNameInternal() for each element, which
produced the slot-name-specific error for inputs such as 'invalid*'.
With patch synchronized_standby_slots validation use the syncrep
parser and a syntax error is reported instead.
I think we should update the corresponding expected file.
[1]: https://github.com/postgresql-cfbot/postgresql/actions/runs/27541996426/job/81406065089
Thanks,
Shlok Kyal
view thread (28+ 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: <CANhcyEXPYAsLRJOwY6dqtLn82yhyPGv8kCtkc_HcwkbADr_j-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