On Thu, Jul 18, 2024 at 9:25 AM shveta malik <shveta.ma...@gmail.com> wrote: > > On Tue, Jul 9, 2024 at 12:42 AM John H <johnh...@gmail.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