public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v19 2/4] Move removal of old serialized snapshots to custodian.
19+ messages / 6 participants
[nested] [flat]
* [PATCH v19 2/4] Move removal of old serialized snapshots to custodian.
@ 2021-12-06 06:02 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 19+ messages in thread
From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw)
This was only done during checkpoints because it was a convenient
place to put it. However, if there are many snapshots to remove,
it can significantly extend checkpoint time. To avoid this, move
this work to the newly-introduced custodian process.
---
contrib/test_decoding/expected/rewrite.out | 21 +++++++++++++++++++++
contrib/test_decoding/sql/rewrite.sql | 17 +++++++++++++++++
src/backend/access/transam/xlog.c | 6 ++++--
src/backend/postmaster/custodian.c | 2 ++
src/backend/replication/logical/snapbuild.c | 9 ++++-----
src/include/postmaster/custodian.h | 2 +-
src/include/replication/snapbuild.h | 2 +-
7 files changed, 50 insertions(+), 9 deletions(-)
diff --git a/contrib/test_decoding/expected/rewrite.out b/contrib/test_decoding/expected/rewrite.out
index b30999c436..8b97f15f6f 100644
--- a/contrib/test_decoding/expected/rewrite.out
+++ b/contrib/test_decoding/expected/rewrite.out
@@ -162,3 +162,24 @@ DROP TABLE IF EXISTS replication_example;
DROP FUNCTION iamalongfunction();
DROP FUNCTION exec(text);
DROP ROLE regress_justforcomments;
+-- make sure custodian cleans up files
+CHECKPOINT;
+DO $$
+DECLARE
+ snaps_removed bool;
+ loops int := 0;
+BEGIN
+ LOOP
+ snaps_removed := count(*) = 0 FROM pg_ls_logicalsnapdir();
+ IF snaps_removed OR loops > 120 * 100 THEN EXIT; END IF;
+ PERFORM pg_sleep(0.01);
+ loops := loops + 1;
+ END LOOP;
+END
+$$;
+SELECT count(*) = 0 FROM pg_ls_logicalsnapdir();
+ ?column?
+----------
+ t
+(1 row)
+
diff --git a/contrib/test_decoding/sql/rewrite.sql b/contrib/test_decoding/sql/rewrite.sql
index 62dead3a9b..d268fa559a 100644
--- a/contrib/test_decoding/sql/rewrite.sql
+++ b/contrib/test_decoding/sql/rewrite.sql
@@ -105,3 +105,20 @@ DROP TABLE IF EXISTS replication_example;
DROP FUNCTION iamalongfunction();
DROP FUNCTION exec(text);
DROP ROLE regress_justforcomments;
+
+-- make sure custodian cleans up files
+CHECKPOINT;
+DO $$
+DECLARE
+ snaps_removed bool;
+ loops int := 0;
+BEGIN
+ LOOP
+ snaps_removed := count(*) = 0 FROM pg_ls_logicalsnapdir();
+ IF snaps_removed OR loops > 120 * 100 THEN EXIT; END IF;
+ PERFORM pg_sleep(0.01);
+ loops := loops + 1;
+ END LOOP;
+END
+$$;
+SELECT count(*) = 0 FROM pg_ls_logicalsnapdir();
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index fb4c860bde..382e59f723 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -76,12 +76,12 @@
#include "port/atomics.h"
#include "port/pg_iovec.h"
#include "postmaster/bgwriter.h"
+#include "postmaster/custodian.h"
#include "postmaster/startup.h"
#include "postmaster/walwriter.h"
#include "replication/logical.h"
#include "replication/origin.h"
#include "replication/slot.h"
-#include "replication/snapbuild.h"
#include "replication/walreceiver.h"
#include "replication/walsender.h"
#include "storage/bufmgr.h"
@@ -6997,10 +6997,12 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags)
{
CheckPointRelationMap();
CheckPointReplicationSlots();
- CheckPointSnapBuild();
CheckPointLogicalRewriteHeap();
CheckPointReplicationOrigin();
+ /* tasks offloaded to custodian */
+ RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, (Datum) 0);
+
/* Write out all dirty data in SLRUs and the main buffer pool */
TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags);
CheckpointStats.ckpt_write_t = GetCurrentTimestamp();
diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c
index 98bb9efcfd..4e0ce1f7b3 100644
--- a/src/backend/postmaster/custodian.c
+++ b/src/backend/postmaster/custodian.c
@@ -25,6 +25,7 @@
#include "pgstat.h"
#include "postmaster/custodian.h"
#include "postmaster/interrupt.h"
+#include "replication/snapbuild.h"
#include "storage/bufmgr.h"
#include "storage/condition_variable.h"
#include "storage/fd.h"
@@ -70,6 +71,7 @@ struct cust_task_funcs_entry
* whether the task is already enqueued.
*/
static const struct cust_task_funcs_entry cust_task_functions[] = {
+ {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL},
{INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */
};
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index 829c568112..6b403a2bb4 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -2036,14 +2036,13 @@ SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path)
/*
* Remove all serialized snapshots that are not required anymore because no
- * slot can need them. This doesn't actually have to run during a checkpoint,
- * but it's a convenient point to schedule this.
+ * slot can need them.
*
- * NB: We run this during checkpoints even if logical decoding is disabled so
- * we cleanup old slots at some point after it got disabled.
+ * NB: We run this even if logical decoding is disabled so we cleanup old slots
+ * at some point after it got disabled.
*/
void
-CheckPointSnapBuild(void)
+RemoveOldSerializedSnapshots(void)
{
XLogRecPtr cutoff;
XLogRecPtr redo;
diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h
index 73d0bc5f02..ab6d4283b9 100644
--- a/src/include/postmaster/custodian.h
+++ b/src/include/postmaster/custodian.h
@@ -18,7 +18,7 @@
*/
typedef enum CustodianTask
{
- FAKE_TASK, /* placeholder until we have a real task */
+ CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS,
NUM_CUSTODIAN_TASKS, /* new tasks go above */
INVALID_CUSTODIAN_TASK
diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h
index f49b941b53..5f1ba3842c 100644
--- a/src/include/replication/snapbuild.h
+++ b/src/include/replication/snapbuild.h
@@ -57,7 +57,7 @@ struct ReorderBuffer;
struct xl_heap_new_cid;
struct xl_running_xacts;
-extern void CheckPointSnapBuild(void);
+extern void RemoveOldSerializedSnapshots(void);
extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *reorder,
TransactionId xmin_horizon, XLogRecPtr start_lsn,
--
2.25.1
--x+6KMIRAuhnl3hBn
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v19-0003-Move-removal-of-old-logical-rewrite-mapping-file.patch"
^ permalink raw reply [nested|flat] 19+ messages in thread
* RE: [Proposal] Add foreign-server health checks infrastructure
@ 2023-01-10 16:26 Hayato Kuroda (Fujitsu) <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-01-10 16:26 UTC (permalink / raw)
To: 'Tom Lane' <[email protected]>; +Cc: 'vignesh C' <[email protected]>; [email protected] <[email protected]>; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Kyotaro Horiguchi <[email protected]>
Dear tom,
> I think that it's a really bad idea to require postgres_fdw.sql
> to have two expected-files: that will be a maintenance nightmare.
> Please put whatever it is that needs a variant expected-file
> into its own, hopefully very small and seldom-changed, test script.
> Or rethink whether you really need a test case that has
> platform-dependent output.
Thank you for giving the suggestion. I agreed your saying and modifed that.
I added new functions on the libpq and postgres-fdw layer that check whether the
checking works well or not. In the test, at first, the platform is checked and
the checking function is called only when it is supported.
An alternative approach is that PQCanConncheck() can be combined with PQConncheck().
This can reduce the libpq function, but we must define another returned value to
the function like -2. I was not sure which approach was better.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/octet-stream] v23-0001-Add-PQConncheck-to-libpq.patch (5.0K, ../../TYAPR01MB586612A29BAF468042F04B20F5FF9@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-v23-0001-Add-PQConncheck-to-libpq.patch)
download | inline diff:
From 23a71fe4818f45d86b6b8931f9ca6cd915291a82 Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Tue, 1 Nov 2022 09:13:20 +0000
Subject: [PATCH v23 1/3] Add PQConncheck to libpq
This new libpq 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.
---
doc/src/sgml/libpq.sgml | 44 ++++++++++++++++++++++++
src/interfaces/libpq/exports.txt | 2 ++
src/interfaces/libpq/fe-misc.c | 57 ++++++++++++++++++++++++++++++++
src/interfaces/libpq/libpq-fe.h | 4 +++
4 files changed, 107 insertions(+)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0e7ae70c70..f7cff16f7f 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2679,6 +2679,50 @@ 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.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="libpq-PQCanConncheck">
+ <term><function>PQCanConncheck</function><indexterm><primary>PQCanConncheck</primary></indexterm></term>
+ <listitem>
+ <para>
+ Returns the status of the socket.
+
+<synopsis>
+int PQCanConncheck(void);
+</synopsis>
+ </para>
+
+ <para>
+ This function checks whether <xref linkend="libpq-PQConncheck"/> is
+ available or not on this platform. <xref linkend="libpq-PQCanConncheck"/>
+ returns <literal>0</literal> if the function is supported, otherwise
+ returns <literal>-1</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index e8bcc88370..b90d178047 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..2888b94c0d 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -1218,6 +1218,63 @@ PQenv2encoding(void)
return encoding;
}
+/*
+ * Helper function for PQconncheck().
+ *
+ * Return >0 if opposite side seems to be disconnected.
+ */
+static int
+pqConncheck_internal(int sock)
+{
+#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+ struct pollfd input_fd;
+ int errflags = POLLHUP | POLLERR | POLLNVAL;
+
+ input_fd.fd = sock;
+ input_fd.events = POLLRDHUP | errflags;
+ input_fd.revents = 0;
+
+ poll(&input_fd, 1, 0);
+
+ 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.
+ */
+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 0 if this can use PQConncheck(), otherwise -1.
+ */
+int
+PQCanConncheck(void)
+{
+#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+ return 0;
+#else
+ return -1;
+#endif
+}
#ifdef ENABLE_NLS
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index f3d9220496..4771ee1124 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] v23-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch (9.1K, ../../TYAPR01MB586612A29BAF468042F04B20F5FF9@TYAPR01MB5866.jpnprd01.prod.outlook.com/3-v23-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch)
download | inline diff:
From 6323b6ab8918929a7fa87b27de760ed262f63cbc Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Tue, 1 Nov 2022 09:13:42 +0000
Subject: [PATCH v23 2/3] 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/connection.c | 107 ++++++++++++++++++
.../postgres_fdw/postgres_fdw--1.0--1.1.sql | 15 +++
doc/src/sgml/postgres-fdw.sgml | 68 +++++++++++
3 files changed, 190 insertions(+)
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index ed75ce3f79..b2691139b2 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -86,6 +86,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);
@@ -116,6 +119,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
@@ -1862,3 +1866,106 @@ 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;
+
+ /* quick exit if connection cache has been not 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))
+ {
+ /*
+ * Foreign server might be down, so throw ereport(WARNING).
+ */
+ ForeignServer *server;
+
+ server = GetForeignServer(entry->serverid);
+ ereport(WARNING,
+ (errcode(ERRCODE_CONNECTION_FAILURE),
+ errmsg("could not connect to server \"%s\"",
+ server->servername),
+ errdetail("Socket close is detected."),
+ errhint("Plsease check the health of the server.")));
+
+ result = false;
+ }
+ }
+ }
+
+ 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/postgres_fdw--1.0--1.1.sql b/contrib/postgres_fdw/postgres_fdw--1.0--1.1.sql
index ed4ca378d4..b337760bfa 100644
--- a/contrib/postgres_fdw/postgres_fdw--1.0--1.1.sql
+++ b/contrib/postgres_fdw/postgres_fdw--1.0--1.1.sql
@@ -18,3 +18,18 @@ CREATE FUNCTION postgres_fdw_disconnect_all ()
RETURNS bool
AS 'MODULE_PATHNAME'
LANGUAGE C STRICT PARALLEL RESTRICTED;
+
+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/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 78f2d7d8d5..bfd1dbae93 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -825,6 +825,74 @@ 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 that are 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 a 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_foreign_servers(false);
+ 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 remote connections that are 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 a lastly 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_foreign_servers(false);
+ 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_verify_connection_states_all();
+ postgres_fdw_verify_connection_states_all
+-------------------------------------------
+ t
+</screen>
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect2>
--
2.27.0
[application/octet-stream] v23-0003-add-test.patch (4.8K, ../../TYAPR01MB586612A29BAF468042F04B20F5FF9@TYAPR01MB5866.jpnprd01.prod.outlook.com/4-v23-0003-add-test.patch)
download | inline diff:
From 8e004ecf5c1972d0caab445485d7d5db0ed663c0 Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Wed, 21 Sep 2022 06:52:23 +0000
Subject: [PATCH v23 3/3] add test
Note that this test has two comparison files. Alternative comparison file is needed
because some platforms like Windows cannot check the status of the socket.
---
.../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 c0267a99d2..5359ae6f0f 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -11804,3 +11804,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..4e99360a31 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
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [Proposal] Add foreign-server health checks infrastructure
@ 2023-01-10 16:38 Ted Yu <[email protected]>
parent: Hayato Kuroda (Fujitsu) <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Ted Yu @ 2023-01-10 16:38 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; +Cc: Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected] <[email protected]>; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Kyotaro Horiguchi <[email protected]>
On Tue, Jan 10, 2023 at 8:26 AM Hayato Kuroda (Fujitsu) <
[email protected]> wrote:
> Dear tom,
>
> > I think that it's a really bad idea to require postgres_fdw.sql
> > to have two expected-files: that will be a maintenance nightmare.
> > Please put whatever it is that needs a variant expected-file
> > into its own, hopefully very small and seldom-changed, test script.
> > Or rethink whether you really need a test case that has
> > platform-dependent output.
>
> Thank you for giving the suggestion. I agreed your saying and modifed that.
>
> I added new functions on the libpq and postgres-fdw layer that check
> whether the
> checking works well or not. In the test, at first, the platform is checked
> and
> the checking function is called only when it is supported.
>
> An alternative approach is that PQCanConncheck() can be combined with
> PQConncheck().
> This can reduce the libpq function, but we must define another returned
> value to
> the function like -2. I was not sure which approach was better.
>
> Best Regards,
> Hayato Kuroda
> FUJITSU LIMITED
>
Hi,
+ /* quick exit if connection cache has been not initialized yet. */
been not initialized -> not been initialized
+
(errcode(ERRCODE_CONNECTION_FAILURE),
+ errmsg("could not connect
to server \"%s\"",
Currently each server which is not connected would log a warning.
Is it better to concatenate names for such servers and log one line ? This
would be cleaner when there are multiple such servers.
Cheers
^ permalink raw reply [nested|flat] 19+ messages in thread
* RE: [Proposal] Add foreign-server health checks infrastructure
@ 2023-01-11 07:37 Hayato Kuroda (Fujitsu) <[email protected]>
parent: Ted Yu <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-01-11 07:37 UTC (permalink / raw)
To: 'Ted Yu' <[email protected]>; +Cc: Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected] <[email protected]>; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Kyotaro Horiguchi <[email protected]>
Dear Ted,
Thank you for reviewing! PSA new version.
> + /* quick exit if connection cache has been not initialized yet. */
>
> been not initialized -> not been initialized
Fixed.
> + (errcode(ERRCODE_CONNECTION_FAILURE),
> + errmsg("could not connect to server \"%s\"",
>
> Currently each server which is not connected would log a warning.
> Is it better to concatenate names for such servers and log one line ? This would be cleaner when there are multiple such servers.
Sounds good, fixed as you said. The following shows the case that two disconnections
are detected by postgres_fdw_verify_connection_states_all().
```
postgres=*# select postgres_fdw_verify_connection_states_all ();
WARNING: could not connect to server "my_external_server2", "my_external_server"
DETAIL: Socket close is detected.
HINT: Plsease check the health of server.
postgres_fdw_verify_connection_states_all
-------------------------------------------
f
(1 row)
```
Currently, the name of servers is concatenated without doing unique checks. IIUC
a backend process cannot connect to the same foreign server by using different
user mapping, so there is no possibility that the same name appears twice.
If the user mapping is altered in the transaction, the cache entry is invalidated
and will not be checked.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/octet-stream] v24-0001-Add-PQConncheck-and-PQCanConncheck-to-libpq.patch (5.1K, ../../TYAPR01MB5866413814861A5016839506F5FC9@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-v24-0001-Add-PQConncheck-and-PQCanConncheck-to-libpq.patch)
download | inline diff:
From 600d450274a2f4aa3f3e29c4eb0e1d0e128de2ff Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Tue, 1 Nov 2022 09:13:20 +0000
Subject: [PATCH v24 1/3] 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 | 44 ++++++++++++++++++++++++
src/interfaces/libpq/exports.txt | 2 ++
src/interfaces/libpq/fe-misc.c | 57 ++++++++++++++++++++++++++++++++
src/interfaces/libpq/libpq-fe.h | 4 +++
4 files changed, 107 insertions(+)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0e7ae70c70..f7cff16f7f 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2679,6 +2679,50 @@ 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.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="libpq-PQCanConncheck">
+ <term><function>PQCanConncheck</function><indexterm><primary>PQCanConncheck</primary></indexterm></term>
+ <listitem>
+ <para>
+ Returns the status of the socket.
+
+<synopsis>
+int PQCanConncheck(void);
+</synopsis>
+ </para>
+
+ <para>
+ This function checks whether <xref linkend="libpq-PQConncheck"/> is
+ available or not on this platform. <xref linkend="libpq-PQCanConncheck"/>
+ returns <literal>0</literal> if the function is supported, otherwise
+ returns <literal>-1</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index e8bcc88370..b90d178047 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..2888b94c0d 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -1218,6 +1218,63 @@ PQenv2encoding(void)
return encoding;
}
+/*
+ * Helper function for PQconncheck().
+ *
+ * Return >0 if opposite side seems to be disconnected.
+ */
+static int
+pqConncheck_internal(int sock)
+{
+#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+ struct pollfd input_fd;
+ int errflags = POLLHUP | POLLERR | POLLNVAL;
+
+ input_fd.fd = sock;
+ input_fd.events = POLLRDHUP | errflags;
+ input_fd.revents = 0;
+
+ poll(&input_fd, 1, 0);
+
+ 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.
+ */
+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 0 if this can use PQConncheck(), otherwise -1.
+ */
+int
+PQCanConncheck(void)
+{
+#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+ return 0;
+#else
+ return -1;
+#endif
+}
#ifdef ENABLE_NLS
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index f3d9220496..4771ee1124 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] v24-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch (9.5K, ../../TYAPR01MB5866413814861A5016839506F5FC9@TYAPR01MB5866.jpnprd01.prod.outlook.com/3-v24-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch)
download | inline diff:
From 3005525d67175deec5cbb01af7565cc7690a5097 Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Tue, 1 Nov 2022 09:13:42 +0000
Subject: [PATCH v24 2/3] 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.
fdw: concat string
---
contrib/postgres_fdw/connection.c | 124 ++++++++++++++++++
.../postgres_fdw/postgres_fdw--1.0--1.1.sql | 15 +++
doc/src/sgml/postgres-fdw.sgml | 68 ++++++++++
3 files changed, 207 insertions(+)
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index ed75ce3f79..5f345ca170 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -86,6 +86,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);
@@ -116,6 +119,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
@@ -1862,3 +1866,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/postgres_fdw--1.0--1.1.sql b/contrib/postgres_fdw/postgres_fdw--1.0--1.1.sql
index ed4ca378d4..b337760bfa 100644
--- a/contrib/postgres_fdw/postgres_fdw--1.0--1.1.sql
+++ b/contrib/postgres_fdw/postgres_fdw--1.0--1.1.sql
@@ -18,3 +18,18 @@ CREATE FUNCTION postgres_fdw_disconnect_all ()
RETURNS bool
AS 'MODULE_PATHNAME'
LANGUAGE C STRICT PARALLEL RESTRICTED;
+
+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/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 78f2d7d8d5..bfd1dbae93 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -825,6 +825,74 @@ 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 that are 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 a 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_foreign_servers(false);
+ 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 remote connections that are 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 a lastly 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_foreign_servers(false);
+ 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_verify_connection_states_all();
+ postgres_fdw_verify_connection_states_all
+-------------------------------------------
+ t
+</screen>
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect2>
--
2.27.0
[application/octet-stream] v24-0003-add-test.patch (4.6K, ../../TYAPR01MB5866413814861A5016839506F5FC9@TYAPR01MB5866.jpnprd01.prod.outlook.com/4-v24-0003-add-test.patch)
download | inline diff:
From 4a2178608b9cce47fe2c90649e40d004ffdb34b0 Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Wed, 21 Sep 2022 06:52:23 +0000
Subject: [PATCH v24 3/3] 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 c0267a99d2..ee1c0eb96b 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -11804,3 +11804,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
^ permalink raw reply [nested|flat] 19+ messages in thread
* RE: [Proposal] Add foreign-server health checks infrastructure
@ 2023-01-11 10:04 Hayato Kuroda (Fujitsu) <[email protected]>
parent: Hayato Kuroda (Fujitsu) <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-01-11 10:04 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; 'Ted Yu' <[email protected]>; +Cc: Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected] <[email protected]>; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Kyotaro Horiguchi <[email protected]>
Dear hackers,
I was not sure, but the cfbot could not be accepted the previous version.
I made the patch again from HEAD(5f6401) without any changes,
so I did not count up the version number.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/octet-stream] v24-0001-Add-PQConncheck-and-PQCanConncheck-to-libpq.patch (5.1K, ../../TYAPR01MB5866C20861167C9CBEDF0A36F5FC9@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-v24-0001-Add-PQConncheck-and-PQCanConncheck-to-libpq.patch)
download | inline diff:
From 24dfa629d2f7682fd80278d86d87f7a92f0c0687 Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Tue, 1 Nov 2022 09:13:20 +0000
Subject: [PATCH v24 1/3] 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 | 44 ++++++++++++++++++++++++
src/interfaces/libpq/exports.txt | 2 ++
src/interfaces/libpq/fe-misc.c | 57 ++++++++++++++++++++++++++++++++
src/interfaces/libpq/libpq-fe.h | 4 +++
4 files changed, 107 insertions(+)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0e7ae70c70..f7cff16f7f 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2679,6 +2679,50 @@ 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.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="libpq-PQCanConncheck">
+ <term><function>PQCanConncheck</function><indexterm><primary>PQCanConncheck</primary></indexterm></term>
+ <listitem>
+ <para>
+ Returns the status of the socket.
+
+<synopsis>
+int PQCanConncheck(void);
+</synopsis>
+ </para>
+
+ <para>
+ This function checks whether <xref linkend="libpq-PQConncheck"/> is
+ available or not on this platform. <xref linkend="libpq-PQCanConncheck"/>
+ returns <literal>0</literal> if the function is supported, otherwise
+ returns <literal>-1</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index e8bcc88370..b90d178047 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..2888b94c0d 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -1218,6 +1218,63 @@ PQenv2encoding(void)
return encoding;
}
+/*
+ * Helper function for PQconncheck().
+ *
+ * Return >0 if opposite side seems to be disconnected.
+ */
+static int
+pqConncheck_internal(int sock)
+{
+#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+ struct pollfd input_fd;
+ int errflags = POLLHUP | POLLERR | POLLNVAL;
+
+ input_fd.fd = sock;
+ input_fd.events = POLLRDHUP | errflags;
+ input_fd.revents = 0;
+
+ poll(&input_fd, 1, 0);
+
+ 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.
+ */
+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 0 if this can use PQConncheck(), otherwise -1.
+ */
+int
+PQCanConncheck(void)
+{
+#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+ return 0;
+#else
+ return -1;
+#endif
+}
#ifdef ENABLE_NLS
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index f3d9220496..4771ee1124 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] v24-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch (9.5K, ../../TYAPR01MB5866C20861167C9CBEDF0A36F5FC9@TYAPR01MB5866.jpnprd01.prod.outlook.com/3-v24-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch)
download | inline diff:
From f1e93bdee4be01b58623cddfc94c4f383d5ec98e Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Tue, 1 Nov 2022 09:13:42 +0000
Subject: [PATCH v24 2/3] 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/connection.c | 124 ++++++++++++++++++
.../postgres_fdw/postgres_fdw--1.0--1.1.sql | 15 +++
doc/src/sgml/postgres-fdw.sgml | 68 ++++++++++
3 files changed, 207 insertions(+)
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index ed75ce3f79..5f345ca170 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -86,6 +86,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);
@@ -116,6 +119,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
@@ -1862,3 +1866,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/postgres_fdw--1.0--1.1.sql b/contrib/postgres_fdw/postgres_fdw--1.0--1.1.sql
index ed4ca378d4..b337760bfa 100644
--- a/contrib/postgres_fdw/postgres_fdw--1.0--1.1.sql
+++ b/contrib/postgres_fdw/postgres_fdw--1.0--1.1.sql
@@ -18,3 +18,18 @@ CREATE FUNCTION postgres_fdw_disconnect_all ()
RETURNS bool
AS 'MODULE_PATHNAME'
LANGUAGE C STRICT PARALLEL RESTRICTED;
+
+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/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 78f2d7d8d5..bfd1dbae93 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -825,6 +825,74 @@ 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 that are 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 a 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_foreign_servers(false);
+ 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 remote connections that are 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 a lastly 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_foreign_servers(false);
+ 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_verify_connection_states_all();
+ postgres_fdw_verify_connection_states_all
+-------------------------------------------
+ t
+</screen>
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect2>
--
2.27.0
[application/octet-stream] v24-0003-add-test.patch (4.6K, ../../TYAPR01MB5866C20861167C9CBEDF0A36F5FC9@TYAPR01MB5866.jpnprd01.prod.outlook.com/4-v24-0003-add-test.patch)
download | inline diff:
From 615ffeac27733039aff876548d9b370285bc25b7 Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Wed, 21 Sep 2022 06:52:23 +0000
Subject: [PATCH v24 3/3] 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 c0267a99d2..ee1c0eb96b 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -11804,3 +11804,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
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [Proposal] Add foreign-server health checks infrastructure
@ 2023-01-20 10:41 Katsuragi Yuta <[email protected]>
parent: Hayato Kuroda (Fujitsu) <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Katsuragi Yuta @ 2023-01-20 10:41 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; +Cc: 'Ted Yu' <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected]; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected]; Kyotaro Horiguchi <[email protected]>
On 2023-01-11 19:04, Hayato Kuroda (Fujitsu) wrote:
> Dear hackers,
>
> I was not sure, but the cfbot could not be accepted the previous
> version.
> I made the patch again from HEAD(5f6401) without any changes,
> so I did not count up the version number.
>
> Best Regards,
> Hayato Kuroda
> FUJITSU LIMITED
Hi,
Thanks for the patch!
I read the patch v24. These are my comments. Please check.
## v24-0001-Add-PQConncheck-and-PQCanConncheck-to-libpq.patch
+ <varlistentry id="libpq-PQCanConncheck">
+
<term><function>PQCanConncheck</function><indexterm><primary>PQCanConncheck</primary></indexterm></term>
+ <listitem>
+ <para>
+ Returns the status of the socket.
Is this description right? I think this description is for
PQConncheck. Something like "Checks whether PQConncheck is
available on this platform." seems better.
+/* Check whether the postgres server is still alive or not */
+extern int PQConncheck(PGconn *conn);
+extern int PQCanConncheck(void);
Should the names of these functions be in the form of PQconnCheck?
Not PQConncheck (c.f. The section of fe-misc.c in libpq-fe.h).
## v24-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch
+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);
This patch adds new functions to postgres_fdw for PostgreSQL 16.
So, I think it is necessary to update the version of postgres_fdw (v1.1
to v1.2).
+ <term><function>postgres_fdw_verify_connection_states_all() returns
boolean</function></term>
+ <listitem>
+ <para>
+ This function checks the status of remote connections that are
established
+ by <filename>postgres_fdw</filename> from the local session to
the foreign
+ servers. This check is performed by polling the socket and allows
It seems better to add a description that states this function
checks all the connections. Like the description of
postgres_fdw_disconnect_all(). For example, "This function
checks the status of 'all the' remote connections..."?
regards,
--
Katsuragi Yuta
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 19+ messages in thread
* RE: [Proposal] Add foreign-server health checks infrastructure
@ 2023-01-21 12:03 Hayato Kuroda (Fujitsu) <[email protected]>
parent: Katsuragi Yuta <[email protected]>
0 siblings, 2 replies; 19+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-01-21 12:03 UTC (permalink / raw)
To: 'Katsuragi Yuta' <[email protected]>; +Cc: 'Ted Yu' <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected] <[email protected]>; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Kyotaro Horiguchi <[email protected]>
Dear Katsuragi-san,
Thank you for reviewing! PSA new patch set.
> ## v24-0001-Add-PQConncheck-and-PQCanConncheck-to-libpq.patch
> + <varlistentry id="libpq-PQCanConncheck">
> +
> <term><function>PQCanConncheck</function><indexterm><primary>PQCan
> Conncheck</primary></indexterm></term>
> + <listitem>
> + <para>
> + Returns the status of the socket.
>
> Is this description right? I think this description is for
> PQConncheck. Something like "Checks whether PQConncheck is
> available on this platform." seems better.
It might be copy-and-paste error. Thanks for reporting.
According to other parts, the sentence should be started like "Returns ...".
So I followed the style and did cosmetic change.
> +/* Check whether the postgres server is still alive or not */
> +extern int PQConncheck(PGconn *conn);
> +extern int PQCanConncheck(void);
>
> Should the names of these functions be in the form of PQconnCheck?
> Not PQConncheck (c.f. The section of fe-misc.c in libpq-fe.h).
Agreed, fixed.
> ## v24-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch
> +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);
>
> This patch adds new functions to postgres_fdw for PostgreSQL 16.
> So, I think it is necessary to update the version of postgres_fdw (v1.1
> to v1.2).
I checked postgres_fdw commit log, and it seemed that the version was
updated when SQL functions are added. Fixed.
> + <term><function>postgres_fdw_verify_connection_states_all() returns
> boolean</function></term>
> + <listitem>
> + <para>
> + This function checks the status of remote connections that are
> established
> + by <filename>postgres_fdw</filename> from the local session to
> the foreign
> + servers. This check is performed by polling the socket and allows
>
> It seems better to add a description that states this function
> checks all the connections. Like the description of
> postgres_fdw_disconnect_all(). For example, "This function
> checks the status of 'all the' remote connections..."?
I checked the docs and fixed. Moreover, some inconsistent statements were
also fixed.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/octet-stream] v25-0001-Add-PQConnCheck-and-PQCanConnCheck-to-libpq.patch (5.0K, ../../TYAPR01MB5866E17F6C47AB41DCB8ADBEF5CA9@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-v25-0001-Add-PQConnCheck-and-PQCanConnCheck-to-libpq.patch)
download | inline diff:
From 257217ac713c4a695a2f7d2b6384ee9e4c29da5d Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Tue, 1 Nov 2022 09:13:20 +0000
Subject: [PATCH v25 1/3] 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 | 57 ++++++++++++++++++++++++++++++++
src/interfaces/libpq/libpq-fe.h | 4 +++
4 files changed, 103 insertions(+)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0e7ae70c70..4821898f84 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 PQConnCeck(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.
+ </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>0</literal> if the function is supported, otherwise returns
+ <literal>-1</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..267cfdcb48 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..184fda90d3 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -1218,6 +1218,63 @@ PQenv2encoding(void)
return encoding;
}
+/*
+ * Helper function for PQconnCheck().
+ *
+ * Return >0 if opposite side seems to be disconnected.
+ */
+static int
+pqConnCheck_internal(int sock)
+{
+#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+ struct pollfd input_fd;
+ int errflags = POLLHUP | POLLERR | POLLNVAL;
+
+ input_fd.fd = sock;
+ input_fd.events = POLLRDHUP | errflags;
+ input_fd.revents = 0;
+
+ poll(&input_fd, 1, 0);
+
+ 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.
+ */
+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 0 if this can use PQConnCheck(), otherwise -1.
+ */
+int
+PQCanConnCheck(void)
+{
+#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+ return 0;
+#else
+ return -1;
+#endif
+}
#ifdef ENABLE_NLS
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index f3d9220496..053fad0760 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] v25-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch (10.6K, ../../TYAPR01MB5866E17F6C47AB41DCB8ADBEF5CA9@TYAPR01MB5866.jpnprd01.prod.outlook.com/3-v25-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch)
download | inline diff:
From 4ad0426668fcc73988878b1d2217365cd8c9d22c Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Tue, 1 Nov 2022 09:13:42 +0000
Subject: [PATCH v25 2/3] 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 ++++++++++++++++++
.../postgres_fdw/postgres_fdw--1.1--1.2.sql | 19 +++
contrib/postgres_fdw/postgres_fdw.control | 2 +-
doc/src/sgml/postgres-fdw.sgml | 69 ++++++++++
5 files changed, 214 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 ed75ce3f79..a6702169d4 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -86,6 +86,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);
@@ -116,6 +119,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
@@ -1862,3 +1866,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/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] v25-0003-add-test.patch (4.6K, ../../TYAPR01MB5866E17F6C47AB41DCB8ADBEF5CA9@TYAPR01MB5866.jpnprd01.prod.outlook.com/4-v25-0003-add-test.patch)
download | inline diff:
From eceaeabbb8faa319f2f3b5589e994214e04a4663 Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Wed, 21 Sep 2022 06:52:23 +0000
Subject: [PATCH v25 3/3] 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
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [Proposal] Add foreign-server health checks infrastructure
@ 2023-01-21 12:33 Ted Yu <[email protected]>
parent: Hayato Kuroda (Fujitsu) <[email protected]>
1 sibling, 1 reply; 19+ messages in thread
From: Ted Yu @ 2023-01-21 12:33 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; +Cc: Katsuragi Yuta <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected] <[email protected]>; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Kyotaro Horiguchi <[email protected]>
On Sat, Jan 21, 2023 at 4:03 AM Hayato Kuroda (Fujitsu) <
[email protected]> wrote:
> Dear Katsuragi-san,
>
> Thank you for reviewing! PSA new patch set.
>
> > ## v24-0001-Add-PQConncheck-and-PQCanConncheck-to-libpq.patch
> > + <varlistentry id="libpq-PQCanConncheck">
> > +
> > <term><function>PQCanConncheck</function><indexterm><primary>PQCan
> > Conncheck</primary></indexterm></term>
> > + <listitem>
> > + <para>
> > + Returns the status of the socket.
> >
> > Is this description right? I think this description is for
> > PQConncheck. Something like "Checks whether PQConncheck is
> > available on this platform." seems better.
>
> It might be copy-and-paste error. Thanks for reporting.
> According to other parts, the sentence should be started like "Returns
> ...".
> So I followed the style and did cosmetic change.
>
> > +/* Check whether the postgres server is still alive or not */
> > +extern int PQConncheck(PGconn *conn);
> > +extern int PQCanConncheck(void);
> >
> > Should the names of these functions be in the form of PQconnCheck?
> > Not PQConncheck (c.f. The section of fe-misc.c in libpq-fe.h).
>
> Agreed, fixed.
>
> > ## v24-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch
> > +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);
> >
> > This patch adds new functions to postgres_fdw for PostgreSQL 16.
> > So, I think it is necessary to update the version of postgres_fdw (v1.1
> > to v1.2).
>
> I checked postgres_fdw commit log, and it seemed that the version was
> updated when SQL functions are added. Fixed.
>
> > + <term><function>postgres_fdw_verify_connection_states_all() returns
> > boolean</function></term>
> > + <listitem>
> > + <para>
> > + This function checks the status of remote connections that are
> > established
> > + by <filename>postgres_fdw</filename> from the local session to
> > the foreign
> > + servers. This check is performed by polling the socket and allows
> >
> > It seems better to add a description that states this function
> > checks all the connections. Like the description of
> > postgres_fdw_disconnect_all(). For example, "This function
> > checks the status of 'all the' remote connections..."?
>
> I checked the docs and fixed. Moreover, some inconsistent statements were
> also fixed.
>
> Best Regards,
> Hayato Kuroda
> FUJITSU LIMITED
>
> Hi,
For v25-0001-Add-PQConnCheck-and-PQCanConnCheck-to-libpq.patch ,
`pqConnCheck_internal` only has one caller which is quite short.
Can pqConnCheck_internal and PQConnCheck be merged into one func ?
+int
+PQCanConnCheck(void)
It seems the return value should be of bool type.
Cheers
^ permalink raw reply [nested|flat] 19+ messages in thread
* RE: [Proposal] Add foreign-server health checks infrastructure
@ 2023-01-23 04:57 Hayato Kuroda (Fujitsu) <[email protected]>
parent: Hayato Kuroda (Fujitsu) <[email protected]>
1 sibling, 0 replies; 19+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-01-23 04:57 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; +Cc: 'Ted Yu' <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected] <[email protected]>; Önder Kalacı <[email protected]>; 'Katsuragi Yuta' <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Kyotaro Horiguchi <[email protected]>
> Thank you for reviewing! PSA new patch set.
Sorry, I missed the updated file in the patch. New version will be posted soon.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
^ permalink raw reply [nested|flat] 19+ messages in thread
* RE: [Proposal] Add foreign-server health checks infrastructure
@ 2023-01-23 05:40 Hayato Kuroda (Fujitsu) <[email protected]>
parent: Ted Yu <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-01-23 05:40 UTC (permalink / raw)
To: 'Ted Yu' <[email protected]>; +Cc: Katsuragi Yuta <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected] <[email protected]>; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Kyotaro Horiguchi <[email protected]>
Dear Ted,
Thanks for reviewing! PSA new version.
> For v25-0001-Add-PQConnCheck-and-PQCanConnCheck-to-libpq.patch , `pqConnCheck_internal` only has one caller which is quite short.
> Can pqConnCheck_internal and PQConnCheck be merged into one func ?
I divided the function for feature expandability. Currently it works on linux platform,
but the limitation should be removed in future and internal function will be longer.
Therefore I want to keep this style.
> +int
> +PQCanConnCheck(void)
>
> It seems the return value should be of bool type.
I slightly changed the returned value like true/false. But IIUC libpq functions
cannot define as "bool" datatype. E.g. PQconnectionNeedsPassword() returns true/false,
but the function is defined as int.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/octet-stream] v26-0001-Add-PQConnCheck-and-PQCanConnCheck-to-libpq.patch (5.0K, ../../TYAPR01MB58668DD17F19CAFC97A3134EF5C89@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-v26-0001-Add-PQConnCheck-and-PQCanConnCheck-to-libpq.patch)
download | inline diff:
From 3ad9df1e9c3a6a21268e9599b5b941b047843c72 Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Tue, 1 Nov 2022 09:13:20 +0000
Subject: [PATCH v26 1/3] 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 | 57 ++++++++++++++++++++++++++++++++
src/interfaces/libpq/libpq-fe.h | 4 +++
4 files changed, 103 insertions(+)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0e7ae70c70..086b97340d 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.
+ </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..267cfdcb48 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..445a1da721 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -1218,6 +1218,63 @@ PQenv2encoding(void)
return encoding;
}
+/*
+ * Helper function for PQconnCheck().
+ *
+ * Return >0 if opposite side seems to be disconnected.
+ */
+static int
+pqConnCheck_internal(int sock)
+{
+#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+ struct pollfd input_fd;
+ int errflags = POLLHUP | POLLERR | POLLNVAL;
+
+ input_fd.fd = sock;
+ input_fd.events = POLLRDHUP | errflags;
+ input_fd.revents = 0;
+
+ poll(&input_fd, 1, 0);
+
+ 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.
+ */
+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..053fad0760 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] v26-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch (11.1K, ../../TYAPR01MB58668DD17F19CAFC97A3134EF5C89@TYAPR01MB5866.jpnprd01.prod.outlook.com/3-v26-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch)
download | inline diff:
From c97328f7c6ca16ed4fb5437abd2e2ce8d2d7c27d Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Tue, 1 Nov 2022 09:13:42 +0000
Subject: [PATCH v26 2/3] 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 ed75ce3f79..4dc26f48bf 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -86,6 +86,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);
@@ -116,6 +119,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
@@ -1862,3 +1866,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] v26-0003-add-test.patch (4.6K, ../../TYAPR01MB58668DD17F19CAFC97A3134EF5C89@TYAPR01MB5866.jpnprd01.prod.outlook.com/4-v26-0003-add-test.patch)
download | inline diff:
From e772dff2ea94ce696c26754f18bd27ef49ab4862 Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Wed, 21 Sep 2022 06:52:23 +0000
Subject: [PATCH v26 3/3] 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
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [Proposal] Add foreign-server health checks infrastructure
@ 2023-01-25 08:15 Katsuragi Yuta <[email protected]>
parent: Hayato Kuroda (Fujitsu) <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Katsuragi Yuta @ 2023-01-25 08:15 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; +Cc: 'Ted Yu' <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected]; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected]; Kyotaro Horiguchi <[email protected]>
On 2023-01-23 14:40, Hayato Kuroda (Fujitsu) wrote:
> Dear Ted,
>
> Thanks for reviewing! PSA new version.
>
>> For v25-0001-Add-PQConnCheck-and-PQCanConnCheck-to-libpq.patch ,
>> `pqConnCheck_internal` only has one caller which is quite short.
>> Can pqConnCheck_internal and PQConnCheck be merged into one func ?
>
> I divided the function for feature expandability. Currently it works
> on linux platform,
> but the limitation should be removed in future and internal function
> will be longer.
> Therefore I want to keep this style.
>
>> +int
>> +PQCanConnCheck(void)
>>
>> It seems the return value should be of bool type.
>
> I slightly changed the returned value like true/false. But IIUC libpq
> functions
> cannot define as "bool" datatype. E.g. PQconnectionNeedsPassword()
> returns true/false,
> but the function is defined as int.
>
> Best Regards,
> Hayato Kuroda
> FUJITSU LIMITED
Thank you for updating the patch!
+/* Check whether the postgres server is still alive or not */
+extern int PQConnCheck(PGconn *conn);
+extern int PQCanConnCheck(void);
Aren't these PQconnCheck and PQcanConnCheck? I think the first letter
following 'PQ' should be lower case.
regards.
--
Katsuragi Yuta
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 19+ messages in thread
* RE: [Proposal] Add foreign-server health checks infrastructure
@ 2023-01-25 11:07 Hayato Kuroda (Fujitsu) <[email protected]>
parent: Katsuragi Yuta <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-01-25 11:07 UTC (permalink / raw)
To: 'Katsuragi Yuta' <[email protected]>; +Cc: 'Ted Yu' <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected] <[email protected]>; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Kyotaro Horiguchi <[email protected]>
Dear Katsuragi-san,
Thank you for reading the patch! PSA new version.
> Thank you for updating the patch!
>
> +/* Check whether the postgres server is still alive or not */
> +extern int PQConnCheck(PGconn *conn);
> +extern int PQCanConnCheck(void);
>
> Aren't these PQconnCheck and PQcanConnCheck? I think the first letter
> following 'PQ' should be lower case.
I cannot find any rules about it, but seems right. Fixed.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/octet-stream] v27-0001-Add-PQconnCheck-and-PQcanConnCheck-to-libpq.patch (5.0K, ../../TYAPR01MB5866969DBAEEEC5AE07591BFF5CE9@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-v27-0001-Add-PQconnCheck-and-PQcanConnCheck-to-libpq.patch)
download | inline diff:
From 0ddb539dacbdf58448f527ef63cbb748dcf390d8 Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Tue, 1 Nov 2022 09:13:20 +0000
Subject: [PATCH v27 1/3] 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 | 57 ++++++++++++++++++++++++++++++++
src/interfaces/libpq/libpq-fe.h | 4 +++
4 files changed, 103 insertions(+)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0e7ae70c70..c8789ecd95 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.
+ </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..49f5ca3479 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -1218,6 +1218,63 @@ PQenv2encoding(void)
return encoding;
}
+/*
+ * Helper function for PQconnCheck().
+ *
+ * Return >0 if opposite side seems to be disconnected.
+ */
+static int
+pqconnCheck_internal(int sock)
+{
+#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+ struct pollfd input_fd;
+ int errflags = POLLHUP | POLLERR | POLLNVAL;
+
+ input_fd.fd = sock;
+ input_fd.events = POLLRDHUP | errflags;
+ input_fd.revents = 0;
+
+ poll(&input_fd, 1, 0);
+
+ 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.
+ */
+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] v27-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch (11.1K, ../../TYAPR01MB5866969DBAEEEC5AE07591BFF5CE9@TYAPR01MB5866.jpnprd01.prod.outlook.com/3-v27-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch)
download | inline diff:
From 4d28e091ae033fd5ae58ee26af9073f01ed3d059 Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Tue, 1 Nov 2022 09:13:42 +0000
Subject: [PATCH v27 2/3] 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 ed75ce3f79..35a11c22e7 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -86,6 +86,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);
@@ -116,6 +119,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
@@ -1862,3 +1866,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] v27-0003-add-test.patch (4.6K, ../../TYAPR01MB5866969DBAEEEC5AE07591BFF5CE9@TYAPR01MB5866.jpnprd01.prod.outlook.com/4-v27-0003-add-test.patch)
download | inline diff:
From b338eeecc5f3810fdf551dd873851e065b5314dd Mon Sep 17 00:00:00 2001
From: "kuroda.hayato%40jp.fujitsu.com" <[email protected]>
Date: Wed, 21 Sep 2022 06:52:23 +0000
Subject: [PATCH v27 3/3] 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
^ permalink raw reply [nested|flat] 19+ messages in thread
* RE: [Proposal] Add foreign-server health checks infrastructure
@ 2023-01-27 03:50 Hayato Kuroda (Fujitsu) <[email protected]>
parent: Hayato Kuroda (Fujitsu) <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-01-27 03:50 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; 'Katsuragi Yuta' <[email protected]>; +Cc: 'Ted Yu' <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected] <[email protected]>; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Kyotaro Horiguchi <[email protected]>
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
^ permalink raw reply [nested|flat] 19+ messages in thread
* RE: [Proposal] Add foreign-server health checks infrastructure
@ 2023-01-27 06:57 Hayato Kuroda (Fujitsu) <[email protected]>
parent: Hayato Kuroda (Fujitsu) <[email protected]>
0 siblings, 2 replies; 19+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-01-27 06:57 UTC (permalink / raw)
To: 'Katsuragi Yuta' <[email protected]>; +Cc: 'Ted Yu' <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected] <[email protected]>; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Kyotaro Horiguchi <[email protected]>
I found cfbot failure, PSA fixed version.
Sorry for noise.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/octet-stream] v29-0003-add-test.patch (4.6K, ../../TYAPR01MB586675882721E646693E13FBF5CC9@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-v29-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 v29 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] v29-0004-add-kqueue-support-for-PQconnCheck-and-PQcanConn.patch (5.3K, ../../TYAPR01MB586675882721E646693E13FBF5CC9@TYAPR01MB5866.jpnprd01.prod.outlook.com/3-v29-0004-add-kqueue-support-for-PQconnCheck-and-PQcanConn.patch)
download | inline diff:
From bc44376b0045a5d718b80079a6ee882c7c7f05e2 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Fri, 27 Jan 2023 03:17:41 +0000
Subject: [PATCH v29 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 | 47 +++++++++++++++++++++++++++++++++-
3 files changed, 59 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..4d493b7091 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -45,6 +45,14 @@
#include <poll.h>
#endif
+/* We use poll(2) if it has POLLRDHUP event, otherwise kqueue(2) */
+#if (!(defined(HAVE_POLL) && defined(POLLRDHUP)) && \
+ defined(HAVE_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"
@@ -1227,6 +1235,7 @@ PQenv2encoding(void)
static int
pqconnCheck_internal(int sock)
{
+ /* We use poll(2) if it has POLLRDHUP event, otherwise kqueue(2) */
#if (defined(HAVE_POLL) && defined(POLLRDHUP))
struct pollfd input_fd;
int errflags = POLLHUP | POLLERR | POLLNVAL;
@@ -1249,6 +1258,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 +1325,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
[application/octet-stream] v29-0001-Add-PQconnCheck-and-PQcanConnCheck-to-libpq.patch (5.3K, ../../TYAPR01MB586675882721E646693E13FBF5CC9@TYAPR01MB5866.jpnprd01.prod.outlook.com/4-v29-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 v29 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] v29-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch (11.0K, ../../TYAPR01MB586675882721E646693E13FBF5CC9@TYAPR01MB5866.jpnprd01.prod.outlook.com/5-v29-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 v29 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
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [Proposal] Add foreign-server health checks infrastructure
@ 2023-02-08 07:10 Katsuragi Yuta <[email protected]>
parent: Hayato Kuroda (Fujitsu) <[email protected]>
1 sibling, 1 reply; 19+ messages in thread
From: Katsuragi Yuta @ 2023-02-08 07:10 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; +Cc: 'Ted Yu' <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected]; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected]; Kyotaro Horiguchi <[email protected]>
On 2023-01-27 15:57, Hayato Kuroda (Fujitsu) wrote:
> I found cfbot failure, PSA fixed version.
> Sorry for noise.
>
> Best Regards,
> Hayato Kuroda
> FUJITSU LIMITED
Hi Kuroda-san,
Thank you for updating the patch! Sorry for the late reply.
0001:
+ while (result < 0 && errno == EINTR);
+
+ if (result < 0)
+ return -1;
this `return -1` is not indented properly.
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.
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?
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?
0004:
Sorry, I have not read 0004. I will read.
[1]:
https://github.com/postgres/postgres/blob/master/doc/src/sgml/postgres-fdw.sgml#L764-L765
regards,
--
Katsuragi Yuta
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [Proposal] Add foreign-server health checks infrastructure
@ 2023-02-09 02:31 Kyotaro Horiguchi <[email protected]>
parent: Hayato Kuroda (Fujitsu) <[email protected]>
1 sibling, 0 replies; 19+ messages in thread
From: Kyotaro Horiguchi @ 2023-02-09 02:31 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
At Fri, 27 Jan 2023 06:57:01 +0000, "Hayato Kuroda (Fujitsu)" <[email protected]> wrote in
> I found cfbot failure, PSA fixed version.
+ 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
I find it quite confusing that we have pqSocketCheck and PQconnCheck,
that does almost the same thing.. Since pqSocketCheck is a static
function, we can modify the function as we like.
I still don't understand why we need pqconnCheck_internal separate
from pqSocketPoll(), and PQconnCheck from pqSocketCheck.
https://www.postgresql.org/message-id/flat/TYAPR01MB58665BF23D38EDF10028DE2AF5299%40TYAPR01MB5866.jp...
> IIUC, pqSocketCheck () calls pqSocketPoll(),
> and in the pqSocketPoll() we poll()'d the POLLIN or POLLOUT event.
> But according to [1], we must wait POLLRDHUP event,
> so we cannot reuse it straightforward.
Yeah, I didn't suggest to use the function as-is. Couldn't we extend
the fucntion by letting it accept end_time = 0 && !forRead &&
!forWrite, not causing side effects?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 19+ messages in thread
* RE: [Proposal] Add foreign-server health checks infrastructure
@ 2023-02-09 14:39 Hayato Kuroda (Fujitsu) <[email protected]>
parent: Katsuragi Yuta <[email protected]>
0 siblings, 2 replies; 19+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-02-09 14:39 UTC (permalink / raw)
To: 'Katsuragi Yuta' <[email protected]>; +Cc: 'Ted Yu' <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected] <[email protected]>; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Kyotaro Horiguchi <[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
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [Proposal] Add foreign-server health checks infrastructure
@ 2023-02-17 08:43 Katsuragi Yuta <[email protected]>
parent: Hayato Kuroda (Fujitsu) <[email protected]>
1 sibling, 0 replies; 19+ messages in thread
From: Katsuragi Yuta @ 2023-02-17 08:43 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; +Cc: 'Ted Yu' <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected]; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected]; Kyotaro Horiguchi <[email protected]>
On 2023-02-09 23:39, Hayato Kuroda (Fujitsu) wrote:
> Dear Katsuragi-san,
>
> Thank you for reviewing! PSA new version patches.
Thank you for updating the patch! These are my comments,
please check.
0001:
Extending pqSocketPoll seems to be a better way because we can
avoid having multiple similar functions. I also would like to hear
horiguchi-san's opinion whether this matches his expectation.
Improvements of pqSocketPoll/pqSocketCheck is discussed in this
thread[1]. I'm concerned with the discussion.
As for the function's name, what do you think about keeping
current name (pqSocketCheck)? pqSocketIsReadable... describes
the functionality very well though.
pqConnCheck seems to be a family of pqReadReady or pqWriteRedy,
so how about placing pqConnCheck below them?
+ * Moreover, when neither forRead nor forWrite is requested and timeout
is
+ * disabled, try to check the health of socket.
Isn't it better to put the comment on how the arguments are
interpreted before the description of return value?
+#if defined(POLLRDHUP)
+ input_fd.events = POLLRDHUP | POLLHUP | POLLNVAL;
...
+ input_fd.events |= POLLERR;
To my understanding, POLLHUP, POLLNVAL and POLLERR are ignored
in event. Are they necessary?
0002:
As for the return value of postgres_fdw_verify_connection_states,
what do you think about returning NULL when connection-checking
is not performed? I think there are two cases 1) ConnectionHash
is not initialized or 2) connection is not found for specified
server name, That is, no entry passes the first if statement below
(case 2)).
```
if (all || entry->serverid == serverid)
{
if (PQconnCheck(entry->conn))
{
```
0004:
I'm wondering if we should add kqueue support in this version,
because adding kqueue support introduces additional things to
be considered. What do you think about focusing on the main
functionality using poll in this patch and going for kqueue
support after this patch?
[1]:
https://www.postgresql.org/message-id/20230209.115009.2229702014236187289.horikyota.ntt%40gmail.com
regards,
--
Katsuragi Yuta
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [Proposal] Add foreign-server health checks infrastructure
@ 2023-02-20 01:23 Peter Smith <[email protected]>
parent: Hayato Kuroda (Fujitsu) <[email protected]>
1 sibling, 0 replies; 19+ messages in thread
From: Peter Smith @ 2023-02-20 01:23 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; +Cc: Katsuragi Yuta <[email protected]>; Ted Yu <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected] <[email protected]>; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Kyotaro Horiguchi <[email protected]>
Here is a code review only for patch v31-0001.
======
General Comment
1.
PQcanConnCheck seemed like a strange API name. Maybe it can have the
same prefix as the other?
e.g.
- PQconnCheck()
- PGconnCheckSupported()
or
- PQconnCheck()
- PGconnCheckable()
======
Commit Message
2.
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.
~
2a.
"status of socket" --> "status of the connection"
~
2b.
"above function" --> "the above function"
======
doc/src/sgml/libpq.sgml
3. PQconnCheck
Returns the health of the socket.
int PQconnCheck(PGconn *conn);
Unlike 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 POLLRDHUP extension to
the poll system call, including Linux. PQconnCheck returns greater
than zero if the remote peer seems to be closed, returns 0 if the
socket is valid, and returns -1 if the connection has been already
invalid or an error is error occurred.
~
3a.
Should these descriptions be referring to the health of the
*connection* rather than the health of the socket?
~
3b.
"has been already invalid" ?? wording
~~~
4. PQcanConnCheck
Returns whether PQconnCheck is available on this platform.
PQcanConnCheck returns 1 if the function is supported, otherwise
returns 0.
~
I thought this should be worded using "true" and "false" same as other
boolean functions on this page.
SUGGESTION
Returns true (1) or false (0) to indicate if the PQconnCheck function
is supported on this platform.
======
src/interfaces/libpq/fe-misc.c
5.
-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);
I was not 100% sure overloading this API is the right thing to do.
Doesn't this introduce a subtle side-effect on some of the existing
callers? e.g. Previously pqWaitTimed would ALWAYS return 0 if
forRead/forWrite were both false. But now other return values like
errors will be possible. Is that OK?
~~~
6. pqSocketPoll
/*
* Check a file descriptor for read and/or write data, possibly waiting.
* If neither forRead nor forWrite are set, immediately return a timeout
* condition (without waiting). Return >0 if condition is met, 0
* if a timeout occurred, -1 if an error or interrupt occurred.
*
* 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.
*/
The new comment "Moreover..." is contrary to the earlier part of the
same comment which already said, "If neither forRead nor forWrite are
set, immediately return a timeout condition (without waiting)."
There might be side-effects to previous/existing callers of this
function (e.g. pqWaitTimed via pqSocketCheck)
~~~
7.
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;
+ }
FYI - I think the new code can be simpler without needing #else by
calling your other new function.
SUGGESTION
if (!forRead && !forWrite)
{
if (!PQcanConnCheck() || end_time != 0)
return 0;
/* Check the connection health when end_time is 0 */
Assert(PQcanConnCheck() && end_time == 0);
#if defined(POLLRDHUP)
input_fd.events = POLLRDHUP | POLLHUP | POLLNVAL;
#endif
}
~~~
8. PQconnCheck
+/*
+ * 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
+}
~
8a.
"can work well" --> "works"
~
8b.
Maybe better to say "true (1)" and "otherwise false (0)"
------
Kind Regards,
Peter Smith.
Fujitsu Australia
^ permalink raw reply [nested|flat] 19+ messages in thread
end of thread, other threads:[~2023-02-20 01:23 UTC | newest]
Thread overview: 19+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-12-06 06:02 [PATCH v19 2/4] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]>
2023-01-10 16:26 RE: [Proposal] Add foreign-server health checks infrastructure Hayato Kuroda (Fujitsu) <[email protected]>
2023-01-10 16:38 ` Re: [Proposal] Add foreign-server health checks infrastructure Ted Yu <[email protected]>
2023-01-11 07:37 ` RE: [Proposal] Add foreign-server health checks infrastructure Hayato Kuroda (Fujitsu) <[email protected]>
2023-01-11 10:04 ` RE: [Proposal] Add foreign-server health checks infrastructure Hayato Kuroda (Fujitsu) <[email protected]>
2023-01-20 10:41 ` Re: [Proposal] Add foreign-server health checks infrastructure Katsuragi Yuta <[email protected]>
2023-01-21 12:03 ` RE: [Proposal] Add foreign-server health checks infrastructure Hayato Kuroda (Fujitsu) <[email protected]>
2023-01-21 12:33 ` Re: [Proposal] Add foreign-server health checks infrastructure Ted Yu <[email protected]>
2023-01-23 05:40 ` RE: [Proposal] Add foreign-server health checks infrastructure Hayato Kuroda (Fujitsu) <[email protected]>
2023-01-25 08:15 ` Re: [Proposal] Add foreign-server health checks infrastructure Katsuragi Yuta <[email protected]>
2023-01-25 11:07 ` RE: [Proposal] Add foreign-server health checks infrastructure Hayato Kuroda (Fujitsu) <[email protected]>
2023-01-27 03:50 ` RE: [Proposal] Add foreign-server health checks infrastructure Hayato Kuroda (Fujitsu) <[email protected]>
2023-01-27 06:57 ` RE: [Proposal] Add foreign-server health checks infrastructure Hayato Kuroda (Fujitsu) <[email protected]>
2023-02-08 07:10 ` Re: [Proposal] Add foreign-server health checks infrastructure Katsuragi Yuta <[email protected]>
2023-02-09 14:39 ` RE: [Proposal] Add foreign-server health checks infrastructure Hayato Kuroda (Fujitsu) <[email protected]>
2023-02-17 08:43 ` Re: [Proposal] Add foreign-server health checks infrastructure Katsuragi Yuta <[email protected]>
2023-02-20 01:23 ` Re: [Proposal] Add foreign-server health checks infrastructure Peter Smith <[email protected]>
2023-02-09 02:31 ` Re: [Proposal] Add foreign-server health checks infrastructure Kyotaro Horiguchi <[email protected]>
2023-01-23 04:57 ` RE: [Proposal] Add foreign-server health checks infrastructure Hayato Kuroda (Fujitsu) <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox