Re: Copy function for logical replication slots

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Copy function for logical replication slots
Date: 2018-11-26 00:29:54
Message-ID: CAD21AoANwkcrB2GFpkrQ8MafP7=7rUTW==9hOTHXLEdkmEM1vw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
v7-0001-Copy-function-for-logical-and-physical-replicatio.patch application/octet-stream 40.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-11-26 00:33:51 Re: pgsql: Add PGXS options to control TAP and isolation tests
Previous Message Michael Paquier 2018-11-26 00:28:49 Re: pgsql: Integrate recovery.conf into postgresql.conf