RE: [Proposal] Add foreign-server health checks infrastructure
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Fujii Masao' <masao.fujii@oss.nttdata.com>
Cc: 'Katsuragi Yuta' <katsuragiy@oss.nttdata.com>, 'vignesh C' <vignesh21@gmail.com>, 'Ted Yu' <yuzhihong@gmail.com>, 'Tom Lane' <tgl@sss.pgh.pa.us>, "'pgsql-hackers@lists.postgresql.org'" <pgsql-hackers@lists.postgresql.org>, 'Önder Kalacı' <onderkalaci@gmail.com>, "'Shinya11.Kato@oss.nttdata.com'" <Shinya11.Kato@oss.nttdata.com>, 'Kyotaro Horiguchi' <horikyota.ntt@gmail.com>, 'Shubham Khanna' <khannashubham1197@gmail.com>
Date: 2024-07-26T03:15:23Z
Lists: pgsql-hackers
I apologize to post incomplete message, here is a correct version.
Dear Fujii-san,
> Thanks for updating the patches!
>
> I’ve created a new base patch and split the v42-0001 patch into two parts
> to implement the feature and improvements step by step. After pushing these
> patches, I’ll focus on the v42-0002 patch next.
+1.
> I’ve attached the three patches.
>
> v43-0001:
> This new patch enhances documentation for postgres_fdw_get_connections()
> output columns. The output columns were documented in text format,
> which is manageable for the current two columns. However, upcoming patches
> will add new columns, making text descriptions less readable.
> This patch updates the documentation to use a table format,
> making it easier for users to understand each output column.
Agreed to add the table. I ran a proofreading tool, and it said below points.
You can revise if they are acceptable.
```
+ This function returns information about all open connections that
```
-> "that" can be removed.
```
+ the current transaction but its foreign server or
```
-> can add comma before "but".
> I’ve also made several code improvements, for example adding a typedef for
> pgfdwVersion to simplify its usage, and updated typedefs.list.
>
> +enum pgfdwVersion
> +{
> + PGFDW_V1_0 = 0,
> + PGFDW_V1_2,
> +} pgfdwVersion;
It was intentionally not added, because while developing pg_createsubscriber,
I got a comment that local-use data have not have to be typedef'd [1]:
```
- src/bin/pg_basebackup/pg_createsubscriber.c
+typedef struct CreateSubscriberOptions
+typedef struct LogicalRepInfo
I think these kinds of local-use struct don't need to be typedef'ed.
(Then you also don't need to update typdefs.list.)
```
A comment for 0002.
```
+Datum
+postgres_fdw_get_connections_1_2(PG_FUNCTION_ARGS)
+{
+ postgres_fdw_get_connections_internal(fcinfo, PGFDW_V1_2);
+
+ return (Datum) 0;
+}
```
I know they have a same meaning, but can you clarify why (Datun) 0
is returned instead of PG_RETURN_VOID()?
[1]: https://www.postgresql.org/message-id/3ee79f2c-e8b3-4342-857c-a31b87e1afda%40eisentraut.org
Best regards,
Hayato Kuroda
FUJITSU LIMITED
Commits
-
postgres_fdw: Extend postgres_fdw_get_connections to return user name.
- 4f08ab554577 18.0 landed
-
postgres_fdw: Fix bug in connection status check.
- 454aab4b738e 18.0 landed
-
postgres_fdw: Add connection status check to postgres_fdw_get_connections().
- 857df3cef7be 18.0 landed
-
postgres_fdw: Add "used_in_xact" column to postgres_fdw_get_connections().
- c297a47c5f8d 18.0 landed
-
doc: Enhance documentation for postgres_fdw_get_connections() output columns.
- 284c030a10b8 18.0 landed
-
Avoid reference to nonexistent array element in ExecInitAgg().
- 92957ed98c5c 16.0 cited
-
Add WL_SOCKET_CLOSED for socket shutdown events.
- 50e570a59e7f 15.0 cited
-
postgres_fdw: reestablish new connection if cached one is detected as broken.
- 32a9c0bdf493 14.0 cited