Re: Copy function for logical replication slots

On Sun, Nov 25, 2018 at 12:27 AM Petr Jelinek
<petr(dot)jelinek(at)2ndquadrant(dot)com> wrote:
>
> Hi,
>
> On 31/08/2018 07:03, Masahiko Sawada wrote:
> >
> > Attached a new version patch incorporated the all comments I got.
> >
>
> This looks pretty reasonable.

Thank you for looking at this patch.

>
> I am personally not big fan of the C wrappers for overloaded functions,
> but that's what we need to do for opr_sanity to pass so I guess we'll
> have to use them.
>
> The more serious thing is:
>
> > + if (MyReplicationSlot)
> > + ReplicationSlotRelease();
> > +
> > + /* Release the saved slot if exist while preventing double releasing */
> > + if (savedslot && savedslot != MyReplicationSlot)
>
> This won't work as intended as the ReplicationSlotRelease() will set
> MyReplicationSlot to NULL, you might need to set aside MyReplicationSlot
> to yet another temp variable inside this function prior to releasing it.
>

You're right. I've fixed it by checking if we need to release the
saved slot before releasing the origin slot. Is that right?
Attached an updated patch.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center