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

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