pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed
From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
To: 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>
Subject: Re: [Proposal] Add foreign-server health checks infrastructure
Date: Tue, 4 Apr 2023 02:56:22 +0900
Message-ID: <b492a935-6c7e-8c08-e485-3c1d64d7d10f@oss.nttdata.com> (raw)
In-Reply-To: <TYAPR01MB5866DBFEC61E81CCE9F67142F5B99@TYAPR01MB5866.jpnprd01.prod.outlook.com>
References: <TYAPR01MB58669EAAC02493BFF9F39B06F5D99@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<f09b19e3bf464eeeae21464c2051024a@oss.nttdata.com>
	<TYAPR01MB5866904CE4F2C30E1CEEFC04F5AA9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<3cb43b39672c17786566063b0aa1225d@oss.nttdata.com>
	<TYAPR01MB586674A3C56AECB9B150BC84F5B39@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<f94c09551891249fe316bfe55015b3e7@oss.nttdata.com>
	<TYCPR01MB58700498838127C54BC9BD48F5B69@TYCPR01MB5870.jpnprd01.prod.outlook.com>
	<f723a26e8c34001ae32e3399951b8066@oss.nttdata.com>
	<TYAPR01MB5866F8EA3C06E1B43E42E4E4F5B79@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<CALDaNm2H9ZyiPAOJCKHo5dHYv-u6u+8bv_T9PkdrvqVHnxyrmQ@mail.gmail.com>
	<TYAPR01MB586622DBD4A0C3BA57136899F5B49@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<36981865ac13b6c40d53bf3f98667eee@oss.nttdata.com>
	<300fccae11de8a4ed1a2d970db1a8680@oss.nttdata.com>
	<TYAPR01MB5866DBFEC61E81CCE9F67142F5B99@TYAPR01MB5866.jpnprd01.prod.outlook.com>



On 2023/03/13 16:05, Hayato Kuroda (Fujitsu) wrote:
> Thank you so much for your reviewing!
> Now we can wait comments from senior members and committers.

Thanks for working on this patch!

Regarding 0001 patch, on second thought, to me, it seems odd to expose
a function that doesn't have anything to directly do with PostgreSQL
as a libpq function. The function simply calls poll() on the socket
with POLLRDHUP if it is supported. While it is certainly convenient to
have this function, I'm not sure that it fits within the scope of libpq.
Thought?

Regarding 0002 patch, the behavior of postgres_fdw_verify_connection_states()
in regards to multiple connections using different user mappings seems
not well documented. The function seems to return false if either of
those connections has been closed.

This behavior means that it's difficult to identify which connection
has been closed when there are multiple ones to the given server.
To make it easier to identify that, it could be helpful to extend
the postgres_fdw_verify_connection_states() function so that it accepts
a unique connection as an input instead of just the server name.
One suggestion is to extend the function so that it accepts
both the server name and the user name used for the connection,
and checks the specified connection. If only the server name is specified,
the function should check all connections to the server and return false
if any of them are closed. This would be helpful since there is typically
only one connection to the server in most cases.

Additionally, it would be helpful to extend the postgres_fdw_get_connections()
function to also return the user name used for each connection,
as currently there is no straightforward way to identify it.

The function name "postgres_fdw_verify_connection_states" may seem
unnecessarily long to me. A simpler name like
"postgres_fdw_verify_connection" may be enough?

The patch may not be ready for commit due to the review comments,
and with the feature freeze approaching in a few days,
it may not be possible to include this feature in v16.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION





view thread (129+ messages)  latest in thread

Message-ID: <b492a935-6c7e-8c08-e485-3c1d64d7d10f@oss.nttdata.com>
Permalink:  ../b492a935-6c7e-8c08-e485-3c1d64d7d10f@oss.nttdata.com/
Also on:    postgresql.org/message-id/b492a935-6c7e-8c08-e485-3c1d64d7d10f@oss.nttdata.com

 · 

reply
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
  Subject: Re: [Proposal] Add foreign-server health checks infrastructure
  In-Reply-To: <b492a935-6c7e-8c08-e485-3c1d64d7d10f@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