From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
Cc: Katsuragi Yuta <katsuragiy@oss.nttdata.com>
Cc: vignesh C <vignesh21@gmail.com>
Cc: Ted Yu <yuzhihong@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>
Cc: 'Önder Kalacı' <onderkalaci@gmail.com>
Cc: 'Shinya11.Kato@oss.nttdata.com' <Shinya11.Kato@oss.nttdata.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: Shubham Khanna <khannashubham1197@gmail.com>
Subject: Re: [Proposal] Add foreign-server health checks infrastructure
Date: Thu, 8 Aug 2024 01:20:57 +0900
Message-ID: <b0d192d1-51ec-40f5-acde-0230ef2cc885@oss.nttdata.com> (raw)
In-Reply-To: <TYAPR01MB5692DA275D54AC6653604C6BF5B32@TYAPR01MB5692.jpnprd01.prod.outlook.com>
References: <TYAPR01MB58669EAAC02493BFF9F39B06F5D99@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<be9382f7-5072-4760-8b3f-31d6dffa8d62@oss.nttdata.com>
<TYAPR01MB56923ACAE33FD04A3560F413F5AC2@TYAPR01MB5692.jpnprd01.prod.outlook.com>
<613adbf9-c86e-41dc-ab97-27dc4e7281c7@oss.nttdata.com>
<TYCPR01MB56933FCF37A676B868362575F5AA2@TYCPR01MB5693.jpnprd01.prod.outlook.com>
<d04aae8d-05f5-42f4-a263-b962334d9f75@oss.nttdata.com>
<TYCPR01MB569331643314B5129312C332F5B42@TYCPR01MB5693.jpnprd01.prod.outlook.com>
<TYCPR01MB56934DE73B0AF7A37D8CF7B2F5B42@TYCPR01MB5693.jpnprd01.prod.outlook.com>
<768032ee-fb57-494b-b674-1ccb65b6f969@oss.nttdata.com>
<TYCPR01MB5693504C160299B2031FA6FFF5B42@TYCPR01MB5693.jpnprd01.prod.outlook.com>
<1aa89c91-e3f1-4071-9fa9-14bffa600dd2@oss.nttdata.com>
<TYAPR01MB5692411E16AB98D8CF72C2E7F5B72@TYAPR01MB5692.jpnprd01.prod.outlook.com>
<4ab30d00-d434-4e82-af5d-260609075ff7@oss.nttdata.com>
<TYAPR01MB5692DA275D54AC6653604C6BF5B32@TYAPR01MB5692.jpnprd01.prod.outlook.com>
On 2024/08/02 14:56, Hayato Kuroda (Fujitsu) wrote:
> I moved the function to connection.c, which uses the SearchSysCache1().
>
> I've tried both ways, and they worked well. One difference is that when we use
> the extended ConnCacheEntry approach and the entry has been invalidated, we cannot
> distinguish the reason. For example, in the below case, the entry is invalidated,
> so the user_name of the output record will be NULL, whereas the user mapping is
> actually still valid. We may be able to add the reason for invalidation, but
> I'm not very motivated to modify the part.
Understood. Also thanks for updating the patch!
<term><function>postgres_fdw_get_connections(
IN check_conn boolean DEFAULT false, OUT server_name text,
OUT valid boolean, OUT used_in_xact boolean, OUT closed boolean)
returns setof record</function></term>
In the documentation, this part should be updated to include the user_name output column.
+ <entry><structfield>user_name</structfield></entry>
+ <entry><type>text</type></entry>
+ <entry>
+ The local user name of this connection. If the user mapping is
+ dropped but the connection remains open (i.e., marked as
+ invalid), this will be <literal>NULL</literal>.
How about changing the first description to "Name of the local user mapped to the foreign server of this connection, or "public" if a public mapping is used." for more precision?
- server_name | valid | used_in_xact | closed
--------------+-------+--------------+--------
- loopback1 | t | t |
- loopback2 | f | t |
+ server_name | user_name | valid | used_in_xact | closed
+-------------+-----------+-------+--------------+--------
+ loopback1 | postgres | t | t |
+ loopback2 | postgres | t | t |
How about displaying the record with loopback2 and valid=false like the previous usage example?
+UserMapping *
+GetUserMappingByOid(Oid umid, bool missing_ok)
postgres_fdw doesn't need a generic function to return UserMapping. How about simplifying the function by removing unnecessary code, e.g., as follows?
----------
tp = SearchSysCache1(USERMAPPINGOID, ObjectIdGetDatum(umid));
if (!HeapTupleIsValid(tp))
nulls[i++] = true;
else
{
Oid userid = ((Form_pg_user_mapping) GETSTRUCT(tp))->userid;
values[i++] = CStringGetTextDatum(MappingUserName(userid));
ReleaseSysCache(tp);
}
----------
-ForeignTable *
-GetForeignTable(Oid relid);
-</programlisting>
-
- This function returns a <structname>ForeignTable</structname> object for
- the foreign table with the given OID. A
- <structname>ForeignTable</structname> object contains properties of the
- foreign table (see <filename>foreign/foreign.h</filename> for details).
- </para>
-
- <para>
-<programlisting>
Why did you remove these code? Just mistake?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: masao.fujii@oss.nttdata.com, kuroda.hayato@fujitsu.com, katsuragiy@oss.nttdata.com, vignesh21@gmail.com, yuzhihong@gmail.com, tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org, onderkalaci@gmail.com, Shinya11.Kato@oss.nttdata.com, horikyota.ntt@gmail.com, khannashubham1197@gmail.com
Subject: Re: [Proposal] Add foreign-server health checks infrastructure
In-Reply-To: <b0d192d1-51ec-40f5-acde-0230ef2cc885@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox