On Wed, Apr 2, 2025 at 8:30 PM Bertrand Drouvot
<bertranddrouvot...@gmail.com> wrote:
>
> On Wed, Apr 02, 2025 at 12:13:52PM +0000, Hayato Kuroda (Fujitsu) wrote:
> > Dear Amit, Bertrand,
> >
> > > The other idea to simplify the changes for backbranches:
> > > sub reactive_slots_change_hfs_and_wait_for_xmins
> > > {
> > > ...
> > > + my ($slot_prefix, $hsf, $invalidated, $needs_active_slot) = @_;
> > >
> > > # create the logical slots
> > > - create_logical_slots($node_standby, $slot_prefix);
> > > + create_logical_slots($node_standby, $slot_prefix, $needs_active_slot);
> > >
> > > ...
> > > + if ($needs_active_slot)
> > > + {
> > > + $handle =
> > > + make_slot_active($node_standby, $slot_prefix, 1, \$stdout,
> > > \$stderr);
> > > + }
> > >
> > > What if this function doesn't take input parameter needs_active_slot
> > > and rather removes the call to make_slot_active? We will call
> > > make_slot_active only at the required places. This should make the
> > > changes much less because after that, we don't need to make changes
> > > related to drop and create. Sure, in some cases, we will test two
> > > inactive slots instead of one, but I guess that would be the price to
> > > keep the tests simple and more like HEAD.
> >
> > Actually, I could not decide which one is better, so let me share both
> > drafts.
>
> Thanks!
>
> > V5-PG17-1 uses the previous approach, and v5-PG17-2 uses new proposed one.
> > Bertrand, which one do you like?
>
> I do prefer v5-PG17-2 as it is "closer" to HEAD. That said, I think that we
> should
> keep the slots active and only avoid doing the checks for them (they are
> invalidated
> that's fine, they are not that's fine too).
>
I don't mind doing that, but there is no benefit in making slots
active unless we can validate them. And we will end up adding some
more checks, as in function check_slots_conflict_reason without any
advantage. I feel Kuroda-San's second patch is simple, and we have
fewer chances to make mistakes and easy to maintain in the future as
well.
--
With Regards,
Amit Kapila.
