2012-10-10 20:36 keltezéssel, Boszormenyi Zoltan írta:
2012-10-10 18:23 keltezéssel, Fujii Masao írta:
On Wed, Oct 10, 2012 at 10:12 PM, Boszormenyi Zoltan <z...@cybertec.at> wrote:
Hi,
thanks for the new review.
2012-10-10 08:58 keltezéssel, Amit Kapila írta:
On Thursday, October 04, 2012 9:50 PM Boszormenyi Zoltan
2012-10-04 16:43 keltezéssel, Tom Lane írta:
Boszormenyi Zoltan <z...@cybertec.at> writes:
Did you think about something like the attached code?
Or rather this one, which fixes a bug so fillPGconn() and
PQconninfo() are symmetric and work for "requiressl".
That's incredibly ugly. I'm not sure where we should keep the "R"
information, but shoehorning it into the existing PQconninfoOption
struct like that seems totally unacceptable. Either we're willing to
break backwards compatibility on the Disp-Char strings, or we need to
put that info somewhere else.
I hope only handling the new flag part is ugly. It can be hidden in the
PQconninfoMapping[] array and PQconninfo(conn, true) pre-filters the
list as in the attached version.
Please find the review of the patch.
Basic stuff:
------------
- patch apply failed at exports.txt file. Needs rebase to the current
master.
Done.
- Compiles cleanly with no warnings
What it does:
--------------
pg_basebackup does the base backup from the primary machine and also
writes
the recovery.conf file with the option -R,
which is used for the standby to connect to primary for streaming
replication.
Testing:
---------
1. Test pg_basebackup with option -R to check that the recovery.conf file
is
written to data directory.
--recovery.conf file is created in data directory.
2. Test pg_basebackup with option -R to check that the recovery.conf
file is
not able to create because of disk full.
--Error is given as recovery.conf file is not able to create.
3. Test pg_basebackup with option -R including -h, -U, -p, -w and -W.
verify the recovery.conf which is created in data directory.
--All the primary connection info parameters are working fine.
4. Test pg_basebackup with conflict options (-x or -X and -R).
--Error is given when the conflict options are provided to
pg_basebackup.
5. Test pg_basebackup with option -R from a standby server.
--recovery.conf file is created in data directory.
Code Review:
-------------
1.
typedef struct PQconninfoMapping {
+ char *keyword;
+ size_t member_offset;
+ bool for_replication;
+ char *conninfoValue; /* Special value
mapping
*/
+ char *connValue;
+} PQconninfoMapping;
Provide the better explanation of conninfoValue and connValue, how and
where
these are used?
OK. This is only used for " requiressl='1' " (in the connection string)
and if '1' is set, PGconn.sslmode will be set to "require". All other
values are treated as it's being unset. This simplistic mapping
is used because there is only one such setting where different values
are used on the conninfo and the PGconn sides.
2. if (tmp && strncmp(tmp, mapping->conninfoValue, len) == 0)
In any case if the above condition is not satisfied then the PGconn data
is
not filled with PQconninfoOption.
Is it correct?
Yes, it stays NULL as makeEmptyPGconn() initialized it. This case only
happens
with the "requiressl" setting with its special mapping. If you set "
requiressl = '0' "
then it means that " requiressl='1' " was not set so the PGconn side stays
as NULL.
The special casing was there in the old code too and behaves the same.
Documentation:
-------------
1.
+ <para>
+ The <literal>for_replication</> argument can be used to exclude
some
+ options from the list which are used by the walreceiver module.
+ <application>pg_basebackup</application> uses this pre-filtered
list
+ to construct <literal>primary_conninfo</> in the automatically
generated
+ recovery.conf file.
+ </para>
I feel the explanation should be as follows,
exclude some options from the list which are not used by the walreceiver
module?
Err, no. The list excludes those[1] that *are* used (would be
overridden) by the walreceiver module:
----8<--------8<--------8<--------8<--------8<----
static bool
libpqrcv_connect(char *conninfo, XLogRecPtr startpoint)
{
...
snprintf(conninfo_repl, sizeof(conninfo_repl),
"%s dbname=replication replication=true
fallback_application_name=walreceiver",
conninfo);
----8<--------8<--------8<--------8<--------8<----
[1] Actually, more than these 3 options are excluded. The deprecated
ones are also excluded.
Observations/Issues:
-------------------
1. If the password contains any escape sequence characters, It is leading
to
problems while walreceiver connecting to primary
by using the primary conninfo from recovery.conf
please log an warning message or a note in document to handle such a
case
manually by the user.
Done at both places.
Also, to adapt to the style of other error messages, now all my fprintf()
statements
are prefixed with: "%s: ...", progname.
In new patches, when I ran "pg_basebackup -D hoge -c fast -R" on MacOS,
I got the following error message. BTW, I compiled the patched PostgreSQL
with --enable-debug and --enable-cassert options.
pg_basebackup(41751) malloc: *** error for object 0x106001af0: pointer
being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
Abort trap: 6
Can you show a backtrace? I compiled it on Fedora 17/x86_64 with
--enable-depend --enable-debug --enable-cassert. GLIBC is also smart
enough to catch improper free() calls, too.
I was able to test it on OSX and I found my bug. Attached is the new code.
The problem was in conninfo_init(), the last entry in the filtered list was
not initialized to 0. It seems that for some reason, my Linux machine gave
me pre-initialized memory.
$ uname -a
Darwin hrk.local 11.4.2 Darwin Kernel Version 11.4.2: Thu Aug 23
16:25:48 PDT 2012; root:xnu-1699.32.7~1/RELEASE_X86_64 x86_64
When tar output format is specified together with -R option, recovery.conf is
not included in base.tar. I think it should.
Why? This patch only promises to write the recovery.conf into the
directory specified with -D.
+ setting up the standby. Since creating a backup for a standalone
+ server with <option>-x</option> or <option>-X</option> and adding
+ a recovery.conf to it wouldn't make a working standby, these options
+ naturally conflict.
Is this right? ISTM that basically we can use pg_basebackup -x to take
a base backup for starting the standby for now. No?
I don't know. Pointers?
Thanks,
Zoltán Böszörményi
--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/
diff -durpN postgresql/doc/src/sgml/libpq.sgml postgresql.1/doc/src/sgml/libpq.sgml
--- postgresql/doc/src/sgml/libpq.sgml 2012-08-03 09:39:30.114266570 +0200
+++ postgresql.1/doc/src/sgml/libpq.sgml 2012-10-10 14:09:31.924890230 +0200
@@ -496,6 +496,37 @@ typedef struct
</listitem>
</varlistentry>
+ <varlistentry id="libpq-pqconninfo">
+ <term><function>PQconninfo</function><indexterm><primary>PQconninfo</></></term>
+ <listitem>
+ <para>
+ Returns the connection options used by a live connection.
+<synopsis>
+PQconninfoOption *PQconninfo(PGconn *conn, bool for_replication);
+</synopsis>
+ </para>
+
+ <para>
+ Returns a connection options array. This can be used to determine
+ all possible <function>PQconnectdb</function> options and their
+ current values that were used to connect to the server. The return
+ value points to an array of <structname>PQconninfoOption</structname>
+ structures, which ends with an entry having a null <structfield>keyword</>
+ pointer. Every notes above for <function>PQconndefaults</function> also apply.
+ </para>
+
+ <para>
+ The <literal>for_replication</> argument can be used to exclude some
+ options from the list which are used by the walreceiver module.
+ <application>pg_basebackup</application> uses this pre-filtered list
+ to construct <literal>primary_conninfo</> in the automatically generated
+ recovery.conf file.
+ </para>
+
+ </listitem>
+ </varlistentry>
+
+
<varlistentry id="libpq-pqconninfoparse">
<term><function>PQconninfoParse</function><indexterm><primary>PQconninfoParse</></></term>
<listitem>
diff -durpN postgresql/src/interfaces/libpq/exports.txt postgresql.1/src/interfaces/libpq/exports.txt
--- postgresql/src/interfaces/libpq/exports.txt 2012-10-09 09:58:14.342782974 +0200
+++ postgresql.1/src/interfaces/libpq/exports.txt 2012-10-10 14:09:56.052057835 +0200
@@ -164,3 +164,4 @@ PQsetSingleRowMode 161
lo_lseek64 162
lo_tell64 163
lo_truncate64 164
+PQconninfo 165
diff -durpN postgresql/src/interfaces/libpq/fe-connect.c postgresql.1/src/interfaces/libpq/fe-connect.c
--- postgresql/src/interfaces/libpq/fe-connect.c 2012-09-09 08:11:09.470401480 +0200
+++ postgresql.1/src/interfaces/libpq/fe-connect.c 2012-10-10 21:53:05.502966398 +0200
@@ -137,6 +137,9 @@ static int ldapServiceLookup(const char
* PQconninfoOptions[] *must* be NULL. In a working copy, non-null "val"
* fields point to malloc'd strings that should be freed when the working
* array is freed (see PQconninfoFree).
+ *
+ * If you add a new connection option to this list, remember to add it to
+ * PQconninfoMappings[] below.
* ----------
*/
static const PQconninfoOption PQconninfoOptions[] = {
@@ -264,6 +267,66 @@ static const PQconninfoOption PQconninfo
NULL, NULL, 0}
};
+/*
+ * We need a mapping between the PQconninfoOptions[] array
+ * and PGconn members. We have to keep it separate from
+ * PQconninfoOptions[] to not leak info about PGconn members
+ * to clients.
+ */
+typedef struct PQconninfoMapping {
+ char *keyword;
+ size_t member_offset;
+ bool for_replication;
+ /*
+ * Special and simplistic value mapping between
+ * PQconninfoOption and PGconn. Only used by "requiressl".
+ */
+ char *conninfoValue;
+ char *connValue;
+} PQconninfoMapping;
+#define PGCONNMEMBERADDR(conn, mapping) ((char **)((char *)conn + mapping->member_offset))
+
+static const PQconninfoMapping PQconninfoMappings[] =
+{
+ /* "authtype" is not used anymore, there is no mapping to PGconn */
+ /* there is no mapping of "service" to PGconn */
+ { "user", offsetof(struct pg_conn, pguser), true, NULL, NULL },
+ { "password", offsetof(struct pg_conn, pgpass), true, NULL, NULL },
+ { "connect_timeout", offsetof(struct pg_conn, connect_timeout), true, NULL, NULL },
+ { "dbname", offsetof(struct pg_conn, dbName), false, NULL, NULL },
+ { "host", offsetof(struct pg_conn, pghost), true, NULL, NULL },
+ { "hostaddr", offsetof(struct pg_conn, pghostaddr), true, NULL, NULL },
+ { "port", offsetof(struct pg_conn, pgport), true, NULL, NULL },
+ { "client_encoding", offsetof(struct pg_conn, client_encoding_initial), false, NULL, NULL },
+ { "tty", offsetof(struct pg_conn, pgtty), false, NULL, NULL },
+ { "options", offsetof(struct pg_conn, pgoptions), true, NULL, NULL },
+ { "application_name", offsetof(struct pg_conn, appname), false, NULL, NULL },
+ { "fallback_application_name", offsetof(struct pg_conn, fbappname), false, NULL, NULL },
+ { "keepalives", offsetof(struct pg_conn, keepalives), true, NULL, NULL },
+ { "keepalives_idle", offsetof(struct pg_conn, keepalives_idle), true, NULL, NULL },
+ { "keepalives_interval", offsetof(struct pg_conn, keepalives_interval), true, NULL, NULL },
+ { "keepalives_count", offsetof(struct pg_conn, keepalives_count), true, NULL, NULL },
+#ifdef USE_SSL
+ { "requiressl", offsetof(struct pg_conn, sslmode), false, "1", "require" },
+#endif
+ { "sslmode", offsetof(struct pg_conn, sslmode), true, NULL, NULL },
+ { "sslcompression", offsetof(struct pg_conn, sslcompression), true, NULL, NULL },
+ { "sslcert", offsetof(struct pg_conn, sslcert), true, NULL, NULL },
+ { "sslkey", offsetof(struct pg_conn, sslkey), true, NULL, NULL },
+ { "sslrootcert", offsetof(struct pg_conn, sslrootcert), true, NULL, NULL },
+ { "sslcrl", offsetof(struct pg_conn, sslcrl), true, NULL, NULL },
+ { "requirepeer", offsetof(struct pg_conn, requirepeer), true, NULL, NULL },
+#if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI)
+ { "krbsrvname", offsetof(struct pg_conn, krbsrvname), true, NULL, NULL },
+#endif
+#if defined(ENABLE_GSS) && defined(ENABLE_SSPI)
+ { "gsslib", offsetof(struct pg_conn, requirepeer), true, NULL, NULL },
+#endif
+ { "replication", offsetof(struct pg_conn, replication), false, NULL, NULL },
+ /* Terminating entry --- MUST BE LAST */
+ { NULL, 0, false, NULL, NULL }
+};
+
static const PQEnvironmentOption EnvironmentOptions[] =
{
/* common user-interface settings */
@@ -295,7 +358,8 @@ static PGconn *makeEmptyPGconn(void);
static void fillPGconn(PGconn *conn, PQconninfoOption *connOptions);
static void freePGconn(PGconn *conn);
static void closePGconn(PGconn *conn);
-static PQconninfoOption *conninfo_init(PQExpBuffer errorMessage);
+static PQconninfoOption *conninfo_init(PQExpBuffer errorMessage,
+ bool for_replication);
static PQconninfoOption *parse_connection_string(const char *conninfo,
PQExpBuffer errorMessage, bool use_defaults);
static int uri_prefix_length(const char *connstr);
@@ -318,6 +382,8 @@ static char *conninfo_uri_decode(const c
static bool get_hexdigit(char digit, int *value);
static const char *conninfo_getval(PQconninfoOption *connOptions,
const char *keyword);
+static void conninfo_setval(PQconninfoOption *connOptions,
+ const char *keyword, const char *val);
static PQconninfoOption *conninfo_storeval(PQconninfoOption *connOptions,
const char *keyword, const char *value,
PQExpBuffer errorMessage, bool ignoreMissing, bool uri_decode);
@@ -627,7 +693,9 @@ PQconnectStart(const char *conninfo)
static void
fillPGconn(PGconn *conn, PQconninfoOption *connOptions)
{
+ const PQconninfoMapping *mapping;
const char *tmp;
+ char **memberaddr;
/*
* Move option values into conn structure
@@ -637,72 +705,24 @@ fillPGconn(PGconn *conn, PQconninfoOptio
*
* XXX: probably worth checking strdup() return value here...
*/
- tmp = conninfo_getval(connOptions, "hostaddr");
- conn->pghostaddr = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "host");
- conn->pghost = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "port");
- conn->pgport = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "tty");
- conn->pgtty = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "options");
- conn->pgoptions = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "application_name");
- conn->appname = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "fallback_application_name");
- conn->fbappname = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "dbname");
- conn->dbName = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "user");
- conn->pguser = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "password");
- conn->pgpass = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "connect_timeout");
- conn->connect_timeout = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "client_encoding");
- conn->client_encoding_initial = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "keepalives");
- conn->keepalives = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "keepalives_idle");
- conn->keepalives_idle = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "keepalives_interval");
- conn->keepalives_interval = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "keepalives_count");
- conn->keepalives_count = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "sslmode");
- conn->sslmode = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "sslcompression");
- conn->sslcompression = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "sslkey");
- conn->sslkey = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "sslcert");
- conn->sslcert = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "sslrootcert");
- conn->sslrootcert = tmp ? strdup(tmp) : NULL;
- tmp = conninfo_getval(connOptions, "sslcrl");
- conn->sslcrl = tmp ? strdup(tmp) : NULL;
-#ifdef USE_SSL
- tmp = conninfo_getval(connOptions, "requiressl");
- if (tmp && tmp[0] == '1')
+ for (mapping = PQconninfoMappings; mapping->keyword; mapping++)
{
- /* here warn that the requiressl option is deprecated? */
- if (conn->sslmode)
- free(conn->sslmode);
- conn->sslmode = strdup("require");
+ tmp = conninfo_getval(connOptions, mapping->keyword);
+ memberaddr = PGCONNMEMBERADDR(conn, mapping);
+
+ if (mapping->conninfoValue && mapping->connValue)
+ {
+ size_t len = strlen(mapping->conninfoValue);
+ if (tmp && strncmp(tmp, mapping->conninfoValue, len) == 0)
+ {
+ if (*memberaddr)
+ free(*memberaddr);
+ *memberaddr = strdup(mapping->connValue);
+ }
+ }
+ else
+ *memberaddr = tmp ? strdup(tmp) : NULL;
}
-#endif
- tmp = conninfo_getval(connOptions, "requirepeer");
- conn->requirepeer = tmp ? strdup(tmp) : NULL;
-#if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI)
- tmp = conninfo_getval(connOptions, "krbsrvname");
- conn->krbsrvname = tmp ? strdup(tmp) : NULL;
-#endif
-#if defined(ENABLE_GSS) && defined(ENABLE_SSPI)
- tmp = conninfo_getval(connOptions, "gsslib");
- conn->gsslib = tmp ? strdup(tmp) : NULL;
-#endif
- tmp = conninfo_getval(connOptions, "replication");
- conn->replication = tmp ? strdup(tmp) : NULL;
}
/*
@@ -884,7 +904,7 @@ PQconndefaults(void)
if (PQExpBufferDataBroken(errorBuf))
return NULL; /* out of memory already :-( */
- connOptions = conninfo_init(&errorBuf);
+ connOptions = conninfo_init(&errorBuf, false);
if (connOptions != NULL)
{
if (!conninfo_add_defaults(connOptions, &errorBuf))
@@ -4006,9 +4026,11 @@ PQconninfoParse(const char *conninfo, ch
/*
* Build a working copy of the constant PQconninfoOptions array.
+ * If for_replication is true, only return the options that are
+ * not added by libpqwalreceiver.
*/
static PQconninfoOption *
-conninfo_init(PQExpBuffer errorMessage)
+conninfo_init(PQExpBuffer errorMessage, bool for_replication)
{
PQconninfoOption *options;
@@ -4019,7 +4041,28 @@ conninfo_init(PQExpBuffer errorMessage)
libpq_gettext("out of memory\n"));
return NULL;
}
- memcpy(options, PQconninfoOptions, sizeof(PQconninfoOptions));
+ if (for_replication)
+ {
+ const PQconninfoMapping *mapping = PQconninfoMappings;
+ PQconninfoOption *opt_dest = options;
+
+ while (mapping->keyword)
+ {
+ PQconninfoOption *opt_src = conninfo_find((PQconninfoOption *)PQconninfoOptions, mapping->keyword);
+
+ if (opt_src && mapping->for_replication)
+ {
+ memcpy(opt_dest, opt_src, sizeof(PQconninfoOption));
+ opt_dest++;
+ }
+
+ opt_src++;
+ mapping++;
+ }
+ MemSet(opt_dest, 0, sizeof(PQconninfoOption));
+ }
+ else
+ memcpy(options, PQconninfoOptions, sizeof(PQconninfoOptions));
return options;
}
@@ -4095,7 +4138,7 @@ conninfo_parse(const char *conninfo, PQE
PQconninfoOption *options;
/* Make a working copy of PQconninfoOptions */
- options = conninfo_init(errorMessage);
+ options = conninfo_init(errorMessage, false);
if (options == NULL)
return NULL;
@@ -4295,7 +4338,7 @@ conninfo_array_parse(const char *const *
}
/* Make a working copy of PQconninfoOptions */
- options = conninfo_init(errorMessage);
+ options = conninfo_init(errorMessage, false);
if (options == NULL)
{
PQconninfoFree(dbname_options);
@@ -4485,7 +4528,7 @@ conninfo_uri_parse(const char *uri, PQEx
PQconninfoOption *options;
/* Make a working copy of PQconninfoOptions */
- options = conninfo_init(errorMessage);
+ options = conninfo_init(errorMessage, false);
if (options == NULL)
return NULL;
@@ -4985,6 +5028,24 @@ conninfo_getval(PQconninfoOption *connOp
}
/*
+ * Set an option value corresponding to the keyword in the connOptions array.
+ */
+static void
+conninfo_setval(PQconninfoOption *connOptions, const char *keyword,
+ const char *val)
+{
+ PQconninfoOption *option;
+
+ option = conninfo_find(connOptions, keyword);
+ if (option)
+ {
+ if (option->val)
+ free(option->val);
+ option->val = val ? strdup(val) : NULL;
+ }
+}
+
+/*
* Store a (new) value for an option corresponding to the keyword in
* connOptions array.
*
@@ -5066,6 +5127,50 @@ conninfo_find(PQconninfoOption *connOpti
}
+/*
+ * Return the connection options used for the connections
+ */
+PQconninfoOption *
+PQconninfo(PGconn *conn, bool for_replication)
+{
+ PQExpBufferData errorBuf;
+ PQconninfoOption *connOptions;
+
+ if (conn == NULL)
+ return NULL;
+
+ /* We don't actually report any errors here, but callees want a buffer */
+ initPQExpBuffer(&errorBuf);
+ if (PQExpBufferDataBroken(errorBuf))
+ return NULL; /* out of memory already :-( */
+
+ connOptions = conninfo_init(&errorBuf, for_replication);
+
+ termPQExpBuffer(&errorBuf);
+
+ if (connOptions != NULL)
+ {
+ const PQconninfoMapping *mapping;
+
+ for (mapping = PQconninfoMappings; mapping->keyword; mapping++)
+ {
+ char **memberaddr = PGCONNMEMBERADDR(conn, mapping);
+
+ if (mapping->conninfoValue && mapping->connValue)
+ {
+ size_t len = strlen(mapping->connValue);
+ if (*memberaddr && strncmp(*memberaddr, mapping->connValue, len) == 0)
+ conninfo_setval(connOptions, mapping->keyword, mapping->conninfoValue);
+ }
+ else
+ conninfo_setval(connOptions, mapping->keyword, *memberaddr);
+ }
+ }
+
+ return connOptions;
+}
+
+
void
PQconninfoFree(PQconninfoOption *connOptions)
{
diff -durpN postgresql/src/interfaces/libpq/libpq-fe.h postgresql.1/src/interfaces/libpq/libpq-fe.h
--- postgresql/src/interfaces/libpq/libpq-fe.h 2012-10-09 09:58:14.343782980 +0200
+++ postgresql.1/src/interfaces/libpq/libpq-fe.h 2012-10-10 14:09:31.936890314 +0200
@@ -262,6 +262,9 @@ extern PQconninfoOption *PQconndefaults(
/* parse connection options in same way as PQconnectdb */
extern PQconninfoOption *PQconninfoParse(const char *conninfo, char **errmsg);
+/* return the connection options used by a live connection */
+extern PQconninfoOption *PQconninfo(PGconn *conn, bool for_replication);
+
/* free the data structure returned by PQconndefaults() or PQconninfoParse() */
extern void PQconninfoFree(PQconninfoOption *connOptions);
diff -durpN postgresql.1/doc/src/sgml/ref/pg_basebackup.sgml postgresql.2/doc/src/sgml/ref/pg_basebackup.sgml
--- postgresql.1/doc/src/sgml/ref/pg_basebackup.sgml 2012-08-24 09:49:22.960530329 +0200
+++ postgresql.2/doc/src/sgml/ref/pg_basebackup.sgml 2012-10-10 14:49:40.869336237 +0200
@@ -189,6 +189,36 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>-R</option></term>
+ <term><option>--write-recovery-conf</option></term>
+ <listitem>
+
+ <para>
+ Write a minimal recovery.conf into the output directory using
+ the connection parameters from the command line to ease
+ setting up the standby. Since creating a backup for a standalone
+ server with <option>-x</option> or <option>-X</option> and adding
+ a recovery.conf to it wouldn't make a working standby, these options
+ naturally conflict.
+ </para>
+
+ <para>
+ The password written into recovery.conf is not escaped even if special
+ characters appear in it. The administrator must review recovery.conf
+ to ensure proper escaping.
+ </para>
+
+ <para>
+ When this option is specified and taking the base backup succeeded,
+ failing to write the <filename>recovery.conf</filename> results
+ in the error code 2. This way, scripts can distinguish between different
+ failure cases of <application>pg_basebackup</application>.
+ </para>
+
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>-x</option></term>
<term><option>--xlog</option></term>
<listitem>
diff -durpN postgresql.1/src/bin/pg_basebackup/pg_basebackup.c postgresql.2/src/bin/pg_basebackup/pg_basebackup.c
--- postgresql.1/src/bin/pg_basebackup/pg_basebackup.c 2012-10-03 10:40:48.297207389 +0200
+++ postgresql.2/src/bin/pg_basebackup/pg_basebackup.c 2012-10-10 14:53:18.717764755 +0200
@@ -46,6 +46,7 @@ int compresslevel = 0;
bool includewal = false;
bool streamwal = false;
bool fastcheckpoint = false;
+bool writerecoveryconf = false;
int standby_message_timeout = 10 * 1000; /* 10 sec = default */
/* Progress counters */
@@ -70,14 +71,18 @@ static int has_xlogendptr = 0;
static volatile LONG has_xlogendptr = 0;
#endif
+static PQconninfoOption *connOptions = NULL;
+
/* Function headers */
static void usage(void);
static void verify_dir_is_empty_or_create(char *dirname);
static void progress_report(int tablespacenum, const char *filename);
+static void stderr_write_error(FILE *cf, char *filename);
static void ReceiveTarFile(PGconn *conn, PGresult *res, int rownum);
static void ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum);
static void BaseBackup(void);
+static void WriteRecoveryConf(void);
static bool reached_end_position(XLogRecPtr segendpos, uint32 timeline,
bool segment_finished);
@@ -107,6 +112,8 @@ usage(void)
printf(_("\nOptions controlling the output:\n"));
printf(_(" -D, --pgdata=DIRECTORY receive base backup into directory\n"));
printf(_(" -F, --format=p|t output format (plain (default), tar)\n"));
+ printf(_(" -R, --write-recovery-conf\n"
+ " write recovery.conf after backup\n"));
printf(_(" -x, --xlog include required WAL files in backup (fetch mode)\n"));
printf(_(" -X, --xlog-method=fetch|stream\n"
" include required WAL files with specified method\n"));
@@ -960,6 +967,10 @@ BaseBackup(void)
/* Error message already written in GetConnection() */
exit(1);
+ /* If recovery.conf is to be written, keep the connection parameters for later usage */
+ if (writerecoveryconf)
+ connOptions = PQconninfo(conn, true);
+
/*
* Run IDENTIFY_SYSTEM so we can get the timeline
*/
@@ -1234,6 +1245,73 @@ BaseBackup(void)
}
+static void
+stderr_write_error(FILE *cf, char *filename)
+{
+ fprintf(stderr, _("%s: cannot write to %s: %s"), progname, filename, strerror(errno));
+ fclose(cf);
+ unlink(filename);
+ exit(2);
+}
+
+
+/*
+ * Attempt to create recovery.conf and write the expected contents to it.
+ */
+static void
+WriteRecoveryConf(void)
+{
+ char filename[MAXPGPATH];
+ PQconninfoOption *option;
+ FILE *cf;
+
+ if (!writerecoveryconf)
+ return;
+
+ sprintf(filename, "%s/recovery.conf", basedir);
+
+ cf = fopen(filename, "w");
+ if (cf == NULL)
+ {
+ fprintf(stderr, _("%s: cannot create %s: %s"), progname, filename, strerror(errno));
+ exit(2);
+ }
+
+ if (fprintf(cf, "standby_mode = 'on'\n") < 0)
+ stderr_write_error(cf, filename);
+
+ if (fprintf(cf, "primary_conninfo = '") < 0)
+ stderr_write_error(cf, filename);
+
+ for (option = connOptions; option && option->keyword; option++)
+ {
+ /*
+ * Do not emit this setting if not set, empty or default.
+ * The list of options was already pre-filtered for options
+ * usable for replication with PQconninfo(conn, true).
+ */
+ if (option->val == NULL ||
+ (option->val != NULL && option->val[0] == '\0') ||
+ (option->val &&
+ option->compiled &&
+ strcmp(option->val, option->compiled) == 0))
+ continue;
+
+ /* write "keyword='value'" pieces, single quotes doubled */
+ if (fprintf(cf, "%s=''%s'' ", option->keyword, option->val) < 0)
+ stderr_write_error(cf, filename);
+ }
+
+ if (fprintf(cf, "'\n") < 0)
+ stderr_write_error(cf, filename);
+
+ PQconninfoFree(connOptions);
+
+ fclose(cf);
+
+ fprintf(stderr, _("%s: recovery.conf written. Please, add escaping to the password field in \"primary_conninfo\" if needed.\n"), progname);
+}
+
int
main(int argc, char **argv)
{
@@ -1243,6 +1321,7 @@ main(int argc, char **argv)
{"pgdata", required_argument, NULL, 'D'},
{"format", required_argument, NULL, 'F'},
{"checkpoint", required_argument, NULL, 'c'},
+ {"write-recovery-conf", no_argument, NULL, 'R'},
{"xlog", no_argument, NULL, 'x'},
{"xlog-method", required_argument, NULL, 'X'},
{"gzip", no_argument, NULL, 'z'},
@@ -1280,7 +1359,7 @@ main(int argc, char **argv)
}
}
- while ((c = getopt_long(argc, argv, "D:F:xX:l:zZ:c:h:p:U:s:wWvP",
+ while ((c = getopt_long(argc, argv, "D:F:RxX:l:zZ:c:h:p:U:s:wWvP",
long_options, &option_index)) != -1)
{
switch (c)
@@ -1301,6 +1380,9 @@ main(int argc, char **argv)
exit(1);
}
break;
+ case 'R':
+ writerecoveryconf = true;
+ break;
case 'x':
if (includewal)
{
@@ -1466,6 +1548,13 @@ main(int argc, char **argv)
}
#endif
+ if (writerecoveryconf && includewal)
+ {
+ fprintf(stderr,
+ _("--xlog and --writerecoveryconf are mutually exclusive\n"));
+ exit(1);
+ }
+
/*
* Verify that the target directory exists, or create it. For plaintext
* backups, always require the directory. For tar backups, require it
@@ -1476,5 +1565,7 @@ main(int argc, char **argv)
BaseBackup();
+ WriteRecoveryConf();
+
return 0;
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
