On Mon, 11 Mar 2024 at 17:16, Amit Kapila <[email protected]> wrote:
>
> On Tue, Feb 27, 2024 at 2:07 PM Hayato Kuroda (Fujitsu)
> <[email protected]> wrote:
> >
> > > 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.
> >
> > Oh, this patch missed the straightforward case:
> >
> > pg_basebackup -D data_N2 -d "user=postgres dbname=replication" -R
> > -> dbname would not be appeared in primary_conninfo.
> >
> > So I think it cannot be applied as-is. Sorry for sharing the bad item.
> >
>
> Can you please share the patch that can be considered for review?
Here is a patch with few changes: a) Removed the version check based
on discussion from [1] b) updated the documentation.
I have tested various scenarios with the attached patch, the dbname
that is written in postgresql.auto.conf for each of the cases with
pg_basebackup is given below:
1) ./pg_basebackup -U vignesh -R
-> primary_conninfo = "dbname=vignesh" (In this case primary_conninfo
will have dbname as username specified)
2) ./pg_basebackup -D data -R
-> primary_conninfo = "dbname=vignesh" (In this case primary_conninfo
will have the dbname as username (which is the same as the OS user
while setting defaults))
3) ./pg_basebackup -d "user=vignesh" -D data -R
-> primary_conninfo = "dbname=replication" (In this case
primary_conninfo will have dbname as replication which is the default
value from GetConnection as connection string is specified)
4) ./pg_basebackup -d "user=vignesh dbname=postgres" -D data -R
-> primary_conninfo = "dbname=postgres" (In this case primary_conninfo
will have the dbname as the dbname specified)
5) ./pg_basebackup -d "" -D data -R
-> primary_conninfo = "dbname=replication" (In this case
primary_conninfo will have dbname as replication which is the default
value from GetConnection as connection string is specified)
I have mentioned the reasons as to why dbname is being set to
replication or username in each of the cases. How about replacing
these values in postgresql.auto.conf manually.
[1] -
https://www.postgresql.org/message-id/CAGECzQTh9oB3nu98DsHMpRaVaqXPDRgTDEikY82OAKYF0%3DhVMA%40mail.gmail.com
Regards,
Vignesh
diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index 88c689e725..1a6980d1b0 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -243,7 +243,10 @@ PostgreSQL documentation
The <filename>postgresql.auto.conf</filename> file will record the connection
settings and, if specified, the replication slot
that <application>pg_basebackup</application> is using, so that
- streaming replication will use the same settings later on.
+ a) synchronization of logical replication slots on the primary to the
+ hot_standby from <link linkend="pg-sync-replication-slots">
+ <function>pg_sync_replication_slots</function></link> or slot sync worker
+ and b) streaming replication will use the same settings later on.
</para>
</listitem>
diff --git a/src/fe_utils/recovery_gen.c b/src/fe_utils/recovery_gen.c
index 2585f11939..cb37703ab9 100644
--- a/src/fe_utils/recovery_gen.c
+++ b/src/fe_utils/recovery_gen.c
@@ -49,7 +49,6 @@ 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'))
