public inbox for [email protected]
help / color / mirror / Atom feedFrom: Hayato Kuroda (Fujitsu) <[email protected]>
To: 'Amit Kapila' <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Subject: RE: Have pg_basebackup write "dbname" in "primary_conninfo"?
Date: Tue, 27 Feb 2024 08:30:19 +0000
Message-ID: <TYCPR01MB1207779B65625AAEC975B5EC0F5592@TYCPR01MB12077.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAA4eK1Jh=V38dRV059wdbacT9TkQ0oWGcqDfo5V4X2shvKykfA@mail.gmail.com>
References: <CA+TgmoYfagyp5AvKMfGpRUgXNJZ0ymuqiWTzOc2tV_ZMM0UkCA@mail.gmail.com>
<TYCPR01MB1207777150173002564A753B8F5502@TYCPR01MB12077.jpnprd01.prod.outlook.com>
<CA+TgmoaLRCcCeY3_n7h0rw-r-UfJt_GiFZiJQ1pFZ-iQNUozug@mail.gmail.com>
<[email protected]>
<CAA4eK1Jh=V38dRV059wdbacT9TkQ0oWGcqDfo5V4X2shvKykfA@mail.gmail.com>
Dear Amit,
> We do append dbname=replication even in libpqrcv_connect for .pgpass
> lookup as mentioned in comments. See below:
> libpqrcv_connect()
> {
> ....
> else
> {
> /*
> * The database name is ignored by the server in replication mode,
> * but specify "replication" for .pgpass lookup.
> */
> keys[++i] = "dbname";
> vals[i] = "replication";
> }
> ...
> }
OK. So we must add the value for the authorization, right?
I think it should be described even in GetConnection().
> I think as part of this effort we should just add dbname to
> primary_conninfo written in postgresql.auto.conf file. As above, the
> question is valid whether we should do it just for 17 or for prior
> versions. Let's discuss more on that. I am not sure of the use case
> for versions before 17 but commit cca97ce6a665 mentioned that some
> middleware or proxies might however need to know the dbname to make
> the correct routing decision for the connection. Does that apply here
> as well? If so, we can do it and update the docs, otherwise, let's do
> it just for 17.
Hmm, I might lose your requirements again. So, we must satisfy all of below
ones, right?
1) add {"dbname", "replication"} key-value pair to look up .pgpass file correctly.
2) If the -R is given, output dbname=xxx value to be used by slotsync worker.
3) If the dbname is not given in the connection string, the same string as
username must be used to keep the libpq connection rule.
4) No need to add dbname=replication pair
PSA the patch for implementing it. It is basically same as Ian's one.
However, this patch still cannot satisfy the condition 3).
`pg_basebackup -D data_N2 -d "user=postgres" -R`
-> dbname would not be appeared in primary_conninfo.
This is because `if (connection_string)` case in GetConnection() explicy override
a dbname to "replication". I've tried to add a dummy entry {"dbname", NULL} pair
before the overriding, but it is no-op. Because The replacement of the dbname in
pqConnectOptions2() would be done only for the valid (=lastly specified)
connection options.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
Attachments:
[application/octet-stream] pg_basebackup-write-dbname.v0002.patch (1.6K, ../TYCPR01MB1207779B65625AAEC975B5EC0F5592@TYCPR01MB12077.jpnprd01.prod.outlook.com/2-pg_basebackup-write-dbname.v0002.patch)
download | inline diff:
diff --git a/src/fe_utils/recovery_gen.c b/src/fe_utils/recovery_gen.c
index 2585f11939..c3eb1a59a0 100644
--- a/src/fe_utils/recovery_gen.c
+++ b/src/fe_utils/recovery_gen.c
@@ -49,12 +49,20 @@ GenerateRecoveryConfig(PGconn *pgconn, char *replication_slot)
{
/* Omit empty settings and those libpqwalreceiver overrides. */
if (strcmp(opt->keyword, "replication") == 0 ||
- strcmp(opt->keyword, "dbname") == 0 ||
strcmp(opt->keyword, "fallback_application_name") == 0 ||
(opt->val == NULL) ||
(opt->val != NULL && opt->val[0] == '\0'))
continue;
+ /*
+ * Omit "dbname" if the special value "replication" is specified, or
+ * the server version is 16 and earlier.
+ */
+ if (strcmp(opt->keyword, "dbname") == 0 &&
+ (strcmp(opt->val, "replication") == 0 ||
+ PQserverVersion(pgconn) < MINIMUM_VERSION_FOR_DBNAME_PARAM))
+ continue;
+
/* Separate key-value pairs with spaces */
if (conninfo_buf.len != 0)
appendPQExpBufferChar(&conninfo_buf, ' ');
diff --git a/src/include/fe_utils/recovery_gen.h b/src/include/fe_utils/recovery_gen.h
index ca2c4800d0..740d90c9d8 100644
--- a/src/include/fe_utils/recovery_gen.h
+++ b/src/include/fe_utils/recovery_gen.h
@@ -20,6 +20,12 @@
*/
#define MINIMUM_VERSION_FOR_RECOVERY_GUC 120000
+/*
+ * from version 17, there is a use-case for adding the dbname parameter
+ * to the generated "primary_conninfo" value
+ */
+#define MINIMUM_VERSION_FOR_DBNAME_PARAM 170000
+
extern PQExpBuffer GenerateRecoveryConfig(PGconn *pgconn,
char *replication_slot);
extern void WriteRecoveryConfig(PGconn *pgconn, char *target_dir,
view thread (13+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: RE: Have pg_basebackup write "dbname" in "primary_conninfo"?
In-Reply-To: <TYCPR01MB1207779B65625AAEC975B5EC0F5592@TYCPR01MB12077.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox