pgsql: Add support for managing physical replication slots to pg_receiv

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

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

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

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

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

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers