On 2014-10-03 10:30:19 +0900, Michael Paquier wrote:
> On Thu, Oct 2, 2014 at 12:44 AM, Andres Freund <andres@2ndquadrant.com>
> wrote:
>
> > I pushed the first part.
> >
> Thanks. Attached is a rebased version of patch 2, implementing the actual
> feature. One thing I noticed with more testing is that if --create is used
> and that the destination folder does not exist, pg_receivexlog was creating
> the slot, and left with an error. This does not look user-friendly so I
> changed the logic a bit to check for the destination folder before creating
> any slot. This results in a bit of refactoring, but this way behavior is
> more intuitive.
Ok.
> <para>
> + <application>pg_receivexlog</application> can run in one of two following
> + modes, which control physical replication slot:
I don't think that's good enough. There's also the important mode where
it's not doing --create/--drop at all.
> + /*
> + * Run IDENTIFY_SYSTEM so we can get the timeline and current xlog
> + * position.
> + */
> + if (!RunIdentifySystem(conn, NULL, NULL, NULL, &db_name))
> + disconnect_and_exit(1);
> +
> + /*
> + * Check that there is a database associated with connection, none
> + * should be defined in this context.
> + */
> + if (db_name)
> + {
> + fprintf(stderr,
> + _("%s: database defined for replication connection \"%s\"\n"),
> + progname, replication_slot);
> + disconnect_and_exit(1);
> + }
I don't like 'defined' here. 'replication connection unexpectedly is
database specific' or something would be better.
I do wonder whether --create/--drop aren't somewhat wierd for
pg_receivexlog. It's not that clear what it means. It'd be ugly, but we
could rename them --create-slot/drop-slot.
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
