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 | Resend email |
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 |
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 |