Re: synchronized_standby_slots behavior inconsistent with quorum-based synchronous replication

On Mon, 15 Jun 2026 at 16:09, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> 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 <shlok(dot)kyal(dot)oss(at)gmail(dot)com> 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