Hi,
While testing wal2json, I faced some problems with pg_recvlogical.
Attached is a serie of patches that can improve pg_recvlogical. Patches
#2 and #3 are bugfixes (and should be applied to 9.5 too). Patch #1 is
not mandatory to 9.5.
Short description:
#1: add a bunch of checks to complain when using an option that is not
available in the specified action;
#2: there is a wrong check because startpos option can be specified with
--create-slot;
#3: doesn't ignore startpos in --create-slot because that action could
be specified together with --start action (that uses that option);
--
Euler Taveira Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
>From 9b3d63d1744e2b65a7f1a1d44ed166f4c9684771 Mon Sep 17 00:00:00 2001
From: Euler Taveira <eu...@timbira.com.br>
Date: Tue, 1 Sep 2015 23:52:55 -0300
Subject: [PATCH 3/3] Fix a startpos override.
Since --create-slot and --start can be specified together, we can't
override startpos while creating a slot (it'll ignore the replication
start position, if specified).
---
src/bin/pg_basebackup/pg_recvlogical.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index b59fe13..a2284d2 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -969,7 +969,6 @@ main(int argc, char **argv)
if (!CreateReplicationSlot(conn, replication_slot, plugin,
false, slot_exists_ok))
disconnect_and_exit(1);
- startpos = InvalidXLogRecPtr;
}
if (!do_start_slot)
--
2.1.4
>From 8393aa62cddeaff8cc66b19c6ba36a371b090db1 Mon Sep 17 00:00:00 2001
From: Euler Taveira <eu...@timbira.com.br>
Date: Tue, 1 Sep 2015 23:46:33 -0300
Subject: [PATCH 2/3] Fix startpos parameter check.
startpos parameter can be specified together with --create-slot (--start
must be specified too). This check is wrong since --create-slot and
--start could be specified together.
---
src/bin/pg_basebackup/pg_recvlogical.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 9ce237f..b59fe13 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -848,9 +848,9 @@ main(int argc, char **argv)
exit(1);
}
- if (startpos != InvalidXLogRecPtr && (do_create_slot || do_drop_slot))
+ if (startpos != InvalidXLogRecPtr && !do_start_slot)
{
- fprintf(stderr, _("%s: cannot use --create-slot or --drop-slot together with --startpos\n"), progname);
+ fprintf(stderr, _("%s: can only use --startpos together with --start\n"), progname);
fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
progname);
exit(1);
--
2.1.4
>From da241d9e2529888297c0f68bd89706fb59ae5884 Mon Sep 17 00:00:00 2001
From: Euler Taveira <eu...@timbira.com.br>
Date: Tue, 1 Sep 2015 23:34:51 -0300
Subject: [PATCH 1/3] pg_recvlogical: Tighten checks for action parameters.
Until now, the mistaken parameters are ignored (which is bad because
that parameter won't take the desired effect). Complaining about the
"wrong" parameter will help the user to fix the problem immediately.
---
doc/src/sgml/ref/pg_recvlogical.sgml | 2 +-
src/bin/pg_basebackup/pg_recvlogical.c | 82 +++++++++++++++++++++++++++++++---
2 files changed, 77 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/ref/pg_recvlogical.sgml b/doc/src/sgml/ref/pg_recvlogical.sgml
index 9d0b58b..a513047 100644
--- a/doc/src/sgml/ref/pg_recvlogical.sgml
+++ b/doc/src/sgml/ref/pg_recvlogical.sgml
@@ -149,7 +149,7 @@ PostgreSQL documentation
In <option>--start</option> mode, start replication from the given
LSN. For details on the effect of this, see the documentation
in <xref linkend="logicaldecoding">
- and <xref linkend="protocol-replication">. Ignored in other modes.
+ and <xref linkend="protocol-replication">.
</para>
</listitem>
</varlistentry>
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 93f61c3..9ce237f 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -30,12 +30,17 @@
/* Time to sleep between reconnection attempts */
#define RECONNECT_SLEEP_TIME 5
+#define DEFAULT_MESSAGE_TIMEOUT (10 * 1000) /* 10 secs = default */
+#define DEFAULT_FSYNC_INTERVAL (10 * 1000) /* 10 secs = defualt */
+#define DEFAULT_PLUGIN "test_decoding"
+
/* Global Options */
static char *outfile = NULL;
+static char *plugin = NULL;
static int verbose = 0;
static int noloop = 0;
-static int standby_message_timeout = 10 * 1000; /* 10 sec = default */
-static int fsync_interval = 10 * 1000; /* 10 sec = default */
+static int standby_message_timeout = -1;
+static int fsync_interval = -1;
static XLogRecPtr startpos = InvalidXLogRecPtr;
static bool do_create_slot = false;
static bool slot_exists_ok = false;
@@ -45,7 +50,6 @@ static bool do_drop_slot = false;
/* filled pairwise with option, value. value may be NULL */
static char **options;
static size_t noptions = 0;
-static const char *plugin = "test_decoding";
/* Global State */
static int outfd = -1;
@@ -75,16 +79,16 @@ usage(void)
printf(_("\nOptions:\n"));
printf(_(" -f, --file=FILE receive log into this file, - for stdout\n"));
printf(_(" -F --fsync-interval=SECS\n"
- " time between fsyncs to the output file (default: %d)\n"), (fsync_interval / 1000));
+ " time between fsyncs to the output file (default: %d)\n"), (DEFAULT_FSYNC_INTERVAL / 1000));
printf(_(" --if-not-exists do not error if slot already exists when creating a slot\n"));
printf(_(" -I, --startpos=LSN where in an existing slot should the streaming start\n"));
printf(_(" -n, --no-loop do not loop on connection lost\n"));
printf(_(" -o, --option=NAME[=VALUE]\n"
" pass option NAME with optional value VALUE to the\n"
" output plugin\n"));
- printf(_(" -P, --plugin=PLUGIN use output plugin PLUGIN (default: %s)\n"), plugin);
+ printf(_(" -P, --plugin=PLUGIN use output plugin PLUGIN (default: %s)\n"), DEFAULT_PLUGIN);
printf(_(" -s, --status-interval=SECS\n"
- " time between status packets sent to server (default: %d)\n"), (standby_message_timeout / 1000));
+ " time between status packets sent to server (default: %d)\n"), (DEFAULT_MESSAGE_TIMEOUT / 1000));
printf(_(" -S, --slot=SLOTNAME name of the logical replication slot\n"));
printf(_(" -v, --verbose output verbose messages\n"));
printf(_(" -V, --version output version information, then exit\n"));
@@ -266,6 +270,12 @@ StreamLogicalLog(void)
PQclear(res);
resetPQExpBuffer(query);
+ if (standby_message_timeout == -1)
+ standby_message_timeout = DEFAULT_MESSAGE_TIMEOUT;
+
+ if (fsync_interval == -1)
+ fsync_interval = DEFAULT_FSYNC_INTERVAL;
+
if (verbose)
fprintf(stderr,
_("%s: streaming initiated\n"),
@@ -846,6 +856,63 @@ main(int argc, char **argv)
exit(1);
}
+ if (!do_start_slot && outfile != NULL)
+ {
+ fprintf(stderr, _("%s: --file can only be used with --start\n"), progname);
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit(1);
+ }
+
+ if (!do_create_slot && slot_exists_ok)
+ {
+ fprintf(stderr, _("%s: --if-not-exists can only be used with --create-slot\n"), progname);
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit(1);
+ }
+
+ if (!do_start_slot && noloop == 1)
+ {
+ fprintf(stderr, _("%s: --no-loop can only be used with --start\n"), progname);
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit(1);
+ }
+
+ if (!do_start_slot && noptions > 0)
+ {
+ fprintf(stderr, _("%s: --option can only be used with --start\n"), progname);
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit(1);
+ }
+
+ if (!do_create_slot && plugin != NULL)
+ {
+ fprintf(stderr, _("%s: --plugin can only be used with --create-slot\n"), progname);
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit(1);
+ }
+
+ if (!do_start_slot && standby_message_timeout != -1)
+ {
+ fprintf(stderr, _("%s: --status-interval can only be used with --start\n"), progname);
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit(1);
+ }
+
+ if (!do_start_slot && fsync_interval != -1)
+ {
+ fprintf(stderr, _("%s: --fsync-interval can only be used with --start\n"), progname);
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit(1);
+ }
+
+
#ifndef WIN32
pqsignal(SIGINT, sigint_handler);
pqsignal(SIGHUP, sighup_handler);
@@ -896,6 +963,9 @@ main(int argc, char **argv)
_("%s: creating replication slot \"%s\"\n"),
progname, replication_slot);
+ if (plugin == NULL)
+ plugin = DEFAULT_PLUGIN;
+
if (!CreateReplicationSlot(conn, replication_slot, plugin,
false, slot_exists_ok))
disconnect_and_exit(1);
--
2.1.4
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
