public inbox for [email protected]  

help / color / mirror / Atom feed

From: Hayato Kuroda (Fujitsu) <[email protected]>
To: 'Katsuragi Yuta' <[email protected]>
Cc: 'Ted Yu' <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: vignesh C <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Önder Kalacı <[email protected]>
Cc: Fujii Masao <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Subject: RE: [Proposal] Add foreign-server health checks infrastructure
Date: Wed, 22 Feb 2023 12:34:24 +0000
Message-ID: <TYAPR01MB5866904CE4F2C30E1CEEFC04F5AA9@TYAPR01MB5866.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <[email protected]>
References: <TYAPR01MB58669EAAC02493BFF9F39B06F5D99@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TYAPR01MB58669C95604A0EB7BCC1B58CF5A49@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<[email protected]>

Dear Katsuragi-san,

Thank you for reviewing! New patch set can be available on [1].

> I rethought the pqSocketPoll part. Current interpretation of
> arguments seems a little bit confusing because a specific pattern
> of arguments has a different meaning. What do you think about
> introducing a new argument like `int forConnCheck`? This seems
> straightforward and readable.

I think it may be better, so fixed.
But now we must consider another thing - will we support combination of conncheck
and {read|write} or timeout? Especially about timeout, if someone calls pqSocketPoll()
with forConnCheck = 1 and end_time = -1, the process may be stuck because it waits
till socket peer closed connection.
Currently the forConnCheck can be specified with other request, but timeout must be zero.

> I think there are cases where the given server name does exist,
> however the connection check is not performed (does not pass the
> first if statement above). 1) a case where the server name exist,
> however an open connection to the specified server is not found.
> 2) case where connection for specified server is invalidated.
> 3) case where there is not ConnectionHash entry for the specified
> server. Current implementation returns true in that case, however
> the check is not performed. Suppose the checking mechanism is
> supported on the platform, it does not seem reasonable to return
> true or false when the check is not performed. What do you think?

Thank you for detailed explanation. I agreed your opinion and modified like that.

While making the patch, I come up with idea that postgres_fdw_verify_connection_states*
returns NULL if PQconnCheck() cannot work on this platform. This can be done by
adding PQconnCheckable(). It may be reasonable because the checking is not really
done on this environment. How do you think?

[1]: https://www.postgresql.org/message-id/TYAPR01MB586664F5ED2128E572EA95B0F5AA9%40TYAPR01MB5866.jpnprd0...

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

view thread (6+ messages)

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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: RE: [Proposal] Add foreign-server health checks infrastructure
  In-Reply-To: <TYAPR01MB5866904CE4F2C30E1CEEFC04F5AA9@TYAPR01MB5866.jpnprd01.prod.outlook.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox