MIME-Version: 1.0
References: <CA+TgmoYfagyp5AvKMfGpRUgXNJZ0ymuqiWTzOc2tV_ZMM0UkCA@mail.gmail.com>
 <TYCPR01MB1207777150173002564A753B8F5502@TYCPR01MB12077.jpnprd01.prod.outlook.com>
 <CA+TgmoaLRCcCeY3_n7h0rw-r-UfJt_GiFZiJQ1pFZ-iQNUozug@mail.gmail.com>
 <20240221.120424.79274783106518358.horikyota.ntt@gmail.com>
 <CAA4eK1Jh=V38dRV059wdbacT9TkQ0oWGcqDfo5V4X2shvKykfA@mail.gmail.com>
 <TYCPR01MB1207779B65625AAEC975B5EC0F5592@TYCPR01MB12077.jpnprd01.prod.outlook.com>
 <TYCPR01MB12077B81FCAFDFE104DEEE7BAF5592@TYCPR01MB12077.jpnprd01.prod.outlook.com>
 <CAA4eK1L+39988Rzq4hD0h1y+s0234PH3G-u7NbUmfwE438mbGQ@mail.gmail.com>
 <CALDaNm0tr-t_L+anY1+qseAajpdE1H-cFQzcPzXoRJWUjvOdXg@mail.gmail.com> <CAA4eK1+bbMg+Zbp=r6yCSqb3EPHcyeg-0gHkdUWD_w-d5+2sGw@mail.gmail.com>
In-Reply-To: <CAA4eK1+bbMg+Zbp=r6yCSqb3EPHcyeg-0gHkdUWD_w-d5+2sGw@mail.gmail.com>
From: vignesh C <vignesh21@gmail.com>
Date: Wed, 13 Mar 2024 21:34:20 +0530
Message-ID: <CALDaNm2Z827GyPTM8uj=Y6JcbtmDKVBa73XOK+2ztTkETq=75A@mail.gmail.com>
Subject: Re: Have pg_basebackup write "dbname" in "primary_conninfo"?
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, 
	"robertmhaas@gmail.com" <robertmhaas@gmail.com>, "barwick@gmail.com" <barwick@gmail.com>, 
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Content-Type: multipart/mixed; boundary="00000000000079ad3806138cee9f"
Archived-At: <https://www.postgresql.org/message-id/CALDaNm2Z827GyPTM8uj%3DY6JcbtmDKVBa73XOK%2B2ztTkETq%3D75A%40mail.gmail.com>
Precedence: bulk

--00000000000079ad3806138cee9f
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, 13 Mar 2024 at 16:58, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, Mar 12, 2024 at 5:13=E2=80=AFPM vignesh C <vignesh21@gmail.com> w=
rote:
> >
> > On Mon, 11 Mar 2024 at 17:16, Amit Kapila <amit.kapila16@gmail.com> wro=
te:
> > >
> > > On Tue, Feb 27, 2024 at 2:07=E2=80=AFPM Hayato Kuroda (Fujitsu)
> > > <kuroda.hayato@fujitsu.com> 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=3Dpostgres" -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 {"dbna=
me", NULL} pair
> > > > > before the overriding, but it is no-op. Because The replacement o=
f the dbname in
> > > > > pqConnectOptions2() would be done only for the valid (=3Dlastly s=
pecified)
> > > > > connection options.
> > > >
> > > > Oh, this patch missed the straightforward case:
> > > >
> > > > pg_basebackup -D data_N2 -d "user=3Dpostgres dbname=3Dreplication" =
-R
> > > > -> dbname would not be appeared in primary_conninfo.
> > > >
> > > > So I think it cannot be applied as-is. Sorry for sharing the bad it=
em.
> > > >
> > >
> > > 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 =3D "dbname=3Dvignesh" (In this case primary_connin=
fo
> > will have dbname as username specified)
> > 2) ./pg_basebackup -D data -R
> > -> primary_conninfo =3D "dbname=3Dvignesh" (In this case primary_connin=
fo
> > will have the dbname as username (which is the same as the OS user
> > while setting defaults))
> > 3) ./pg_basebackup -d "user=3Dvignesh"  -D data -R
> > -> primary_conninfo =3D "dbname=3Dreplication"  (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=3Dvignesh dbname=3Dpostgres"  -D data -R
> > -> primary_conninfo =3D "dbname=3Dpostgres" (In this case primary_conni=
nfo
> > will have the dbname as the dbname specified)
> > 5) ./pg_basebackup -d ""  -D data -R
> > -> primary_conninfo =3D "dbname=3Dreplication" (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.
> >
>
> IIUC, the dbname is already allowed in connstring for pg_basebackup by
> commit cca97ce6a6 and with this patch we are just writing it in
> postgresql.auto.conf if -R option is used to write recovery info. This
> will help slot syncworker to automatically connect with database
> without user manually specifying the dbname and replication
> connection, the same will still be ignored. I don't see any problem
> with this. Does anyone else see any problem?
>
> The <filename>postgresql.auto.conf</filename> file will record the connec=
tion
>          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 t=
o the
> +        hot_standby from <link linkend=3D"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=
.
>
> We can simply modify the last line as: ".. streaming replication or
> logical replication slots synchronization will use the same settings
> later on." Additionally, you can give a link reference to [1].

Thanks for the comments, the attached v4 version patch has the changes
for the same.

Regards,
Vignesh

--00000000000079ad3806138cee9f
Content-Type: text/x-patch; charset="US-ASCII"; name="pg_basebackup-write-dbname.v0004.patch"
Content-Disposition: attachment; 
	filename="pg_basebackup-write-dbname.v0004.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_ltpzrhk50>
X-Attachment-Id: f_ltpzrhk50

ZGlmZiAtLWdpdCBhL2RvYy9zcmMvc2dtbC9yZWYvcGdfYmFzZWJhY2t1cC5zZ21sIGIvZG9jL3Ny
Yy9zZ21sL3JlZi9wZ19iYXNlYmFja3VwLnNnbWwKaW5kZXggODhjNjg5ZTcyNS4uNTYzMzQ2ZGQ3
MSAxMDA2NDQKLS0tIGEvZG9jL3NyYy9zZ21sL3JlZi9wZ19iYXNlYmFja3VwLnNnbWwKKysrIGIv
ZG9jL3NyYy9zZ21sL3JlZi9wZ19iYXNlYmFja3VwLnNnbWwKQEAgLTI0Myw3ICsyNDMsOSBAQCBQ
b3N0Z3JlU1FMIGRvY3VtZW50YXRpb24KICAgICAgICAgVGhlIDxmaWxlbmFtZT5wb3N0Z3Jlc3Fs
LmF1dG8uY29uZjwvZmlsZW5hbWU+IGZpbGUgd2lsbCByZWNvcmQgdGhlIGNvbm5lY3Rpb24KICAg
ICAgICAgc2V0dGluZ3MgYW5kLCBpZiBzcGVjaWZpZWQsIHRoZSByZXBsaWNhdGlvbiBzbG90CiAg
ICAgICAgIHRoYXQgPGFwcGxpY2F0aW9uPnBnX2Jhc2ViYWNrdXA8L2FwcGxpY2F0aW9uPiBpcyB1
c2luZywgc28gdGhhdAotICAgICAgICBzdHJlYW1pbmcgcmVwbGljYXRpb24gd2lsbCB1c2UgdGhl
IHNhbWUgc2V0dGluZ3MgbGF0ZXIgb24uCisgICAgICAgIHN0cmVhbWluZyByZXBsaWNhdGlvbiBv
ciA8bGluayBsaW5rZW5kPSJsb2dpY2FsZGVjb2RpbmctcmVwbGljYXRpb24tc2xvdHMtc3luY2hy
b25pemF0aW9uIj4KKyAgICAgICAgbG9naWNhbCByZXBsaWNhdGlvbiBzbG90cyBzeW5jaHJvbml6
YXRpb248L2xpbms+IHdpbGwgdXNlIHRoZSBzYW1lCisgICAgICAgIHNldHRpbmdzIGxhdGVyIG9u
LgogICAgICAgIDwvcGFyYT4KIAogICAgICAgPC9saXN0aXRlbT4KZGlmZiAtLWdpdCBhL3NyYy9m
ZV91dGlscy9yZWNvdmVyeV9nZW4uYyBiL3NyYy9mZV91dGlscy9yZWNvdmVyeV9nZW4uYwppbmRl
eCAyNTg1ZjExOTM5Li5jYjM3NzAzYWI5IDEwMDY0NAotLS0gYS9zcmMvZmVfdXRpbHMvcmVjb3Zl
cnlfZ2VuLmMKKysrIGIvc3JjL2ZlX3V0aWxzL3JlY292ZXJ5X2dlbi5jCkBAIC00OSw3ICs0OSw2
IEBAIEdlbmVyYXRlUmVjb3ZlcnlDb25maWcoUEdjb25uICpwZ2Nvbm4sIGNoYXIgKnJlcGxpY2F0
aW9uX3Nsb3QpCiAJewogCQkvKiBPbWl0IGVtcHR5IHNldHRpbmdzIGFuZCB0aG9zZSBsaWJwcXdh
bHJlY2VpdmVyIG92ZXJyaWRlcy4gKi8KIAkJaWYgKHN0cmNtcChvcHQtPmtleXdvcmQsICJyZXBs
aWNhdGlvbiIpID09IDAgfHwKLQkJCXN0cmNtcChvcHQtPmtleXdvcmQsICJkYm5hbWUiKSA9PSAw
IHx8CiAJCQlzdHJjbXAob3B0LT5rZXl3b3JkLCAiZmFsbGJhY2tfYXBwbGljYXRpb25fbmFtZSIp
ID09IDAgfHwKIAkJCShvcHQtPnZhbCA9PSBOVUxMKSB8fAogCQkJKG9wdC0+dmFsICE9IE5VTEwg
JiYgb3B0LT52YWxbMF0gPT0gJ1wwJykpCg==
--00000000000079ad3806138cee9f--