Re: Introduce XID age and inactive timeout based replication slot invalidation

On Tue, Nov 19, 2024 at 12:47 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Thu, Nov 14, 2024 at 5:29 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> >
> > 12.
> >   /*
> > - * If the slot can be acquired, do so and mark it invalidated
> > - * immediately.  Otherwise we'll signal the owning process, below, and
> > - * retry.
> > + * If the slot can be acquired, do so and mark it as invalidated. If
> > + * the slot is already ours, mark it as invalidated. Otherwise, we'll
> > + * signal the owning process below and retry.
> >   */
> > - if (active_pid == 0)
> > + if (active_pid == 0 ||
> > + (MyReplicationSlot == s &&
> > + active_pid == MyProcPid))
> >
> > I wasn't sure how this change belongs to this patch, because the logic
> > of the previous review comment said for the case of invalidation due
> > to inactivity that active_id must be 0. e.g. Assert(s->active_pid ==
> > 0);
> >
>
> I don't fully understand the purpose of this change yet. I'll look
> into it further and get back.
>

This change applies to all types of invalidation, not just
inactive_timeout case, so moved the change to patch-001. It’s a
general optimization for the case when the current process is the
active PID for the slot.
Also, the Assert(s->active_pid == 0); has been removed (in v50) as it
was unnecessary.

--
Thanks,
Nisha