Re: Have pg_basebackup write "dbname" in "primary_conninfo"?

On Thu, Mar 14, 2024 at 2:27 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Mar 14, 2024 at 5:57 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > This fact makes me think that the slotsync worker might be able to
> > accept the primary_conninfo value even if there is no dbname in the
> > value. That is, if there is no dbname in the primary_conninfo, it uses
> > the username in accordance with the specs of the connection string.
> > Currently, the slotsync worker connects to the local database first
> > and then establishes the connection to the primary server. But if we
> > can reverse the two steps, it can get the dbname that has actually
> > been used to establish the remote connection and use it for the local
> > connection too. That way, the primary_conninfo generated by
> > pg_basebackup could work even without the patch. For example, if the
> > OS user executing pg_basebackup is 'postgres', the slotsync worker
> > would connect to the postgres database. Given the 'postgres' database
> > is created by default and 'postgres' OS user is used in common, I
> > guess it could cover many cases in practice actually.
> >
>
> I think this is worth investigating but I suspect that in most cases
> users will end up using a replication connection without specifying
> the user name and we may not be able to give a meaningful error
> message when slotsync worker won't be able to connect. The same will
> be true even when the dbname same as the username would be used.

What do you mean by not being able to give a meaningful error message?

If the slotsync worker uses the user name as the dbname, and such a
database doesn't exist, the error message the user will get is
"database "test_user" does not exist". ISTM the same is true when the
user specifies the wrong database in the primary_conninfo.

>
> > Having said that, even with (or without) the above change, we might
> > want to change the pg_basebackup so that it writes the dbname to the
> > primary_conninfo if -R option is specified. Since the database where
> > the slotsync worker connects cannot be dropped while the slotsync
> > worker is running, the user might want to change the database to
> > connect, and it would be useful if they can do that using
> > pg_basebackup instead of modifying the configuration file manually.
> >
> > While the current approach makes sense to me, I'm a bit concerned that
> > we might end up having the pg_basebackup search the actual database
> > name (e.g. 'dbname=template1') from the .pgpass file instead of
> > 'dbname=replication'. As far as I tested on my environment, suppose
> > that I execute:
> >
> > pg_basebackup -D tmp -d "dbname=testdb" -R
> >
> > The pg_basebackup established a replication connection but looked for
> > the password of the 'testdb' database. This could be another
> > inconvenience for the existing users who want to use the slot
> > synchronization.
> >
>
> This is true because it is internally using logical replication
> connection (as we will set set replication=database).

Did you mean the pg_basebackup is using a logical replication
connection in this case? As far as I tested, even if we specify dbname
to the -d option of pg_basebackup, it uses a physical replication
connection. For example, it can take a backup even if I specify a
non-existing database name.

> > A random idea I came up with is, we add a new option to the
> > pg_basebackup to overwrite the full or some portion of the connection
> > string that is eventually written in the primary_conninfo in
> > postgresql.auto.conf. For example, the command:
> >
> > pg_basebackup -D tmp -d "host=1.1.1.1 port=5555" -R
> > --primary-coninfo-ext "host=2.2.2.2 dbname=postgres"
> >
> > will produce the connection string that is based on -d option value
> > but is overwritten by --primary-conninfo-ext option value, which will
> > be like:
> >
> > host=2.2.2.2 dbname=postgres port=5555
> >
> > This option might help not only for users who want to use the slotsync
> > worker but also for users who want to take a basebackup from a standby
> > but have the new standby connect to the primary.
> >
>
> Agreed, this could be another way though it would be good to get some
> inputs from users or otherwise about the preferred way to specify
> dbname. One can also imagine using the Alter System for this purpose.

Agreed.

>
> > But it's still just an idea and I might be missing something. And
> > given we're getting closer to the feature freeze, it would be a PG18
> > item.
> >
>
> +1. At this stage, it is important to discuss whether we should allow
> pg_baseback to write dbname (either a specified one or a default one)
> along with other parameters in primary_conninfo?
>

True. While I basically agree that pg_basebackup writes dbname in
primary_conninfo, I'm concerned that writing "dbname=replication"
could be problematic. Quoting the case 3) Vignesh summarized before:

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)

The primary_conninfo generated by pg_basebackup -R is now used by
either a walreceiver (for physical replication connection) or a
slotsync worker (for normal connection). The "dbname=replication" is
okay  for walreceiver. On the other hand, as for the slotsync worker,
it can pass the CheckAndGetDbnameFromConninfo() check but it's very
likely that it cannot connect to the primary since most users won't
create a database with "replication" name. The user will end up
getting an error message like 'database "replication" does not exist'
but I'm not sure it would be informative for users. Rather, the error
message "slot synchronization requires dbname to be specified in
primary_conninfo" might be more informative for users. So I personally
like to omit the dbname if "dbname=replication", at this point.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com