On Wed, Mar 18, 2026 at 10:57 AM Ashutosh Sharma <[email protected]> wrote:
>
> On Wed, Mar 18, 2026 at 10:38 AM shveta malik <[email protected]> wrote:
> >
> > On Wed, Mar 18, 2026 at 10:15 AM Ashutosh Sharma <[email protected]>
> > wrote:
> > > > 8)
> > > > I would like to understand how synchronous_standby_names deal with
> > > > such a case: 'first as server name or FIRST as priority syntax'. I
> > > > could not find any such function/logic there.
> > > >
> >
> > Let me know if you understand this part, else I will debug it once.
> >
>
> This is not needed for synchronous_standby_names because specifying a
> list of values without the FIRST/ANY keyword is implicitly treated as
> FIRST 1, which the syncrep parser already handles. However, this
> behavior differs for synchronized_standby_slots, where a list of
> values without FIRST/ANY is treated as ALL mode. Since we reuse the
> syncrep parser here, we need to distinguish whether the FIRST keyword
> was explicitly provided by the user, which is the purpose of the
> IsExplicitFirstSyncStandbySlotsSyntax function.
>
Ah, got it. Thanks for the details, no idea how I missed that.
Few comments:
1)
+
+# Physical replication slots for the two standbys
+$primary->safe_psql('postgres',
+ "SELECT pg_create_physical_replication_slot('sb1_slot');");
+$primary->safe_psql('postgres',
+ "SELECT pg_create_physical_replication_slot('sb2_slot');");
+$primary->safe_psql('postgres',
+ "SELECT pg_create_physical_replication_slot('first_slot');");
Can we please write a comment atop 'first_slot' creation for its purpose.
The comment '+# Physical replication slots for the two standbys'
confuses as we are creating 3 slots following that comment.
2)
+# PART A: Plain list (ALL mode) blocks when any slot is unavailable
+# PART C: Verify plain list (ALL mode) requires ALL slots
I think we can merge the 2 tests. Is there a specific reason we have
it in 2 different sections (am I missing something?).
Part A can first test blocking on inactive standby (which it is doing
already) and then restart standby and can check that logical decoding
now proceeds. Part C can then be removed.
3)
Same with these 2:
+# PART B: ANY mode (quorum) — logical decoding proceeds with N-of-M slots
+# PART D: ANY mode — verify it works with either standby
Part B tests that it works when standby1 is down.
Part D tests that it works when standby2 is down and also works when
both are up.
I think we don't need these duplicate test cases (unless I am missing
something?), we can merge Part D to Part B itself i.e. test with
standby1 down and up in a single test.
4)
These 2 tests are basically testing the same thing. Do we need both?
+# FIRST 1 should skip sb1_slot (unavailable) and use sb2_slot.
+# Test that FIRST 1 is different from plain list — FIRST 1 succeeds
with one down.
5)
I see that we only need 'first_slot' for the last test and do not have
even a standby associated with it. So we can even move it's creation
to that test itself instead of creating it in the beginning and adding
comments to explain 'why'.
thanks
Shveta
