public inbox for [email protected]
help / color / mirror / Atom feedFrom: 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: Thu, 9 Feb 2023 14:39:34 +0000
Message-ID: <TYAPR01MB58669EAAC02493BFF9F39B06F5D99@TYAPR01MB5866.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <[email protected]>
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>
<TYAPR01MB5866B55A252EF7005AA81D5BF5CC9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<TYAPR01MB586675882721E646693E13FBF5CC9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<[email protected]>
Dear Katsuragi-san,
Thank you for reviewing! PSA new version patches.
> 0001:
> + while (result < 0 && errno == EINTR);
> +
> + if (result < 0)
> + return -1;
>
> this `return -1` is not indented properly.
This part is no longer needed. Please see another discussion[1].
> 0002:
> + <term><function>postgres_fdw_verify_connection_states(server_name
> text) returns boolean</function></term>
> ...
> + 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:
>
> Here, 'still valid' seems a little bit confusing because this 'valid' is
> not
> the same as postgres_fdw_get_connections's 'valid' [1].
>
> Should 'still valid' be 'existing connection is not closed by the remote
> peer'?
> But this description does not cover all the cases where this function
> returns true...
> I think one choice is to write all the cases like 'returns true if any
> of the
> following condition is satisfied. 1) existing connection is not closed
> by the
> remote peer 2) there is no connection for specified server yet 3) the
> checking
> is not supported...'. If my understanding is not correct, please point
> it out.
Modified like you pointed out.
> BTW, is it reasonable to return true if ConnectionHash is not
> initialized or
> there is no ConnCacheEntry for specified remote server? What do you
> think
> about returning NULL in that case?
I'm not sure which one is better, but modified accordingly.
> 0003:
> I think it is better that the test covers all the new functions.
> How about adding a test for postgres_fdw_verify_connection_states_all?
>
>
> +--
> ===================================================
> ================
> +-- test for postgres_fdw_verify_foreign_servers function
> +--
> ===================================================
> ================
>
> Writing all the functions' name like 'test for
> postgres_fdw_verify_connection_states
> and postgres_fdw_can_verify_connection_states' looks straightforward.
> What do you think about this?
Added.
> 0004:
> Sorry, I have not read 0004. I will read.
No problem:-).
[1]: https://www.postgresql.org/message-id/TYAPR01MB58664E039F45959AB321FA1FF5D99%40TYAPR01MB5866.jpnprd0...
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/octet-stream] v31-0001-Add-PQconnCheck-and-PQcanConnCheck-to-libpq.patch (7.1K, ../TYAPR01MB58669EAAC02493BFF9F39B06F5D99@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-v31-0001-Add-PQconnCheck-and-PQcanConnCheck-to-libpq.patch)
download | inline diff:
From f363393138f78d087d8e319d0eccdf04adc2bc00 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Fri, 27 Jan 2023 03:17:18 +0000
Subject: [PATCH v31 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 | 41 +++++++++++++++++++++
src/interfaces/libpq/exports.txt | 2 ++
src/interfaces/libpq/fe-misc.c | 61 +++++++++++++++++++++++++++-----
src/interfaces/libpq/libpq-fe.h | 4 +++
4 files changed, 99 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0e7ae70c70..f49b6e7452 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2679,6 +2679,47 @@ 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 health 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
+ greater than zero 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..3bcd760cd7 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -53,8 +53,8 @@
static int pqPutMsgBytes(const void *buf, size_t len, PGconn *conn);
static int pqSendSome(PGconn *conn, int len);
-static int pqSocketCheck(PGconn *conn, int forRead, int forWrite,
- time_t end_time);
+static int pqSocketIsReadableOrWritableOrValid(PGconn *conn, int forRead,
+ int forWrite, time_t end_time);
static int pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time);
/*
@@ -993,7 +993,7 @@ pqWaitTimed(int forRead, int forWrite, PGconn *conn, time_t finish_time)
{
int result;
- result = pqSocketCheck(conn, forRead, forWrite, finish_time);
+ result = pqSocketIsReadableOrWritableOrValid(conn, forRead, forWrite, finish_time);
if (result < 0)
return -1; /* errorMessage is already set */
@@ -1014,7 +1014,7 @@ pqWaitTimed(int forRead, int forWrite, PGconn *conn, time_t finish_time)
int
pqReadReady(PGconn *conn)
{
- return pqSocketCheck(conn, 1, 0, (time_t) 0);
+ return pqSocketIsReadableOrWritableOrValid(conn, 1, 0, (time_t) 0);
}
/*
@@ -1024,7 +1024,7 @@ pqReadReady(PGconn *conn)
int
pqWriteReady(PGconn *conn)
{
- return pqSocketCheck(conn, 0, 1, (time_t) 0);
+ return pqSocketIsReadableOrWritableOrValid(conn, 0, 1, (time_t) 0);
}
/*
@@ -1034,9 +1034,13 @@ pqWriteReady(PGconn *conn)
*
* If SSL is in use, the SSL buffer is checked prior to checking the socket
* for read data directly.
+ *
+ * Moreover, when neither forRead nor forWrite is requested and timeout is
+ * disabled, try to check the health of socket.
*/
static int
-pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
+pqSocketIsReadableOrWritableOrValid(PGconn *conn, int forRead, int forWrite,
+ time_t end_time)
{
int result;
@@ -1082,20 +1086,33 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
*
* Timeout is infinite if end_time is -1. Timeout is immediate (no blocking)
* if end_time is 0 (or indeed, any time before now).
+ *
+ * Moreover, when neither forRead nor forWrite is requested and timeout is
+ * disabled, try to check the health of socket.
*/
static int
pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time)
{
/* We use poll(2) if available, otherwise select(2) */
#ifdef HAVE_POLL
- struct pollfd input_fd;
+ struct pollfd input_fd = {0};
int timeout_ms;
if (!forRead && !forWrite)
- return 0;
+ {
+ /* Try to check the health if requested */
+ if (end_time == 0)
+#if defined(POLLRDHUP)
+ input_fd.events = POLLRDHUP | POLLHUP | POLLNVAL;
+#else
+ return 0;
+#endif /* defined(POLLRDHUP) */
+ else
+ return 0;
+ }
input_fd.fd = sock;
- input_fd.events = POLLERR;
+ input_fd.events |= POLLERR;
input_fd.revents = 0;
if (forRead)
@@ -1218,6 +1235,32 @@ PQenv2encoding(void)
return encoding;
}
+/*
+ * 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)
+{
+ return pqSocketIsReadableOrWritableOrValid(conn, 0, 0, (time_t) 0);
+}
+
+/*
+ * 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] v31-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch (11.7K, ../TYAPR01MB58669EAAC02493BFF9F39B06F5D99@TYAPR01MB5866.jpnprd01.prod.outlook.com/3-v31-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch)
download | inline diff:
From afc83105b6d734e93d54504df9be7734386a57b1 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Fri, 27 Jan 2023 03:17:30 +0000
Subject: [PATCH v31 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 any of the following condition is satisfied: 1) existing
connection is not closed by the remote peer, 2) there is no connection for specified
server yet, and 3) the checking is not supported on this platform.
---
contrib/postgres_fdw/Makefile | 2 +-
contrib/postgres_fdw/connection.c | 131 ++++++++++++++++++
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 | 74 ++++++++++
6 files changed, 227 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..602d810c08 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,130 @@ 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;
+
+ Assert(ConnectionHash);
+
+ 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. This returns
+ * false only when the verified server seems to be disconnected, and reutrns
+ * NULL if the connection cache has not been initialized yet.
+ *
+ * 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;
+
+ /* quick exit if connection cache has not been initialized yet. */
+ if (!ConnectionHash)
+ PG_RETURN_NULL();
+
+ 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)
+{
+ /* quick exit if connection cache has not been initialized yet. */
+ if (!ConnectionHash)
+ PG_RETURN_NULL();
+
+ 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..e1abdb38cb 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -826,6 +826,80 @@ 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 any of the following condition is
+ satisfied: 1) existing connection is not closed by the remote peer, 2)
+ there is no connection for specified server yet, and 3) the checking is
+ not supported on this platform. <literal>false</literal>
+ is returned if the local session seems to be disconnected from other
+ servers. <literal>NULL</literal> is returned if the local session does
+ not have a connection cache. 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 any of the following condition is
+ satisfied: 1) all connections are not closed by the remote peer, 2)
+ there are no connections yet, and 3) 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. <literal>NULL</literal> is returned if the local session does
+ not have a connection cache. 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] v31-0003-add-test.patch (5.3K, ../TYAPR01MB58669EAAC02493BFF9F39B06F5D99@TYAPR01MB5866.jpnprd01.prod.outlook.com/4-v31-0003-add-test.patch)
download | inline diff:
From 3ed051dd7e6c75194a043eaa643f70f2fd6f514d Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Fri, 27 Jan 2023 03:17:36 +0000
Subject: [PATCH v31 3/4] add test
---
.../postgres_fdw/expected/postgres_fdw.out | 64 +++++++++++++++++++
contrib/postgres_fdw/sql/postgres_fdw.sql | 62 ++++++++++++++++++
2 files changed, 126 insertions(+)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index d5fc61446a..d9926b3857 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -11798,3 +11798,67 @@ 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');
+-- Define procedure for testing verify functions
+CREATE PROCEDURE test_verify_function(use_all boolean) AS $$
+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;
+
+ IF use_all IS TRUE THEN
+ SELECT INTO result postgres_fdw_verify_connection_states_all();
+ ELSE
+ SELECT INTO result postgres_fdw_verify_connection_states('loopback');
+ END IF;
+
+ 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_all() could detect the disconnection, or health check cannot be used on this platform';
+ END IF;
+END;
+$$ LANGUAGE plpgsql;
+-- ..And call above function
+CALL test_verify_function(false);
+INFO: 00000
+ERROR: 08006
+CALL test_verify_function(true);
+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 1e50be137b..81c03d0869 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3974,3 +3974,65 @@ 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');
+
+-- Define procedure for testing verify functions
+CREATE PROCEDURE test_verify_function(use_all boolean) AS $$
+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;
+
+ IF use_all IS TRUE THEN
+ SELECT INTO result postgres_fdw_verify_connection_states_all();
+ ELSE
+ SELECT INTO result postgres_fdw_verify_connection_states('loopback');
+ END IF;
+
+ 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_all() could detect the disconnection, or health check cannot be used on this platform';
+ END IF;
+END;
+$$ LANGUAGE plpgsql;
+
+-- ..And call above function
+CALL test_verify_function(false);
+CALL test_verify_function(true);
+
+-- Clean up
+\set VERBOSITY default
+RESET debug_discard_caches;
--
2.27.0
[application/octet-stream] v31-0004-add-kqueue-support-for-PQconnCheck-and-PQcanConn.patch (8.8K, ../TYAPR01MB58669EAAC02493BFF9F39B06F5D99@TYAPR01MB5866.jpnprd01.prod.outlook.com/5-v31-0004-add-kqueue-support-for-PQconnCheck-and-PQcanConn.patch)
download | inline diff:
From f87cf8bfbcf943b21fd84c8b12b00d92f5367b1d Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Thu, 9 Feb 2023 11:58:35 +0000
Subject: [PATCH v31 4/4] add kqueue support for PQconnCheck and PQcanConnCheck
---
doc/src/sgml/libpq.sgml | 16 +++----
doc/src/sgml/postgres-fdw.sgml | 34 +++++++-------
src/interfaces/libpq/fe-misc.c | 84 ++++++++++++++++++++++++++++++++--
3 files changed, 106 insertions(+), 28 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index f49b6e7452..2abc2ed508 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2692,14 +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
- greater than zero 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 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 greater than zero 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 e1abdb38cb..75ca74a16e 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -835,13 +835,13 @@ postgres=# SELECT postgres_fdw_disconnect_all();
<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 any of the following condition is
- satisfied: 1) existing connection is not closed by the remote peer, 2)
- there is no connection for specified server yet, and 3) the checking is
- not supported on this platform. <literal>false</literal>
+ reports that the connection is closed. 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. This returns <literal>true</literal> if any
+ of the following condition is satisfied: 1) existing connection is not
+ closed by the remote peer, 2) there is no connection for specified server
+ yet, and 3) the checking is not supported on this platform. <literal>false</literal>
is returned if the local session seems to be disconnected from other
servers. <literal>NULL</literal> is returned if the local session does
not have a connection cache. Example usage of the function:
@@ -862,16 +862,16 @@ 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 any of the following condition is
- satisfied: 1) all connections are not closed by the remote peer, 2)
- there are no connections yet, and 3) 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. <literal>NULL</literal> is returned if the local session does
- not have a connection cache. Example usage of the function:
+ that the connection is closed. 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. This returns <literal>true</literal> if any
+ of the following condition is satisfied: 1) all connections are not
+ closed by the remote peer, 2) there are no connections yet, and 3) 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. <literal>NULL</literal> is returned if the local session
+ does not have a connection cache. Example usage of the function:
<screen>
postgres=# SELECT postgres_fdw_verify_connection_states_all();
postgres_fdw_verify_connection_states_all
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 3bcd760cd7..caa33cd2e4 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -45,6 +45,18 @@
#include <poll.h>
#endif
+/* We use poll(2) for checking the health of a socket, otherwise kqueue(2) */
+#if (!(defined(HAVE_POLL) && defined(POLLRDHUP)) && \
+ defined(HAVE_KQUEUE))
+#define CHECK_USE_KQUEUE
+#endif
+
+#if defined(CHECK_USE_KQUEUE)
+#include <sys/types.h>
+#include <sys/event.h>
+#include <sys/time.h>
+#endif
+
#include "libpq-fe.h"
#include "libpq-int.h"
#include "mb/pg_wchar.h"
@@ -57,6 +69,10 @@ static int pqSocketIsReadableOrWritableOrValid(PGconn *conn, int forRead,
int forWrite, time_t end_time);
static int pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time);
+#if defined(CHECK_USE_KQUEUE)
+static int pqSocketKqueue(int sock);
+#endif
+
/*
* PQlibVersion: return the libpq version number
*/
@@ -1104,9 +1120,12 @@ pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time)
if (end_time == 0)
#if defined(POLLRDHUP)
input_fd.events = POLLRDHUP | POLLHUP | POLLNVAL;
+#elif defined(CHECK_USE_KQUEUE)
+ /* Use kqueue(2) instead */
+ return pqSocketKqueue(sock);
#else
return 0;
-#endif /* defined(POLLRDHUP) */
+#endif
else
return 0;
}
@@ -1143,7 +1162,22 @@ pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time)
struct timeval *ptr_timeout;
if (!forRead && !forWrite)
- return 0;
+ {
+ /*
+ * Try to check the health if requested
+ *
+ * XXX: Is there any systems that cannot use poll(2) but have
+ * kqueue(2) system call?
+ */
+ if (end_time == 0)
+#if defined(CHECK_USE_KQUEUE)
+ return pqSocketKqueue(sock);
+#else
+ return 0;
+#endif /* defined(CHECK_USE_KQUEUE) */
+ else
+ return 0;
+ }
FD_ZERO(&input_mask);
FD_ZERO(&output_mask);
@@ -1175,6 +1209,49 @@ pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time)
#endif /* HAVE_POLL */
}
+#if defined(CHECK_USE_KQUEUE)
+static int
+pqSocketKqueue(int sock)
+{
+ struct kevent kev,
+ ret;
+ struct timespec timeout = {0};
+ 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;
+
+ printf("debug print");
+
+ return ret.flags & EV_EOF;
+}
+#endif /* defined(CHECK_USE_KQUEUE) */
/*
* A couple of "miscellaneous" multibyte related functions. They used
@@ -1255,7 +1332,8 @@ PQconnCheck(PGconn *conn)
int
PQcanConnCheck(void)
{
-#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+#if ((defined(HAVE_POLL) && defined(POLLRDHUP)) || \
+ defined(CHECK_USE_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: <TYAPR01MB58669EAAC02493BFF9F39B06F5D99@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