On Wed, Apr 3, 2024 at 2:58 PM shveta malik <shveta.ma...@gmail.com> wrote: > > On Wed, Apr 3, 2024 at 11:17 AM Bharath Rupireddy > <bharath.rupireddyforpostg...@gmail.com> wrote: > > > > On Wed, Apr 3, 2024 at 8:38 AM shveta malik <shveta.ma...@gmail.com> wrote: > > > > > > > > Or a simple solution is that the slotsync worker updates > > > > > inactive_since as it does for non-synced slots, and disables > > > > > timeout-based slot invalidation for synced slots. > > > > > > I like this idea better, it takes care of such a case too when the > > > user is relying on sync-function rather than worker and does not want > > > to get the slots invalidated in between 2 sync function calls. > > > > Please find the attached v31 patches implementing the above idea: > > > > Thanks for the patches, please find few comments: > > v31-001: > > 1) > system-views.sgml: > value will get updated after every synchronization from the > corresponding remote slot on the primary. > > --This is confusing. It will be good to rephrase it. > > 2) > update_synced_slots_inactive_since() > > --May be, we should mention in the header that this function is called > only during promotion. > > 3) 040_standby_failover_slots_sync.pl: > We capture inactive_since_on_primary when we do this for the first time at > #175 > ALTER SUBSCRIPTION regress_mysub1 DISABLE" > > But we again recreate the sub and disable it at line #280. > Do you think we shall get inactive_since_on_primary again here, to be > compared with inactive_since_on_new_primary later? >
I think so. Few additional comments on tests: 1. +is( $standby1->safe_psql( + 'postgres', + "SELECT '$inactive_since_on_primary'::timestamptz < '$inactive_since_on_standby'::timestamptz AND + '$inactive_since_on_standby'::timestamptz < '$slot_sync_time'::timestamptz;" Shall we do <= check as we are doing in the main function get_slot_inactive_since_value as the time duration is less so it can be the same as well? Similarly, please check other tests. 2. +=item $node->get_slot_inactive_since_value(self, slot_name, reference_time) + +Get inactive_since column value for a given replication slot validating it +against optional reference time. + +=cut + +sub get_slot_inactive_since_value I see that all callers validate against reference time. It is better to name it validate_slot_inactive_since rather than using get_* as the main purpose is to validate the passed value. -- With Regards, Amit Kapila.