Re: Allow logical failover slots to wait on synchronous replication

On Thu, Jul 18, 2024 at 9:25 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Tue, Jul 9, 2024 at 12:42 AM John H <johnhyvr(at)gmail(dot)com> wrote:
> >
> >
> > > Can we make it a default
> > > behavior that logical slots marked with a failover option will wait
> > > for 'synchronous_standby_names' as per your patch's idea unless
> > > 'standby_slot_names' is specified? I don't know if there is any value
> > > in setting the 'failover' option for a slot without specifying
> > > 'standby_slot_names', so was wondering if we can additionally tie it
> > > to 'synchronous_standby_names'. Any better ideas?
> > >
> >
> > No, I think that works pretty cleanly actually. Reserving some special
> > keyword isn't great
> > which is the only other thing I can think of. I've updated the patch
> > and tests to reflect that.
> >
> > Attached the patch that addresses these changes.
>
> Thank You for the patch. I like the overall idea, it is a useful one
> and will make user experience better. Please find few comments.
>
> 1)
> I agree with the idea that instead of introducing new GUC, we can make
> failover logical slots wait on 'synchronous_standby_names', but this
> will leave user no option to unlink failover-enabled logical
> subscribers from the wait on synchronous standbys. We provide user a
> way to switch off automatic slot-sync by disabling
> 'sync_replication_slots' and we also provide user a way to manually
> sync the slots using function 'pg_sync_replication_slots()' and
> nowhere we make it mandatory to set 'synchronized_standby_slots'; in
> fact in docs, it is a recommended setting and not a mandatory one.
> User can always create failover slots, switch off automatic slot sync
> and disable wait on standbys by not specifying
> 'synchronized_standby_slots' and do the slot-sync and consistency
> checks manually when needed. I feel, for worst case scenarios, we
> should provide user an option to delink failover-enabled logical
> subscriptions from 'synchronous_standby_names'.
>
> We can have 'synchronized_standby_slots' (standby_slot_names) to
> accept one such option as in 'SAME_AS_SYNCREP_STANDBYS' or say
> 'DEFAULT'. So when 'synchronous_standby_names' is comma separated
> list, we pick those slots; if it is empty, then no wait on standbys,
> and if its value is 'DEFAULT' as configured by the user, then go with
> 'synchronous_standby_names'. Thoughts?

One correction here
('synchronous_standby_names-->synchronized_standby_slots). Corrected
version:

So when 'synchronized_standby_slots' is comma separated list, we pick
those slots; if it is empty, then no wait on standbys, and if its
value is 'DEFAULT' as configured by user, then go with
'synchronous_standby_names'. Thoughts?

>
> 2)
> When 'synchronized_standby_slots' is configured but standby named in
> it is down blocking logical replication, then we get a WARNING in
> subscriber's log file:
>
> WARNING: replication slot "standby_2" specified in parameter
> synchronized_standby_slots does not have active_pid.
> DETAIL: Logical replication is waiting on the standby associated with
> "standby_2".
> HINT: Consider starting standby associated with "standby_2" or amend
> parameter synchronized_standby_slots.
>
> But OTOH, when 'synchronous_standby_names' is configured instead of
> 'synchronized_standby_slots' and any of the standbys listed is down
> blocking logical replication, we do not get any sort of warning. It is
> inconsistent behavior. Also user might be left clueless on why
> subscribers are not getting changes.
>
> thanks
> Shveta