Re: Logical Replication WIP

From: Petr Jelinek <petr(at)2ndquadrant(dot)com>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Cc: Craig Ringer <craig(at)2ndquadrant(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Steve Singer <steve(at)ssinger(dot)info>
Subject: Re: Logical Replication WIP
Date: 2016-11-03 13:31:20
Message-ID: 9b189ab8-e639-9e73-1ede-fdcf361acf55@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 02/11/16 17:22, Peter Eisentraut wrote:
> On 10/24/16 9:22 AM, Petr Jelinek wrote:
>> I added one more prerequisite patch (the first one) which adds ephemeral
>> slots (or well implements UI on top of the code that was mostly already
>> there). The ephemeral slots are different in that they go away either on
>> error or when session is closed. This means the initial data sync does
>> not have to worry about cleaning up the slots after itself. I think this
>> will be useful in other places as well (for example basebackup). I
>> originally wanted to call them temporary slots in the UI but since the
>> behavior is bit different from temp tables I decided to go with what the
>> underlying code calls them in UI as well.
>
> I think it makes sense to expose this.
>
> Some of the comments need some polishing.
>
> Eventually, we might want to convert the option list in
> CREATE_REPLICATION_SLOT into a list instead of adding more and more
> keywords (see also VACUUM), but not necessarily now.
>
> I find the way Acquire and Release are handled now quite confusing.
> Because Release of an ephemeral slot means to delete it, you have
> changed most code to never release them until the end of the session.
> So there is a lot of ugly and confusing code that needs to know this
> difference. I think we need to use some different verbs for different
> purposes here. Acquire and release should keep their meaning of "I'm
> using this", and the calls in proc.c and postgres.c should be something
> like ReplicationSlotCleanup().
>

Release does not really change behavior, it has always dropped ephemeral
slot.

So if I understand correctly what you are proposing is to change
behavior of Release to not remove ephemeral slot, add function that
removes the ephemeral slots of current session and add tracking of
ephemeral slots created in current session? That seems like quite more
complicated than what the patch does with little gain.

What about just releasing the ephemeral slot if the different one is
being acquired instead of the current error?

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Craig Ringer 2016-11-03 13:44:29 Re: Stopping logical replication protocol
Previous Message Brad DeJong 2016-11-03 13:31:07 Re: Remove the comment on the countereffectiveness of large shared_buffers on Windows