postgrespro / pg_probackup

Backup and recovery manager for PostgreSQL
https://postgrespro.github.io/pg_probackup/
Other
718 stars 86 forks source link

Why not use dbname parameter in pgut_connection_replication()? #634

Open japinli opened 3 months ago

japinli commented 3 months ago

Hi,

I find that the pgut_connection_replication() function passes the name parameter, which is only used in elog(). The connection string uses replication as the connection database.

It seems the elog's message is incorrect if the connection is failed. Why not use dbname as the connection database?

fukanchik commented 3 months ago

Physical replication connection connects to the whole server, not a database. Therefore server ignores it: https://github.com/postgres/postgres/blob/REL_16_4/src/backend/postmaster/postmaster.c#L2318

This code was based on pg_basebackup's streamutil where back then it did exactly the same: https://github.com/postgres/postgres/blob/REL9_3_STABLE/src/bin/pg_basebackup/streamutil.c#L94

japinli commented 3 months ago

@fukanchik Thank you for the explanation. IIUC, the pgut_connect_replication() is only used for physical replication, so the dbname has no means, maybe we can remove this parameter, and modify the elog() as following:

 elog(strict ? ERROR : WARNING, "could not connect to server: %s",  PQerrorMessage(tmpconn));