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

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