Postgres Pro
Downloads
Home   >   mailing lists

Thread: pgsql: Add support for managing physical replication slots to pg_receiv

pgsql: Add support for managing physical replication slots to pg_receiv

From
Andres Freund
Date:
Add support for managing physical replication slots to pg_receivexlog.

pg_receivexlog already has the capability to use a replication slot to
reserve WAL on the upstream node. But the used slot currently has to
be created via SQL.

To allow using slots directly, without involving SQL, add
--create-slot and --drop-slot actions, analogous to the logical slot
manipulation support in pg_recvlogical.

Author: Michael Paquier
Discussion: CABUevEx+zrOHZOQg+dPapNPFRJdsk59b=TSVf30Z71GnFXhQaw@mail.gmail.com

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/d9f38c7a555dd5a6b81100c6d1e4aa68342d8771

Modified Files
--------------
doc/src/sgml/ref/pg_receivexlog.sgml   |   31 ++++++-
src/bin/pg_basebackup/pg_receivexlog.c |  155 ++++++++++++++++++++++++++++----
2 files changed, 170 insertions(+), 16 deletions(-)

Re: pgsql: Add support for managing physical replication slots to pg_receiv

From
Fujii Masao
Date:
On Mon, Oct 6, 2014 at 7:57 PM, Andres Freund <andres@anarazel.de> wrote:
> Add support for managing physical replication slots to pg_receivexlog.
>
> pg_receivexlog already has the capability to use a replication slot to
> reserve WAL on the upstream node. But the used slot currently has to
> be created via SQL.
>
> To allow using slots directly, without involving SQL, add
> --create-slot and --drop-slot actions, analogous to the logical slot
> manipulation support in pg_recvlogical.
>
> Author: Michael Paquier
> Discussion: CABUevEx+zrOHZOQg+dPapNPFRJdsk59b=TSVf30Z71GnFXhQaw@mail.gmail.com
>
> Branch
> ------
> master
>
> Details
> -------
> http://git.postgresql.org/pg/commitdiff/d9f38c7a555dd5a6b81100c6d1e4aa68342d8771
>
> Modified Files
> --------------
> doc/src/sgml/ref/pg_receivexlog.sgml   |   31 ++++++-
> src/bin/pg_basebackup/pg_receivexlog.c |  155 ++++++++++++++++++++++++++++----
> 2 files changed, 170 insertions(+), 16 deletions(-)
>
>
> --
> Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-committers

This patch changed pg_receivexlog so that it creates new connection
before creating the slot, etc, but ISTM that it forgets to close the connection.
Probably something like attached patch needs to be applied.

Regards,

--
Fujii Masao
Attachment

Re: pgsql: Add support for managing physical replication slots to pg_receiv

From
Andres Freund
Date:
On 2014-10-07 14:21:59 +0900, Fujii Masao wrote:
> On Mon, Oct 6, 2014 at 7:57 PM, Andres Freund <andres@anarazel.de> wrote:
> > Add support for managing physical replication slots to pg_receivexlog.
> >
> > pg_receivexlog already has the capability to use a replication slot to
> > reserve WAL on the upstream node. But the used slot currently has to
> > be created via SQL.
> >
> > To allow using slots directly, without involving SQL, add
> > --create-slot and --drop-slot actions, analogous to the logical slot
> > manipulation support in pg_recvlogical.
> >
> > Author: Michael Paquier
> > Discussion: CABUevEx+zrOHZOQg+dPapNPFRJdsk59b=TSVf30Z71GnFXhQaw@mail.gmail.com
> >
> > Branch
> > ------
> > master
> >
> > Details
> > -------
> > http://git.postgresql.org/pg/commitdiff/d9f38c7a555dd5a6b81100c6d1e4aa68342d8771
> >
> > Modified Files
> > --------------
> > doc/src/sgml/ref/pg_receivexlog.sgml   |   31 ++++++-
> > src/bin/pg_basebackup/pg_receivexlog.c |  155 ++++++++++++++++++++++++++++----
> > 2 files changed, 170 insertions(+), 16 deletions(-)
> >
> >
> > --
> > Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
> > To make changes to your subscription:
> > http://www.postgresql.org/mailpref/pgsql-committers
>
> This patch changed pg_receivexlog so that it creates new connection
> before creating the slot, etc, but ISTM that it forgets to close the connection.
> Probably something like attached patch needs to be applied.

Hm, yes.

> *** a/src/bin/pg_basebackup/pg_receivexlog.c
> --- b/src/bin/pg_basebackup/pg_receivexlog.c
> ***************
> *** 591,596 **** main(int argc, char **argv)
> --- 591,598 ----
>               disconnect_and_exit(1);
>       }
>
> +     PQfinish(conn);
> +
>       while (true)
>       {
>           StreamLog();

But wouldn't it be better to simply pass in the connection to
StreamLog()?

Greetings,

Andres Freund

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

Re: pgsql: Add support for managing physical replication slots to pg_receiv

From
Fujii Masao
Date:
On Tue, Oct 7, 2014 at 2:29 PM, Andres Freund <andres@anarazel.de> wrote:
> On 2014-10-07 14:21:59 +0900, Fujii Masao wrote:
>> On Mon, Oct 6, 2014 at 7:57 PM, Andres Freund <andres@anarazel.de> wrote:
>> > Add support for managing physical replication slots to pg_receivexlog.
>> >
>> > pg_receivexlog already has the capability to use a replication slot to
>> > reserve WAL on the upstream node. But the used slot currently has to
>> > be created via SQL.
>> >
>> > To allow using slots directly, without involving SQL, add
>> > --create-slot and --drop-slot actions, analogous to the logical slot
>> > manipulation support in pg_recvlogical.
>> >
>> > Author: Michael Paquier
>> > Discussion: CABUevEx+zrOHZOQg+dPapNPFRJdsk59b=TSVf30Z71GnFXhQaw@mail.gmail.com
>> >
>> > Branch
>> > ------
>> > master
>> >
>> > Details
>> > -------
>> > http://git.postgresql.org/pg/commitdiff/d9f38c7a555dd5a6b81100c6d1e4aa68342d8771
>> >
>> > Modified Files
>> > --------------
>> > doc/src/sgml/ref/pg_receivexlog.sgml   |   31 ++++++-
>> > src/bin/pg_basebackup/pg_receivexlog.c |  155 ++++++++++++++++++++++++++++----
>> > 2 files changed, 170 insertions(+), 16 deletions(-)
>> >
>> >
>> > --
>> > Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
>> > To make changes to your subscription:
>> > http://www.postgresql.org/mailpref/pgsql-committers
>>
>> This patch changed pg_receivexlog so that it creates new connection
>> before creating the slot, etc, but ISTM that it forgets to close the connection.
>> Probably something like attached patch needs to be applied.
>
> Hm, yes.
>
>> *** a/src/bin/pg_basebackup/pg_receivexlog.c
>> --- b/src/bin/pg_basebackup/pg_receivexlog.c
>> ***************
>> *** 591,596 **** main(int argc, char **argv)
>> --- 591,598 ----
>>                       disconnect_and_exit(1);
>>       }
>>
>> +     PQfinish(conn);
>> +
>>       while (true)
>>       {
>>               StreamLog();
>
> But wouldn't it be better to simply pass in the connection to
> StreamLog()?

ISTM that the idea would make the code in StreamLog() somewhat complicated,
i.e., StreamLog() needs to always check whether the conn is valid or not before
trying to create new connection. We cannot remove the code to create new
connection in StreamLog() because it needs to reconnect to the server when
the connection is terminated (of course in the case where --no-loop is
not specified).

Regards,

--
Fujii Masao

Re: pgsql: Add support for managing physical replication slots to pg_receiv

From
Andres Freund
Date:
On 2014-10-07 14:51:59 +0900, Fujii Masao wrote:
> >> *** a/src/bin/pg_basebackup/pg_receivexlog.c
> >> --- b/src/bin/pg_basebackup/pg_receivexlog.c
> >> ***************
> >> *** 591,596 **** main(int argc, char **argv)
> >> --- 591,598 ----
> >>                       disconnect_and_exit(1);
> >>       }
> >>
> >> +     PQfinish(conn);
> >> +
> >>       while (true)
> >>       {
> >>               StreamLog();
> >
> > But wouldn't it be better to simply pass in the connection to
> > StreamLog()?
>
> ISTM that the idea would make the code in StreamLog() somewhat complicated,
> i.e., StreamLog() needs to always check whether the conn is valid or not before
> trying to create new connection. We cannot remove the code to create new
> connection in StreamLog() because it needs to reconnect to the server when
> the connection is terminated (of course in the case where --no-loop is
> not specified).

Not that much imo.

if (conn == NULL)
   conn = GetConnection();

if (!conn)
    /* Error message already written in GetConnection() */
    return;

...

PQfinish(conn);
conn = NULL;

Greetings,

Andres Freund

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

Re: pgsql: Add support for managing physical replication slots to pg_receiv

From
Fujii Masao
Date:
On Tue, Oct 7, 2014 at 2:55 PM, Andres Freund <andres@anarazel.de> wrote:
> On 2014-10-07 14:51:59 +0900, Fujii Masao wrote:
>> >> *** a/src/bin/pg_basebackup/pg_receivexlog.c
>> >> --- b/src/bin/pg_basebackup/pg_receivexlog.c
>> >> ***************
>> >> *** 591,596 **** main(int argc, char **argv)
>> >> --- 591,598 ----
>> >>                       disconnect_and_exit(1);
>> >>       }
>> >>
>> >> +     PQfinish(conn);
>> >> +
>> >>       while (true)
>> >>       {
>> >>               StreamLog();
>> >
>> > But wouldn't it be better to simply pass in the connection to
>> > StreamLog()?
>>
>> ISTM that the idea would make the code in StreamLog() somewhat complicated,
>> i.e., StreamLog() needs to always check whether the conn is valid or not before
>> trying to create new connection. We cannot remove the code to create new
>> connection in StreamLog() because it needs to reconnect to the server when
>> the connection is terminated (of course in the case where --no-loop is
>> not specified).
>
> Not that much imo.
>
> if (conn == NULL)
>    conn = GetConnection();
>
> if (!conn)
>         /* Error message already written in GetConnection() */
>         return;
>
> ...
>
> PQfinish(conn);
> conn = NULL;

I'm OK with that.

Regards,

--
Fujii Masao

Re: pgsql: Add support for managing physical replication slots to pg_receiv

From
Fujii Masao
Date:
On Tue, Oct 7, 2014 at 2:58 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
> On Tue, Oct 7, 2014 at 2:55 PM, Andres Freund <andres@anarazel.de> wrote:
>> On 2014-10-07 14:51:59 +0900, Fujii Masao wrote:
>>> >> *** a/src/bin/pg_basebackup/pg_receivexlog.c
>>> >> --- b/src/bin/pg_basebackup/pg_receivexlog.c
>>> >> ***************
>>> >> *** 591,596 **** main(int argc, char **argv)
>>> >> --- 591,598 ----
>>> >>                       disconnect_and_exit(1);
>>> >>       }
>>> >>
>>> >> +     PQfinish(conn);
>>> >> +
>>> >>       while (true)
>>> >>       {
>>> >>               StreamLog();
>>> >
>>> > But wouldn't it be better to simply pass in the connection to
>>> > StreamLog()?
>>>
>>> ISTM that the idea would make the code in StreamLog() somewhat complicated,
>>> i.e., StreamLog() needs to always check whether the conn is valid or not before
>>> trying to create new connection. We cannot remove the code to create new
>>> connection in StreamLog() because it needs to reconnect to the server when
>>> the connection is terminated (of course in the case where --no-loop is
>>> not specified).
>>
>> Not that much imo.
>>
>> if (conn == NULL)
>>    conn = GetConnection();
>>
>> if (!conn)
>>         /* Error message already written in GetConnection() */
>>         return;
>>
>> ...
>>
>> PQfinish(conn);
>> conn = NULL;
>
> I'm OK with that.

Attached patch does what Andres suggested. Barring any objection, I
will apply it.

Regards,

--
Fujii Masao
Attachment

Re: pgsql: Add support for managing physical replication slots to pg_receiv

From
Andres Freund
Date:
On 2014-10-17 20:28:29 +0900, Fujii Masao wrote:
> On Tue, Oct 7, 2014 at 2:58 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
> > On Tue, Oct 7, 2014 at 2:55 PM, Andres Freund <andres@anarazel.de> wrote:
> >> On 2014-10-07 14:51:59 +0900, Fujii Masao wrote:
> >>> >> *** a/src/bin/pg_basebackup/pg_receivexlog.c
> >>> >> --- b/src/bin/pg_basebackup/pg_receivexlog.c
> >>> >> ***************
> >>> >> *** 591,596 **** main(int argc, char **argv)
> >>> >> --- 591,598 ----
> >>> >>                       disconnect_and_exit(1);
> >>> >>       }
> >>> >>
> >>> >> +     PQfinish(conn);
> >>> >> +
> >>> >>       while (true)
> >>> >>       {
> >>> >>               StreamLog();
> >>> >
> >>> > But wouldn't it be better to simply pass in the connection to
> >>> > StreamLog()?
> >>>
> >>> ISTM that the idea would make the code in StreamLog() somewhat complicated,
> >>> i.e., StreamLog() needs to always check whether the conn is valid or not before
> >>> trying to create new connection. We cannot remove the code to create new
> >>> connection in StreamLog() because it needs to reconnect to the server when
> >>> the connection is terminated (of course in the case where --no-loop is
> >>> not specified).
> >>
> >> Not that much imo.
> >>
> >> if (conn == NULL)
> >>    conn = GetConnection();
> >>
> >> if (!conn)
> >>         /* Error message already written in GetConnection() */
> >>         return;
> >>
> >> ...
> >>
> >> PQfinish(conn);
> >> conn = NULL;
> >
> > I'm OK with that.
>
> Attached patch does what Andres suggested.

Thanks. That looks good. Sorry for you having to do that - I'd kind of
hoped that Michael would send a patch...

Greetings,

Andres Freund

Re: pgsql: Add support for managing physical replication slots to pg_receiv

From
Michael Paquier
Date:
On Fri, Oct 17, 2014 at 8:31 PM, Andres Freund <andres@anarazel.de> wrote:
Thanks. That looks good. Sorry for you having to do that - I'd kind of
hoped that Michael would send a patch...
Sorry, I simply slipped through it. The patch looks good.
--
Michael

Re: pgsql: Add support for managing physical replication slots to pg_receiv

From
Fujii Masao
Date:
On Fri, Oct 17, 2014 at 8:53 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Fri, Oct 17, 2014 at 8:31 PM, Andres Freund <andres@anarazel.de> wrote:
>>
>> Thanks. That looks good. Sorry for you having to do that - I'd kind of
>> hoped that Michael would send a patch...
>
> Sorry, I simply slipped through it. The patch looks good.

Thanks for the review! Applied.

Regards,

--
Fujii Masao