RE: [Proposal] Add foreign-server health checks infrastructure - Mailing list pgsql-hackers

From Hayato Kuroda (Fujitsu)
Subject RE: [Proposal] Add foreign-server health checks infrastructure
Date
Msg-id TYCPR01MB56934DE73B0AF7A37D8CF7B2F5B42@TYCPR01MB5693.jpnprd01.prod.outlook.com
Whole thread Raw
In response to RE: [Proposal] Add foreign-server health checks infrastructure  ("Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>)
Responses Re: [Proposal] Add foreign-server health checks infrastructure
List 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


pgsql-hackers by date:

Previous
From: Peter Smith
Date:
Subject: Re: Logical Replication of sequences
Next
From: Yugo Nagata
Date:
Subject: Re: MAINTAIN privilege -- what do we need to un-revert it?