Re: Fix 035_standby_logical_decoding.pl race conditions

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Fix 035_standby_logical_decoding.pl race conditions
Date: 2025-04-03 04:16:02
Message-ID: CAA4eK1Krrie2X40qdj9Qu8D109DjjqS4nH-woJCheBtgdNyR_w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Apr 2, 2025 at 8:30 PM Bertrand Drouvot
<bertranddrouvot(dot)pg(at)gmail(dot)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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amul Sul 2025-04-03 05:07:16 Re: NOT ENFORCED constraint feature
Previous Message Ashutosh Bapat 2025-04-03 04:02:33 Re: Test to dump and restore objects left behind by regression