public inbox for [email protected]  
help / color / mirror / Atom feed
From: Hayato Kuroda (Fujitsu) <[email protected]>
To: 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: Fri, 27 Jan 2023 03:50:18 +0000
Message-ID: <TYAPR01MB5866B55A252EF7005AA81D5BF5CC9@TYAPR01MB5866.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <TYAPR01MB5866969DBAEEEC5AE07591BFF5CE9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
References: <CACawEhW_R=6mKsB24QW3WpCZTQgtxAPH7J0q8yedKkCQY2xT0w@mail.gmail.com>
	<TYAPR01MB5866F419C4261177578AC1CCF55B9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<CACawEhXAy4gjJ4G3-59Xv_yb8Xbtn1VMYP8SOj-0C9dG2zbAcQ@mail.gmail.com>
	<TYAPR01MB5866ACB4D3B845B4B2ED121BF55A9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<CACawEhW19nPfbFpvfke9eidFDxAy+ic36wmY0s936T=xzxgHog@mail.gmail.com>
	<TYAPR01MB5866CFD6BAE6DDF01A27CBF1F5299@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<CACawEhUzpqYJ8mQmSjYgX0ePtPpvb2u9Onjf6pCjUGkoZ=-xSg@mail.gmail.com>
	<TYAPR01MB58668728393648C2F7DC7C85F5399@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<CACawEhW56PHQ83Q59x4U5zpi0rVJ6=0Vn-FeYSLZ13Y0yasebQ@mail.gmail.com>
	<TYAPR01MB5866AAA4EB0632B4001AA2C5F50E9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TYAPR01MB586635DB5F1C5CE4660175FEF51A9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<TYAPR01MB58660C8F72746C2A8C6381F2F5EA9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<TYAPR01MB586613F662443D5A3005CD17F5FF9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TYAPR01MB586612A29BAF468042F04B20F5FF9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<CALte62zppS-YxEhes5jSsHppQsRrgHBw-krViS1HJJPSsV9onw@mail.gmail.com>
	<TYAPR01MB5866413814861A5016839506F5FC9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<TYAPR01MB5866C20861167C9CBEDF0A36F5FC9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TYAPR01MB5866E17F6C47AB41DCB8ADBEF5CA9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<CALte62y83j9p8xOoOS=ftG8PZTwUCUBHAp=WiKM7i6NUon3cNg@mail.gmail.com>
	<TYAPR01MB58668DD17F19CAFC97A3134EF5C89@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TYAPR01MB5866969DBAEEEC5AE07591BFF5CE9@TYAPR01MB5866.jpnprd01.prod.outlook.com>

Dear hackers,

I have updated my patch for error handling and kqueue() support.
Actually I do not have BSD-like machine, but I developed by using github CICD.
I think at first we should focus on 0001-0003, and then work for 0004.

Followings are change notes and my analysis.

0001

* Fix missed replacements from PQConnCheck() to PQconnCheck().
* Error handling was improved. Now we can detect the failure of poll() and return -1 at that time.
* I thought we don't have to add select(2) in PQconnCheck(). According to man page [1],
  select(2) can be only used for watch whether the status is readable, writable, or exceptional condition.
  It means that select() does not have an event corresponding to POLLRDHUP.

0002, 0003
Not changed

0004

* Add kqueue(2) support() for BSD family.
* I did not add epoll() support, because it can be used only on linux and such systems have POLLRDHUP instead. 
 checked other codes in libpq, and they do not use epoll(). We can see that such an event does not specified in POSIX [2]
 and it can be used for linux only. I decided to use poll() as much as possible to keep the policy.
* While coding, I found that there are no good timing to close the kernel event queue.
 It means that the lifetime of kqueue becomes same as the client program and occupy the small memory forever.
 I'm not sure it can be accepted.


[1]: https://man7.org/linux/man-pages/man2/select.2.html
[2]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/poll.html

Best Regards,
Hayato Kuroda
FUJITSU LIMITED



Attachments:

  [application/octet-stream] v28-0001-Add-PQconnCheck-and-PQcanConnCheck-to-libpq.patch (5.3K, ../TYAPR01MB5866B55A252EF7005AA81D5BF5CC9@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-v28-0001-Add-PQconnCheck-and-PQcanConnCheck-to-libpq.patch)
  download | inline diff:
From 2f17e7ce900c17dd06b03aa891890b345917d303 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Fri, 27 Jan 2023 03:17:18 +0000
Subject: [PATCH v28 1/4] Add PQconnCheck and PQcanConnCheck to libpq

PQconnCheck() function allows to check the status of socket by polling
the socket. This function is currently available only on systems that
support the non-standard POLLRDHUP extension to the poll system call,
including Linux.

PQcanConnCheck() checks whether above function is available or not.
---
 doc/src/sgml/libpq.sgml          | 40 ++++++++++++++++++
 src/interfaces/libpq/exports.txt |  2 +
 src/interfaces/libpq/fe-misc.c   | 69 ++++++++++++++++++++++++++++++++
 src/interfaces/libpq/libpq-fe.h  |  4 ++
 4 files changed, 115 insertions(+)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0e7ae70c70..5e07a252ce 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2679,6 +2679,46 @@ void *PQgetssl(const PGconn *conn);
      </listitem>
     </varlistentry>
 
+    <varlistentry id="libpq-PQconnCheck">
+     <term><function>PQconnCheck</function><indexterm><primary>PQconnCheck</primary></indexterm></term>
+     <listitem>
+      <para>
+       Returns the status of the socket.
+
+<synopsis>
+int PQconnCheck(PGconn *conn);
+</synopsis>
+      </para>
+
+      <para>
+       Unlike <xref linkend="libpq-PQstatus"/>, this function checks socket
+       health. This check is performed by polling the socket. This function is
+       currently available only on systems that support the non-standard
+       <symbol>POLLRDHUP</symbol> extension to the <symbol>poll</symbol> system
+       call, including Linux. <xref linkend="libpq-PQconnCheck"/> returns <literal>1</literal>
+       if the remote peer seems to be closed, returns <literal>0</literal> if
+       the socket is valid, and returns <literal>-1</literal> if the connection
+       has been already invalid or an error is error occurred.
+      </para>
+     </listitem>
+    </varlistentry>
+
+    <varlistentry id="libpq-PQcanConnCheck">
+     <term><function>PQcanConnCheck</function><indexterm><primary>PQcanConnCheck</primary></indexterm></term>
+     <listitem>
+      <para>
+       Returns whether <xref linkend="libpq-PQconnCheck"/> is available on this
+       platform. <xref linkend="libpq-PQcanConnCheck"/> returns
+       <literal>1</literal> if the function is supported, otherwise returns
+       <literal>0</literal>.
+
+<synopsis>
+int PQcanConnCheck(void);
+</synopsis>
+      </para>
+     </listitem>
+    </varlistentry>
+
    </variablelist>
   </para>
 
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index e8bcc88370..5c908bfe6e 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -186,3 +186,5 @@ PQpipelineStatus          183
 PQsetTraceFlags           184
 PQmblenBounded            185
 PQsendFlushRequest        186
+PQconnCheck               187
+PQcanConnCheck            188
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 3653a1a8a6..6e807e7c6a 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -1218,6 +1218,75 @@ PQenv2encoding(void)
 	return encoding;
 }
 
+/*
+ * Helper function for PQconnCheck().
+ *
+ * Return >0 if opposite side seems to be disconnected, 0 the socket is valid,
+ * and -1 if an error occurred.
+ */
+static int
+pqconnCheck_internal(int sock)
+{
+#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+	struct pollfd input_fd;
+	int errflags = POLLHUP | POLLERR | POLLNVAL;
+	int result;
+
+	/* Prepare pollfd entry */
+	input_fd.fd = sock;
+	input_fd.events = POLLRDHUP | errflags;
+	input_fd.revents = 0;
+
+	/*
+	 * Check the status of socket. Note that we will retry as long as we get
+	 * EINTR.
+	 */
+	do
+		result = poll(&input_fd, 1, 0);
+	while (result < 0 && errno == EINTR);
+
+	if (result < 0)
+			return -1;
+
+	return input_fd.revents;
+#else
+	/* Do not support socket checking on this platform, return 0 */
+	return 0;
+#endif
+}
+
+/*
+ * Check whether the socket peer closed connection or not.
+ *
+ * Returns >0 if remote peer seems to be closed, 0 if it is valid,
+ * -1 if the input connection is bad or an error occurred.
+ */
+int
+PQconnCheck(PGconn *conn)
+{
+	/* quick exit if invalid connection has come */
+	if (conn == NULL ||
+		conn->sock == PGINVALID_SOCKET ||
+		conn->status != CONNECTION_OK)
+		return -1;
+
+	return pqconnCheck_internal(conn->sock);
+}
+
+/*
+ * Check whether PQconnCheck() can work well on this platform.
+ *
+ * Returns 1 if this can use PQconnCheck(), otherwise 0.
+ */
+int
+PQcanConnCheck(void)
+{
+#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+	return true;
+#else
+	return false;
+#endif
+}
 
 #ifdef ENABLE_NLS
 
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index f3d9220496..04a0395efb 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -648,6 +648,10 @@ extern int	PQdsplen(const char *s, int encoding);
 /* Get encoding id from environment variable PGCLIENTENCODING */
 extern int	PQenv2encoding(void);
 
+/* Check whether the postgres server is still alive or not */
+extern int PQconnCheck(PGconn *conn);
+extern int PQcanConnCheck(void);
+
 /* === in fe-auth.c === */
 
 extern char *PQencryptPassword(const char *passwd, const char *user);
-- 
2.27.0



  [application/octet-stream] v28-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch (11.0K, ../TYAPR01MB5866B55A252EF7005AA81D5BF5CC9@TYAPR01MB5866.jpnprd01.prod.outlook.com/3-v28-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch)
  download | inline diff:
From f36effa4eb0045682249c2113afa354b3733a309 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Fri, 27 Jan 2023 03:17:30 +0000
Subject: [PATCH v28 2/4] postgres_fdw: add
 postgres_fdw_verify_connection_states

This function can verify the status of connections that are establieshed by
postgres_fdw. This check wil be done by PQconnCheck(), which means this is
available only on systems that support the non-standard POLLRDHUP extension
to the poll system call, including Linux.
This returns true if checked connection is still valid, or the checking is
not supported on this platform. False is returned if the connection seems
to be closed.
---
 contrib/postgres_fdw/Makefile                 |   2 +-
 contrib/postgres_fdw/connection.c             | 124 ++++++++++++++++++
 contrib/postgres_fdw/meson.build              |   1 +
 .../postgres_fdw/postgres_fdw--1.1--1.2.sql   |  19 +++
 contrib/postgres_fdw/postgres_fdw.control     |   2 +-
 doc/src/sgml/postgres-fdw.sgml                |  69 ++++++++++
 6 files changed, 215 insertions(+), 2 deletions(-)
 create mode 100644 contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql

diff --git a/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile
index c1b0cad453..6d23768389 100644
--- a/contrib/postgres_fdw/Makefile
+++ b/contrib/postgres_fdw/Makefile
@@ -14,7 +14,7 @@ PG_CPPFLAGS = -I$(libpq_srcdir)
 SHLIB_LINK_INTERNAL = $(libpq)
 
 EXTENSION = postgres_fdw
-DATA = postgres_fdw--1.0.sql postgres_fdw--1.0--1.1.sql
+DATA = postgres_fdw--1.0.sql postgres_fdw--1.0--1.1.sql postgres_fdw--1.1--1.2.sql
 
 REGRESS = postgres_fdw
 
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 7760380f00..1d7e350b33 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -87,6 +87,9 @@ static bool xact_got_connection = false;
 PG_FUNCTION_INFO_V1(postgres_fdw_get_connections);
 PG_FUNCTION_INFO_V1(postgres_fdw_disconnect);
 PG_FUNCTION_INFO_V1(postgres_fdw_disconnect_all);
+PG_FUNCTION_INFO_V1(postgres_fdw_verify_connection_states);
+PG_FUNCTION_INFO_V1(postgres_fdw_verify_connection_states_all);
+PG_FUNCTION_INFO_V1(postgres_fdw_can_verify_connection_states);
 
 /* prototypes of private functions */
 static void make_new_connection(ConnCacheEntry *entry, UserMapping *user);
@@ -117,6 +120,7 @@ static void pgfdw_finish_pre_subcommit_cleanup(List *pending_entries,
 											   int curlevel);
 static bool UserMappingPasswordRequired(UserMapping *user);
 static bool disconnect_cached_connections(Oid serverid);
+static bool verify_cached_connections(Oid serverid);
 
 /*
  * Get a PGconn which can be used to execute queries on the remote PostgreSQL
@@ -1832,3 +1836,123 @@ disconnect_cached_connections(Oid serverid)
 
 	return result;
 }
+
+/*
+ * Workhorse to verify cached connections.
+ *
+ * This function scans all the connection cache entries and verifies the
+ * connections whose foreign server OID matches with the specified one. If
+ * InvalidOid is specified, it verifies all the cached connections.
+ *
+ * This function emits warnings if a disconnection is found, and this returns
+ * false only when the lastly verified server seems to be disconnected.
+ *
+ * Note that the verification can be used on some limited platforms. If this
+ * server does not support it, this function alwayse returns true.
+ */
+static bool
+verify_cached_connections(Oid serverid)
+{
+	HASH_SEQ_STATUS scan;
+	ConnCacheEntry *entry;
+	bool		all = !OidIsValid(serverid);
+	bool		result = true;
+	StringInfoData str;
+
+	/* quick exit if connection cache has not been initialized yet. */
+	if (!ConnectionHash)
+		return true;
+
+	hash_seq_init(&scan, ConnectionHash);
+	while ((entry = (ConnCacheEntry *) hash_seq_search(&scan)))
+	{
+		/* Ignore cache entry if no open connection right now. */
+		if (!entry->conn)
+			continue;
+
+		/* Skip if the entry is invalidated. */
+		if (entry->invalidated)
+			continue;
+
+		if (all || entry->serverid == serverid)
+		{
+			if (PQconnCheck(entry->conn))
+			{
+				/* A foreign server might be down, so construct a message. */
+				ForeignServer *server = GetForeignServer(entry->serverid);
+
+				if (result)
+				{
+					/*
+					 * Initialize and add a prefix if this is the first
+					 * disconnection we found.
+					 */
+					initStringInfo(&str);
+					appendStringInfo(&str, "could not connect to server ");
+
+					result = false;
+				}
+				else
+					appendStringInfo(&str, ", ");
+
+				appendStringInfo(&str, "\"%s\"", server->servername);
+			}
+		}
+	}
+
+	/* Raise a warning if disconnections are found. */
+	if (!result)
+	{
+		Assert(str.len);
+		ereport(WARNING,
+				(errcode(ERRCODE_CONNECTION_FAILURE),
+				 errmsg("%s", str.data),
+				 errdetail("Socket close is detected."),
+				 errhint("Plsease check the health of server.")));
+		pfree(str.data);
+	}
+
+	return result;
+}
+
+/*
+ * Verify the specified cached connections.
+ *
+ * This function verifies the connections that are established by postgres_fdw
+ * from the local session to the foreign server with the given name.
+ *
+ * This function emits a warning if a disconnection is found, and this returns
+ * false only when the verified server seems to be disconnected.
+ *
+ * Note that the verification can be used on some limited platforms. If this
+ * server does not support it, this function alwayse returns true.
+ */
+Datum
+postgres_fdw_verify_connection_states(PG_FUNCTION_ARGS)
+{
+	ForeignServer *server;
+	char	   *servername;
+
+	servername = text_to_cstring(PG_GETARG_TEXT_PP(0));
+	server = GetForeignServerByName(servername, false);
+
+	PG_RETURN_BOOL(verify_cached_connections(server->serverid));
+}
+
+/*
+ * Verify all the cached connections.
+ *
+ * This function verifies all the connections that are established by postgres_fdw
+ * from the local session to the foreign servers.
+ */
+Datum
+postgres_fdw_verify_connection_states_all(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_BOOL(verify_cached_connections(InvalidOid));
+}
+
+Datum
+postgres_fdw_can_verify_connection_states(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_BOOL(PQcanConnCheck());
+}
diff --git a/contrib/postgres_fdw/meson.build b/contrib/postgres_fdw/meson.build
index 2b451f165e..29118d47bb 100644
--- a/contrib/postgres_fdw/meson.build
+++ b/contrib/postgres_fdw/meson.build
@@ -26,6 +26,7 @@ install_data(
   'postgres_fdw.control',
   'postgres_fdw--1.0.sql',
   'postgres_fdw--1.0--1.1.sql',
+  'postgres_fdw--1.1--1.2.sql',
   kwargs: contrib_data_args,
 )
 
diff --git a/contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql b/contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql
new file mode 100644
index 0000000000..a8556b4c9f
--- /dev/null
+++ b/contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql
@@ -0,0 +1,19 @@
+/* contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION postgres_fdw UPDATE TO '1.2'" to load this file. \quit
+
+CREATE FUNCTION postgres_fdw_verify_connection_states (text)
+RETURNS bool
+AS 'MODULE_PATHNAME'
+LANGUAGE C STRICT PARALLEL RESTRICTED;
+
+CREATE FUNCTION postgres_fdw_verify_connection_states_all ()
+RETURNS bool
+AS 'MODULE_PATHNAME'
+LANGUAGE C STRICT PARALLEL RESTRICTED;
+
+CREATE FUNCTION postgres_fdw_can_verify_connection_states ()
+RETURNS bool
+AS 'MODULE_PATHNAME'
+LANGUAGE C STRICT PARALLEL SAFE;
diff --git a/contrib/postgres_fdw/postgres_fdw.control b/contrib/postgres_fdw/postgres_fdw.control
index d489382064..a4b800be4f 100644
--- a/contrib/postgres_fdw/postgres_fdw.control
+++ b/contrib/postgres_fdw/postgres_fdw.control
@@ -1,5 +1,5 @@
 # postgres_fdw extension
 comment = 'foreign-data wrapper for remote PostgreSQL servers'
-default_version = '1.1'
+default_version = '1.2'
 module_pathname = '$libdir/postgres_fdw'
 relocatable = true
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 644f51835b..05265a1ef8 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -826,6 +826,75 @@ postgres=# SELECT postgres_fdw_disconnect_all();
      </para>
     </listitem>
    </varlistentry>
+
+   <varlistentry>
+    <term><function>postgres_fdw_verify_connection_states(server_name text) returns boolean</function></term>
+    <listitem>
+     <para>
+      This function checks the status of remote connections established by
+      <filename>postgres_fdw</filename> from the local session to the foreign
+      server with the given name. This check is performed by polling the socket
+      and allows long-running transactions to be aborted sooner if the kernel
+      reports that the connection is closed. This function is currently
+      available only on systems that support the non-standard <symbol>POLLRDHUP</symbol>
+      extension to the <symbol>poll</symbol> system call, including Linux. This
+      returns <literal>true</literal> if checked connections are still valid,
+      or the checking is not supported on this platform. <literal>false</literal>
+      is returned if the local session seems to be disconnected from other
+      servers. Example usage of the function:
+<screen>
+postgres=# SELECT postgres_fdw_verify_connection_states('loopback1');
+ postgres_fdw_verify_connection_states
+---------------------------------------
+ t
+</screen>
+     </para>
+    </listitem>
+   </varlistentry>
+   <varlistentry>
+    <term><function>postgres_fdw_verify_connection_states_all() returns boolean</function></term>
+    <listitem>
+     <para>
+      This function checks the status of all the remote connections established
+      by <filename>postgres_fdw</filename> from the local session to the
+      foreign servers. This check is performed by polling the socket and allows
+      long-running transactions to be aborted sooner if the kernel reports
+      that the connection is closed. This function is currently available only
+      on systems that support the non-standard <symbol>POLLRDHUP</symbol>
+      extension to the <symbol>poll</symbol> system call, including Linux. This
+      returns <literal>true</literal> if all the remote connections are still
+      valid, or the checking is not supported on this platform.
+      <literal>false</literal> is returned if the local session seems to be
+      disconnected from at least one remote server. Example usage of the
+      function:
+<screen>
+postgres=# SELECT postgres_fdw_verify_connection_states_all();
+ postgres_fdw_verify_connection_states_all
+-------------------------------------------
+ t
+</screen>
+     </para>
+    </listitem>
+   </varlistentry>
+
+   <varlistentry>
+    <term><function>postgres_fdw_can_verify_connection_states() returns boolean</function></term>
+    <listitem>
+     <para>
+      This function checks whether functions the health of remote connectio
+      work well or not. This returns <literal>true</literal> if it can be used,
+      otherwise returns <literal>false</literal>. Example usage of the function:
+
+<screen>
+postgres=# SELECT postgres_fdw_can_verify_connection_states();
+ postgres_fdw_can_verify_connection_states
+-------------------------------------------
+ t
+</screen>
+     </para>
+    </listitem>
+   </varlistentry>
+
    </variablelist>
 
 </sect2>
-- 
2.27.0



  [application/octet-stream] v28-0003-add-test.patch (4.6K, ../TYAPR01MB5866B55A252EF7005AA81D5BF5CC9@TYAPR01MB5866.jpnprd01.prod.outlook.com/4-v28-0003-add-test.patch)
  download | inline diff:
From 7d74923f454f0e5e0bd2a6ff47818efa9d4a5614 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Fri, 27 Jan 2023 03:17:36 +0000
Subject: [PATCH v28 3/4] add test

---
 .../postgres_fdw/expected/postgres_fdw.out    | 54 +++++++++++++++++++
 contrib/postgres_fdw/sql/postgres_fdw.sql     | 53 ++++++++++++++++++
 2 files changed, 107 insertions(+)

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 2350cfe148..28b5af8d78 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -11798,3 +11798,57 @@ ANALYZE analyze_table;
 -- cleanup
 DROP FOREIGN TABLE analyze_ftable;
 DROP TABLE analyze_table;
+-- ===================================================================
+-- test for postgres_fdw_verify_foreign_servers function
+-- ===================================================================
+-- Disable debug_discard_caches in order to manage remote connections
+SET debug_discard_caches TO '0';
+-- -- The text of the error might vary across platforms, so only show SQLSTATE.
+\set VERBOSITY sqlstate
+-- Disconnect once and set application_name to an arbitrary value
+SELECT 1 FROM postgres_fdw_disconnect_all();
+ ?column? 
+----------
+        1
+(1 row)
+
+ALTER SERVER loopback OPTIONS (SET application_name 'healthcheck');
+DO $$
+DECLARE
+	can_verify boolean;
+	result boolean;
+BEGIN
+	PERFORM 1 FROM ft1 LIMIT 1;
+
+	-- Terminate the remote backend process
+	PERFORM pg_terminate_backend(pid, 180000) FROM pg_stat_activity
+		WHERE application_name = 'healthcheck';
+
+	-- Check whether we can do health check on this platform
+	SELECT INTO can_verify postgres_fdw_can_verify_connection_states();
+
+	-- If the checking can be done on this platform, call it
+	IF can_verify IS TRUE THEN
+		-- Set client_min_messages to ERROR temporary because the following
+		-- function only throws a WARNING on the supported platform.
+		SET LOCAL client_min_messages TO ERROR;
+
+		SELECT INTO result postgres_fdw_verify_connection_states('loopback');
+
+		RESET client_min_messages;
+	ELSE
+		result = false;
+	END IF;
+
+	-- If result is FALSE, we succeeded to detect the disconnection or it could
+	-- not be done on this platform. Raise an message.
+	IF result IS FALSE THEN
+		RAISE INFO 'postgres_fdw_verify_connection_states() could detect the disconnection, or health check cannot be used on this platform';
+	END IF;
+END;
+$$;
+INFO:  00000
+ERROR:  08006
+-- Clean up
+\set VERBOSITY default
+RESET debug_discard_caches;
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index c37aa80383..49042eb8c1 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3974,3 +3974,56 @@ ANALYZE analyze_table;
 -- cleanup
 DROP FOREIGN TABLE analyze_ftable;
 DROP TABLE analyze_table;
+
+-- ===================================================================
+-- test for postgres_fdw_verify_foreign_servers function
+-- ===================================================================
+
+-- Disable debug_discard_caches in order to manage remote connections
+SET debug_discard_caches TO '0';
+
+-- -- The text of the error might vary across platforms, so only show SQLSTATE.
+\set VERBOSITY sqlstate
+
+-- Disconnect once and set application_name to an arbitrary value
+SELECT 1 FROM postgres_fdw_disconnect_all();
+ALTER SERVER loopback OPTIONS (SET application_name 'healthcheck');
+
+DO $$
+DECLARE
+	can_verify boolean;
+	result boolean;
+BEGIN
+	PERFORM 1 FROM ft1 LIMIT 1;
+
+	-- Terminate the remote backend process
+	PERFORM pg_terminate_backend(pid, 180000) FROM pg_stat_activity
+		WHERE application_name = 'healthcheck';
+
+	-- Check whether we can do health check on this platform
+	SELECT INTO can_verify postgres_fdw_can_verify_connection_states();
+
+	-- If the checking can be done on this platform, call it
+	IF can_verify IS TRUE THEN
+		-- Set client_min_messages to ERROR temporary because the following
+		-- function only throws a WARNING on the supported platform.
+		SET LOCAL client_min_messages TO ERROR;
+
+		SELECT INTO result postgres_fdw_verify_connection_states('loopback');
+
+		RESET client_min_messages;
+	ELSE
+		result = false;
+	END IF;
+
+	-- If result is FALSE, we succeeded to detect the disconnection or it could
+	-- not be done on this platform. Raise an message.
+	IF result IS FALSE THEN
+		RAISE INFO 'postgres_fdw_verify_connection_states() could detect the disconnection, or health check cannot be used on this platform';
+	END IF;
+END;
+$$;
+
+-- Clean up
+\set VERBOSITY default
+RESET debug_discard_caches;
-- 
2.27.0



  [application/octet-stream] v28-0004-add-kqueue-support-for-PQconnCheck-and-PQcanConn.patch (5.1K, ../TYAPR01MB5866B55A252EF7005AA81D5BF5CC9@TYAPR01MB5866.jpnprd01.prod.outlook.com/5-v28-0004-add-kqueue-support-for-PQconnCheck-and-PQcanConn.patch)
  download | inline diff:
From cbb2d8208e0afce89e8110126c5d745c3e947c92 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Fri, 27 Jan 2023 03:17:41 +0000
Subject: [PATCH v28 4/4] add kqueue support for PQconnCheck and PQcanConnCheck

---
 doc/src/sgml/libpq.sgml        | 15 ++++++------
 doc/src/sgml/postgres-fdw.sgml | 10 ++++----
 src/interfaces/libpq/fe-misc.c | 43 +++++++++++++++++++++++++++++++++-
 3 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 5e07a252ce..56e495b3f1 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2692,13 +2692,14 @@ int PQconnCheck(PGconn *conn);
 
       <para>
        Unlike <xref linkend="libpq-PQstatus"/>, this function checks socket
-       health. This check is performed by polling the socket. This function is
-       currently available only on systems that support the non-standard
-       <symbol>POLLRDHUP</symbol> extension to the <symbol>poll</symbol> system
-       call, including Linux. <xref linkend="libpq-PQconnCheck"/> returns <literal>1</literal>
-       if the remote peer seems to be closed, returns <literal>0</literal> if
-       the socket is valid, and returns <literal>-1</literal> if the connection
-       has been already invalid or an error is error occurred.
+       health. This check is performed by polling the socket. This option
+       relies on kernel events exposed by Linux, macOS, illumos and the BSD
+       family of operating systems, and is not currently available on other
+       systems. <xref linkend="libpq-PQconnCheck"/> returns
+       <literal>1</literal> if the remote peer seems to be closed, returns
+       <literal>0</literal> if the socket is valid, and returns
+       <literal>-1</literal> if the connection has been already invalid or
+       an error is error occurred.
       </para>
      </listitem>
     </varlistentry>
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 05265a1ef8..b6912f4d94 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -859,11 +859,11 @@ postgres=# SELECT postgres_fdw_verify_connection_states('loopback1');
       by <filename>postgres_fdw</filename> from the local session to the
       foreign servers. This check is performed by polling the socket and allows
       long-running transactions to be aborted sooner if the kernel reports
-      that the connection is closed. This function is currently available only
-      on systems that support the non-standard <symbol>POLLRDHUP</symbol>
-      extension to the <symbol>poll</symbol> system call, including Linux. This
-      returns <literal>true</literal> if all the remote connections are still
-      valid, or the checking is not supported on this platform.
+      that the connection is closed. This option relies on kernel events
+      exposed by Linux, macOS, illumos and the BSD family of operating systems,
+      and is not currently available on other systems. This returns
+      <literal>true</literal> if all the remote connections are still valid,
+      or the checking is not supported on this platform.
       <literal>false</literal> is returned if the local session seems to be
       disconnected from at least one remote server. Example usage of the
       function:
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 6e807e7c6a..9578f905f9 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -43,6 +43,10 @@
 
 #ifdef HAVE_POLL_H
 #include <poll.h>
+#elif HAVE_KQUEUE
+#include <sys/types.h>
+#include <sys/event.h>
+#include <sys/time.h>
 #endif
 
 #include "libpq-fe.h"
@@ -1227,6 +1231,7 @@ PQenv2encoding(void)
 static int
 pqconnCheck_internal(int sock)
 {
+	/* We use poll(2) if available, otherwise kqueue(2) */
 #if (defined(HAVE_POLL) && defined(POLLRDHUP))
 	struct pollfd input_fd;
 	int errflags = POLLHUP | POLLERR | POLLNVAL;
@@ -1249,6 +1254,41 @@ pqconnCheck_internal(int sock)
 			return -1;
 
 	return input_fd.revents;
+#elif defined(HAVE_KQUEUE)
+	struct kevent kev, ret;
+	struct timespec timeout = {};
+	static int kq = -1;
+	int result;
+
+	/* If this function has never called yet, create a kernel event queue */
+	if (kq < 0)
+	{
+		kq = kqueue();
+		if (kq < 0)
+			return -1;
+	}
+
+	/* Prepare kevent structure */
+	EV_SET(&kev, sock, EVFILT_READ, EV_ADD, 0, 0, NULL);
+	if (kevent(kq, &kev, 1, NULL, 0, NULL))
+		return -1;
+
+	/*
+	 * Check the status of socket. Note that we will retry as long as we get
+	 * EINTR.
+	 */
+	do
+		result = kevent(kq, NULL, 0, &ret, 1, &timeout);
+	while (result < 0 && errno == EINTR);
+
+	/* Clean up the queue */
+	EV_SET(&kev, sock, EVFILT_READ, EV_DELETE, 0, 0, NULL);
+	kevent(kq, &kev, 1, NULL, 0, NULL);
+
+	if (result < 0)
+		return -1;
+
+	return ret.flags & EV_EOF;
 #else
 	/* Do not support socket checking on this platform, return 0 */
 	return 0;
@@ -1281,7 +1321,8 @@ PQconnCheck(PGconn *conn)
 int
 PQcanConnCheck(void)
 {
-#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+#if ((defined(HAVE_POLL) && defined(POLLRDHUP)) || \
+	 defined(HAVE_KQUEUE))
 	return true;
 #else
 	return false;
-- 
2.27.0



view thread (19+ messages)  latest in thread

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: <TYAPR01MB5866B55A252EF7005AA81D5BF5CC9@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