Dear Amit, > On Fri, Jan 5, 2024 at 3:36 PM Hayato Kuroda (Fujitsu) > <kuroda.hay...@fujitsu.com> wrote: > > > > I love your proposal, so I want to join the review. Here are my first > > comments. > > > > 01. > > Should we restrict that `--subscriber-conninfo` must not have hostname or > > IP? > > We want users to execute pg_subscriber on the target, right? > > > > I don't see any harm in users giving those information but we should > have some checks to ensure that the server is in standby mode and is > running locally. The other related point is do we need to take input > for the target cluster directory from the user? Can't we fetch that > information once we are connected to standby?
I think that functions like inet_client_addr() may be able to use, but it returns NULL only when the connection is via a Unix-domain socket. Can we restrict pg_subscriber to use such a socket? > > > > 05. > > I found that the connection string for each subscriptions have a setting > > "fallback_application_name=pg_subscriber". Can we remove it? > > > > ``` > > postgres=# SELECT subconninfo FROM pg_subscription; > > subconninfo > > > --------------------------------------------------------------------------------- > > user=postgres port=5431 fallback_application_name=pg_subscriber > dbname=postgres > > (1 row) > > ``` > > Can that help distinguish the pg_subscriber connection on the publisher? > Note that this connection string is used between the publisher instance and the subscriber instance (not pg_subscriber client application). Also, the fallback_application_name would be replaced to the name of subscriber in run_apply_worker()->walrcv_connect(). Actually the value would not be used. See below output on publisher. ``` publisher=# SELECT application_name, backend_type FROM pg_stat_activity where backend_type = 'walsender'; application_name | backend_type ----------------------+-------------- pg_subscriber_5_9411 | walsender (1 row) ``` Or, if you mean to say that this can distinguish whether the subscription is used by pg_subscriber or not. I think it is sufficient the current format of name. Best Regards, Hayato Kuroda FUJITSU LIMITED
