public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 1/8] introduce bsearch_arg
6+ messages / 4 participants
[nested] [flat]
* [PATCH 1/8] introduce bsearch_arg
@ 2021-03-04 23:16 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Tomas Vondra @ 2021-03-04 23:16 UTC (permalink / raw)
---
src/backend/statistics/extended_stats.c | 31 --------------
src/include/port.h | 5 +++
.../statistics/extended_stats_internal.h | 5 ---
src/port/Makefile | 1 +
src/port/bsearch_arg.c | 40 +++++++++++++++++++
5 files changed, 46 insertions(+), 36 deletions(-)
create mode 100644 src/port/bsearch_arg.c
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index a030ea3653..fa42851fd5 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -659,37 +659,6 @@ compare_datums_simple(Datum a, Datum b, SortSupport ssup)
return ApplySortComparator(a, false, b, false, ssup);
}
-/* simple counterpart to qsort_arg */
-void *
-bsearch_arg(const void *key, const void *base, size_t nmemb, size_t size,
- int (*compar) (const void *, const void *, void *),
- void *arg)
-{
- size_t l,
- u,
- idx;
- const void *p;
- int comparison;
-
- l = 0;
- u = nmemb;
- while (l < u)
- {
- idx = (l + u) / 2;
- p = (void *) (((const char *) base) + (idx * size));
- comparison = (*compar) (key, p, arg);
-
- if (comparison < 0)
- u = idx;
- else if (comparison > 0)
- l = idx + 1;
- else
- return (void *) p;
- }
-
- return NULL;
-}
-
/*
* build_attnums_array
* Transforms a bitmap into an array of AttrNumber values.
diff --git a/src/include/port.h b/src/include/port.h
index 227ef4b148..82f63de325 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -508,6 +508,11 @@ typedef int (*qsort_arg_comparator) (const void *a, const void *b, void *arg);
extern void qsort_arg(void *base, size_t nel, size_t elsize,
qsort_arg_comparator cmp, void *arg);
+extern void *bsearch_arg(const void *key, const void *base,
+ size_t nmemb, size_t size,
+ int (*compar) (const void *, const void *, void *),
+ void *arg);
+
/* port/chklocale.c */
extern int pg_get_encoding_from_locale(const char *ctype, bool write_message);
diff --git a/src/include/statistics/extended_stats_internal.h b/src/include/statistics/extended_stats_internal.h
index c849bd57c0..a0a3cf5b0f 100644
--- a/src/include/statistics/extended_stats_internal.h
+++ b/src/include/statistics/extended_stats_internal.h
@@ -85,11 +85,6 @@ extern int multi_sort_compare_dims(int start, int end, const SortItem *a,
extern int compare_scalars_simple(const void *a, const void *b, void *arg);
extern int compare_datums_simple(Datum a, Datum b, SortSupport ssup);
-extern void *bsearch_arg(const void *key, const void *base,
- size_t nmemb, size_t size,
- int (*compar) (const void *, const void *, void *),
- void *arg);
-
extern AttrNumber *build_attnums_array(Bitmapset *attrs, int *numattrs);
extern SortItem *build_sorted_items(int numrows, int *nitems, HeapTuple *rows,
diff --git a/src/port/Makefile b/src/port/Makefile
index e41b005c4f..52dbf5783f 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -40,6 +40,7 @@ LIBS += $(PTHREAD_LIBS)
OBJS = \
$(LIBOBJS) \
$(PG_CRC32C_OBJS) \
+ bsearch_arg.o \
chklocale.o \
erand48.o \
inet_net_ntop.o \
diff --git a/src/port/bsearch_arg.c b/src/port/bsearch_arg.c
new file mode 100644
index 0000000000..d24dc4b7c4
--- /dev/null
+++ b/src/port/bsearch_arg.c
@@ -0,0 +1,40 @@
+/*
+ * bsearch_arg.c: bsearch variant with a user-supplied pointer
+ *
+ * src/port/bsearch_arg.c
+ */
+
+
+#include "c.h"
+
+
+/* simple counterpart to qsort_arg */
+void *
+bsearch_arg(const void *key, const void *base, size_t nmemb, size_t size,
+ int (*compar) (const void *, const void *, void *),
+ void *arg)
+{
+ size_t l,
+ u,
+ idx;
+ const void *p;
+ int comparison;
+
+ l = 0;
+ u = nmemb;
+ while (l < u)
+ {
+ idx = (l + u) / 2;
+ p = (void *) (((const char *) base) + (idx * size));
+ comparison = (*compar) (key, p, arg);
+
+ if (comparison < 0)
+ u = idx;
+ else if (comparison > 0)
+ l = idx + 1;
+ else
+ return (void *) p;
+ }
+
+ return NULL;
+}
--
2.26.2
--------------AC3FFB734F56C5F52D422EFC
Content-Type: text/x-patch; charset=UTF-8;
name="0002-Pass-all-scan-keys-to-BRIN-consistent-funct-20210305.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="0002-Pass-all-scan-keys-to-BRIN-consistent-funct-20210305.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 6+ messages in thread
* RE: [Proposal] Add foreign-server health checks infrastructure
@ 2023-02-20 06:42 Hayato Kuroda (Fujitsu) <[email protected]>
0 siblings, 2 replies; 6+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-02-20 06:42 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.
> 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.
I checked the thread and seems correct. I can post +1 to the thread.
And the modification will be automatically reflected to the feature
if we use the same function, I thought.
> As for the function's name, what do you think about keeping
> current name (pqSocketCheck)? pqSocketIsReadable... describes
> the functionality very well though.
No objection, I can keep the shorter name.
> pqConnCheck seems to be a family of pqReadReady or pqWriteRedy,
> so how about placing pqConnCheck below them?
Moved.
> + * 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?
I read man poll(3) again, and I found that these event is ignored when
it sets to the events attribute. So removed.
> 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))
> {
> ```
I think in that case we can follow postgres_fdw_disconnect().
About postgres_fdw_disconnect(), if the given server_name does not exist,
an error is reported. This is a current behavior so I want to keep it.
Besides, I added the description to document.
> 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?
I think it is better because it can keep patches smaller. So I stopped attaching 0004.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/octet-stream] v32-0001-Add-PQconnCheck-and-PQconnCheckable-to-libpq.patch (6.7K, ../../TYAPR01MB58669C95604A0EB7BCC1B58CF5A49@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-v32-0001-Add-PQconnCheck-and-PQconnCheckable-to-libpq.patch)
download | inline diff:
From 9d8dbbf978cf681daf13f6c3bc7768709d6fa5d9 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Fri, 27 Jan 2023 03:17:18 +0000
Subject: [PATCH v32 1/3] Add PQconnCheck and PQconnCheckable to libpq
PQconnCheck() function allows to check the status of the 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.
PQconnCheckable() checks whether the above function is available or not.
---
doc/src/sgml/libpq.sgml | 38 ++++++++++++++++++++
src/interfaces/libpq/exports.txt | 2 ++
src/interfaces/libpq/fe-misc.c | 62 ++++++++++++++++++++++++++------
src/interfaces/libpq/libpq-fe.h | 4 +++
4 files changed, 96 insertions(+), 10 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0e7ae70c70..e5e2662996 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2679,6 +2679,44 @@ 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 connection.
+
+<synopsis>
+int PQconnCheck(PGconn *conn);
+</synopsis>
+ </para>
+
+ <para>
+ This function check the health of the connection. Unlike <xref linkend="libpq-PQstatus"/>,
+ this check is performed by polling the corresponding 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 already been closed or an error has occurred.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="libpq-PQconnCheckable">
+ <term><function>PQconnCheckable</function><indexterm><primary>PQconnCheckable</primary></indexterm></term>
+ <listitem>
+ <para>
+ Returns true (1) or false (0) to indicate if the PQconnCheck function
+ is supported on this platform.
+
+<synopsis>
+int PQconnCheckable(void);
+</synopsis>
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index e8bcc88370..a06dea9acd 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
+PQconnCheckable 188
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 3653a1a8a6..4311d1d21c 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 pqSocketCheck(PGconn *conn, int forRead,
+ int forWrite, time_t end_time);
static int pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time);
/*
@@ -1027,9 +1027,39 @@ pqWriteReady(PGconn *conn)
return pqSocketCheck(conn, 0, 1, (time_t) 0);
}
+/*
+ * 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 pqSocketCheck(conn, 0, 0, (time_t) 0);
+}
+
+/*
+ * Check whether PQconnCheck() works well on this platform.
+ *
+ * Returns true (1) if this can use PQconnCheck(), otherwise false (0).
+ */
+int
+PQconnCheckable(void)
+{
+#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+ return true;
+#else
+ return false;
+#endif
+}
+
/*
* Checks a socket, using poll or select, for data to be read, written,
- * or both. Returns >0 if one or more conditions are met, 0 if it timed
+ * or both. Moreover, when neither forRead nor forWrite is requested and
+ * timeout is disabled, try to check the health of socket.
+ *
+ * Returns >0 if one or more conditions are met, 0 if it timed
* out, -1 if an error occurred.
*
* If SSL is in use, the SSL buffer is checked prior to checking the socket
@@ -1076,9 +1106,13 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
/*
* 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.
+ * When neither forRead nor forWrite are set and timeout is disabled,
+ *
+ * - If the timeout is disabled, try to check the health of the socket
+ * - Otherwise this immediately returns 0
+ *
+ * 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).
@@ -1088,14 +1122,23 @@ 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;
+ {
+ if (!PQconnCheckable() || end_time != 0)
+ return 0;
+
+ /* Check the connection health when end_time is 0 */
+ Assert(PQconnCheckable() && end_time == 0);
+#if defined(POLLRDHUP)
+ input_fd.events = POLLRDHUP;
+#endif
+ }
input_fd.fd = sock;
- input_fd.events = POLLERR;
+ input_fd.events |= POLLERR;
input_fd.revents = 0;
if (forRead)
@@ -1218,7 +1261,6 @@ PQenv2encoding(void)
return encoding;
}
-
#ifdef ENABLE_NLS
static void
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index f3d9220496..e1bd0cd7b7 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 PQconnCheckable(void);
+
/* === in fe-auth.c === */
extern char *PQencryptPassword(const char *passwd, const char *user);
--
2.27.0
[application/octet-stream] v32-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch (11.8K, ../../TYAPR01MB58669C95604A0EB7BCC1B58CF5A49@TYAPR01MB5866.jpnprd01.prod.outlook.com/3-v32-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch)
download | inline diff:
From 53136332097686c28b09455165ff1cfc0d4f9c43 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Fri, 27 Jan 2023 03:17:30 +0000
Subject: [PATCH v32 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 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 | 75 ++++++++++
6 files changed, 228 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..36801fd978 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(PQconnCheckable());
+}
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..e2b3d63422 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -826,6 +826,81 @@ 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. If no foreign server with the given name is
+ found, an error is reported. 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] v32-0003-add-test.patch (5.3K, ../../TYAPR01MB58669C95604A0EB7BCC1B58CF5A49@TYAPR01MB5866.jpnprd01.prod.outlook.com/4-v32-0003-add-test.patch)
download | inline diff:
From ddd7cae776b041310b7abaced3975ffa1f24c017 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Fri, 27 Jan 2023 03:17:36 +0000
Subject: [PATCH v32 3/3] 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
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [Proposal] Add foreign-server health checks infrastructure
@ 2023-02-21 07:11 Peter Smith <[email protected]>
parent: Hayato Kuroda (Fujitsu) <[email protected]>
1 sibling, 1 reply; 6+ messages in thread
From: Peter Smith @ 2023-02-21 07:11 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; +Cc: [email protected] <[email protected]>
Here are some review comments for v32-0001.
======
Commit message
1.
PQconnCheck() function allows to check the status of the 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.
~
(missed fix from previous review)
"status of the socket" --> "status of the connection"
====
doc/src/sgml/libpq.sgml
2. PQconnCheck
+ <para>
+ This function check the health of the connection. Unlike <xref
linkend="libpq-PQstatus"/>,
+ this check is performed by polling the corresponding 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 already been closed or an error has occurred.
+ </para>
"check the health" --> "checks the health"
~~~
3. PQcanConnCheck
+ <para>
+ Returns true (1) or false (0) to indicate if the PQconnCheck function
+ is supported on this platform.
Should the reference to PQconnCheck be a link as it previously was?
======
src/interfaces/libpq/fe-misc.c
4. PQconnCheck
+/*
+ * 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 pqSocketCheck(conn, 0, 0, (time_t) 0);
+}
I'm confused. This comment says =0 means connection is valid. But the
pqSocketCheck comment says =0 means it timed out.
So those two function comments don't seem compatible
~~~
5. PQconnCheckable
+/*
+ * Check whether PQconnCheck() works well on this platform.
+ *
+ * Returns true (1) if this can use PQconnCheck(), otherwise false (0).
+ */
+int
+PQconnCheckable(void)
+{
+#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+ return true;
+#else
+ return false;
+#endif
+}
Why say "works well"? IMO it either works or doesn't work – there is no "well".
SUGGESTION1
Check whether PQconnCheck() works on this platform.
SUGGESTION2
Check whether PQconnCheck() can work on this platform.
~~~
6. pqSocketCheck
/*
* Checks a socket, using poll or select, for data to be read, written,
- * or both. Returns >0 if one or more conditions are met, 0 if it timed
+ * or both. Moreover, when neither forRead nor forWrite is requested and
+ * timeout is disabled, try to check the health of socket.
+ *
+ * Returns >0 if one or more conditions are met, 0 if it timed
* out, -1 if an error occurred.
*
* If SSL is in use, the SSL buffer is checked prior to checking the socket
~
See review comment #4. (e.g. This says =0 if it timed out).
~~~
7. pqSocketPoll
+ * When neither forRead nor forWrite are set and timeout is disabled,
+ *
+ * - If the timeout is disabled, try to check the health of the socket
+ * - Otherwise this immediately returns 0
+ *
+ * Return >0 if condition is met, 0 if a timeout occurred, -1 if an error
+ * or interrupt occurred.
Don't say "and timeout is disabled," because it clashes with the 1st
bullet which also says "- If the timeout is disabled,".
------
Kind Regards,
Peter Smith.
Fujitsu Australia
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [Proposal] Add foreign-server health checks infrastructure
@ 2023-02-21 07:59 Katsuragi Yuta <[email protected]>
parent: Hayato Kuroda (Fujitsu) <[email protected]>
1 sibling, 1 reply; 6+ messages in thread
From: Katsuragi Yuta @ 2023-02-21 07:59 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]>
Hi Kuroda-san,
Thank you for updating the patch!
On 2023-02-20 15:42, Hayato Kuroda (Fujitsu) wrote:
> Dear Katsuragi-san,
>
> Thank you for reviewing! PSA new version.
I rethought the pqSocketPoll part. Current interpretation of
arguments seems a little bit confusing because a specific pattern
of arguments has a different meaning. What do you think about
introducing a new argument like `int forConnCheck`? This seems
straightforward and readable.
>> 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))
>> {
>> ```
>
> I think in that case we can follow postgres_fdw_disconnect().
> About postgres_fdw_disconnect(), if the given server_name does not
> exist,
> an error is reported.
Yes, I think this error is fine.
I think there are cases where the given server name does exist,
however the connection check is not performed (does not pass the
first if statement above). 1) a case where the server name exist,
however an open connection to the specified server is not found.
2) case where connection for specified server is invalidated.
3) case where there is not ConnectionHash entry for the specified
server. Current implementation returns true in that case, however
the check is not performed. Suppose the checking mechanism is
supported on the platform, it does not seem reasonable to return
true or false when the check is not performed. What do you think?
regards,
--
Katsuragi Yuta
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 6+ messages in thread
* RE: [Proposal] Add foreign-server health checks infrastructure
@ 2023-02-22 12:33 Hayato Kuroda (Fujitsu) <[email protected]>
parent: Peter Smith <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-02-22 12:33 UTC (permalink / raw)
To: 'Peter Smith' <[email protected]>; +Cc: [email protected] <[email protected]>
Dear Peter,
Thank you for reviewing! PSA new version.
> 1.
> PQconnCheck() function allows to check the status of the 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.
>
> ~
>
> (missed fix from previous review)
>
> "status of the socket" --> "status of the connection"
Sorry, fixed.
> ====
> doc/src/sgml/libpq.sgml
>
> 2. PQconnCheck
> + <para>
> + This function check the health of the connection. Unlike <xref
> linkend="libpq-PQstatus"/>,
> + this check is performed by polling the corresponding 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 already been closed or an error has occurred.
> + </para>
>
> "check the health" --> "checks the health"
Fixed.
> 3. PQcanConnCheck
>
> + <para>
> + Returns true (1) or false (0) to indicate if the PQconnCheck function
> + is supported on this platform.
>
> Should the reference to PQconnCheck be a link as it previously was?
Right, fixed.
> src/interfaces/libpq/fe-misc.c
>
> 4. PQconnCheck
>
> +/*
> + * 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 pqSocketCheck(conn, 0, 0, (time_t) 0);
> +}
>
> I'm confused. This comment says =0 means connection is valid. But the
> pqSocketCheck comment says =0 means it timed out.
>
> So those two function comments don't seem compatible
Added further descriptions atop pqSocketCheck() and pqSocketPoll().
Is it helpful to understand?
> 5. PQconnCheckable
>
> +/*
> + * Check whether PQconnCheck() works well on this platform.
> + *
> + * Returns true (1) if this can use PQconnCheck(), otherwise false (0).
> + */
> +int
> +PQconnCheckable(void)
> +{
> +#if (defined(HAVE_POLL) && defined(POLLRDHUP))
> + return true;
> +#else
> + return false;
> +#endif
> +}
>
> Why say "works well"? IMO it either works or doesn't work – there is no "well".
>
> SUGGESTION1
> Check whether PQconnCheck() works on this platform.
>
> SUGGESTION2
> Check whether PQconnCheck() can work on this platform.
I choose 2.
> 6. pqSocketCheck
>
> /*
> * Checks a socket, using poll or select, for data to be read, written,
> - * or both. Returns >0 if one or more conditions are met, 0 if it timed
> + * or both. Moreover, when neither forRead nor forWrite is requested and
> + * timeout is disabled, try to check the health of socket.
> + *
> + * Returns >0 if one or more conditions are met, 0 if it timed
> * out, -1 if an error occurred.
> *
> * If SSL is in use, the SSL buffer is checked prior to checking the socket
>
> ~
>
> See review comment #4. (e.g. This says =0 if it timed out).
Descriptions were added.
> 7. pqSocketPoll
>
> + * When neither forRead nor forWrite are set and timeout is disabled,
> + *
> + * - If the timeout is disabled, try to check the health of the socket
> + * - Otherwise this immediately returns 0
> + *
> + * Return >0 if condition is met, 0 if a timeout occurred, -1 if an error
> + * or interrupt occurred.
>
> Don't say "and timeout is disabled," because it clashes with the 1st
> bullet which also says "- If the timeout is disabled,".
This comments were reworded.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/octet-stream] v33-0001-Add-PQconnCheck-and-PQconnCheckable-to-libpq.patch (8.7K, ../../TYAPR01MB586664F5ED2128E572EA95B0F5AA9@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-v33-0001-Add-PQconnCheck-and-PQconnCheckable-to-libpq.patch)
download | inline diff:
From 234b238343cb7e155f57128ee3e3a0ae46e836b9 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Fri, 27 Jan 2023 03:17:18 +0000
Subject: [PATCH v33 1/3] Add PQconnCheck and PQconnCheckable to libpq
PQconnCheck() function allows to check the status of the connection 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.
PQconnCheckable() checks whether the above function is available or not.
---
doc/src/sgml/libpq.sgml | 38 ++++++++++++++++
src/interfaces/libpq/exports.txt | 2 +
src/interfaces/libpq/fe-misc.c | 78 +++++++++++++++++++++++++-------
src/interfaces/libpq/libpq-fe.h | 4 ++
4 files changed, 106 insertions(+), 16 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0e7ae70c70..afa1fe6731 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2679,6 +2679,44 @@ 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 connection.
+
+<synopsis>
+int PQconnCheck(PGconn *conn);
+</synopsis>
+ </para>
+
+ <para>
+ This function checks the health of the connection. Unlike <xref linkend="libpq-PQstatus"/>,
+ this check is performed by polling the corresponding 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 already been closed or an error has occurred.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="libpq-PQconnCheckable">
+ <term><function>PQconnCheckable</function><indexterm><primary>PQconnCheckable</primary></indexterm></term>
+ <listitem>
+ <para>
+ Returns true (1) or false (0) to indicate if the <xref linkend="libpq-PQconnCheck"/>
+ function is supported on this platform.
+
+<synopsis>
+int PQconnCheckable(void);
+</synopsis>
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index e8bcc88370..a06dea9acd 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
+PQconnCheckable 188
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 3653a1a8a6..1a3478dc7e 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -53,9 +53,10 @@
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 pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time);
+static int pqSocketCheck(PGconn *conn, int forRead,
+ int forWrite, int forConnCheck, time_t end_time);
+static int pqSocketPoll(int sock, int forRead,
+ int forWrite, int forConnCheck, time_t end_time);
/*
* PQlibVersion: return the libpq version number
@@ -993,7 +994,7 @@ pqWaitTimed(int forRead, int forWrite, PGconn *conn, time_t finish_time)
{
int result;
- result = pqSocketCheck(conn, forRead, forWrite, finish_time);
+ result = pqSocketCheck(conn, forRead, forWrite, 0, finish_time);
if (result < 0)
return -1; /* errorMessage is already set */
@@ -1014,7 +1015,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 pqSocketCheck(conn, 1, 0, 0, (time_t) 0);
}
/*
@@ -1024,19 +1025,52 @@ pqReadReady(PGconn *conn)
int
pqWriteReady(PGconn *conn)
{
- return pqSocketCheck(conn, 0, 1, (time_t) 0);
+ return pqSocketCheck(conn, 0, 1, 0, (time_t) 0);
+}
+
+/*
+ * 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 pqSocketCheck(conn, 0, 0, 1, (time_t) 0);
+}
+
+/*
+ * Check whether PQconnCheck() can work on this platform.
+ *
+ * Returns true (1) if this can use PQconnCheck(), otherwise false (0).
+ */
+int
+PQconnCheckable(void)
+{
+#if (defined(HAVE_POLL) && defined(POLLRDHUP))
+ return true;
+#else
+ return false;
+#endif
}
/*
* Checks a socket, using poll or select, for data to be read, written,
- * or both. Returns >0 if one or more conditions are met, 0 if it timed
- * out, -1 if an error occurred.
+ * or both. Moreover, this function can check the health of socket on some
+ * limited platforms if end_time is 0.
+ *
+ * Returns >0 if one or more conditions are met, 0 if it timed out, -1 if an
+ * error occurred. Note that if 0 is returned and forConnCheck is requested, it
+ * means that the socket has not matched POLLRDHUP event and the socket has
+ * still survived.
*
* If SSL is in use, the SSL buffer is checked prior to checking the socket
* for read data directly.
*/
static int
-pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
+pqSocketCheck(PGconn *conn, int forRead,
+ int forWrite, int forConnCheck, time_t end_time)
{
int result;
@@ -1059,7 +1093,7 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
/* We will retry as long as we get EINTR */
do
- result = pqSocketPoll(conn->sock, forRead, forWrite, end_time);
+ result = pqSocketPoll(conn->sock, forRead, forWrite, forConnCheck, end_time);
while (result < 0 && SOCK_ERRNO == EINTR);
if (result < 0)
@@ -1076,15 +1110,20 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
/*
* 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.
+ * Moreover, this function can check the health of socket on some limited
+ * platforms if end_time is 0.
+ *
+ * If neither forRead, forWrite nor forConnCheck 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. Note that if 0 is
+ * returned and forConnCheck is requested, it means that the socket has not
+ * matched POLLRDHUP event and the socket has still survived.
*
* Timeout is infinite if end_time is -1. Timeout is immediate (no blocking)
* if end_time is 0 (or indeed, any time before now).
*/
static int
-pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time)
+pqSocketPoll(int sock, int forRead, int forWrite, int forConnCheck, time_t end_time)
{
/* We use poll(2) if available, otherwise select(2) */
#ifdef HAVE_POLL
@@ -1092,7 +1131,10 @@ pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time)
int timeout_ms;
if (!forRead && !forWrite)
- return 0;
+ {
+ if (!forConnCheck || !PQconnCheckable() || end_time != 0)
+ return 0;
+ }
input_fd.fd = sock;
input_fd.events = POLLERR;
@@ -1102,6 +1144,11 @@ pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time)
input_fd.events |= POLLIN;
if (forWrite)
input_fd.events |= POLLOUT;
+#if defined(POLLRDHUP)
+ if (forConnCheck)
+ input_fd.events |= POLLRDHUP;
+#endif
+
/* Compute appropriate timeout interval */
if (end_time == ((time_t) -1))
@@ -1218,7 +1265,6 @@ PQenv2encoding(void)
return encoding;
}
-
#ifdef ENABLE_NLS
static void
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index f3d9220496..e1bd0cd7b7 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 PQconnCheckable(void);
+
/* === in fe-auth.c === */
extern char *PQencryptPassword(const char *passwd, const char *user);
--
2.27.0
[application/octet-stream] v33-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch (12.4K, ../../TYAPR01MB586664F5ED2128E572EA95B0F5AA9@TYAPR01MB5866.jpnprd01.prod.outlook.com/3-v33-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch)
download | inline diff:
From 74b77d9d0ce0ca04db8381a96b9a78ecc617c447 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Fri, 27 Jan 2023 03:17:30 +0000
Subject: [PATCH v33 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 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 | 155 ++++++++++++++++++
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 | 76 +++++++++
6 files changed, 253 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..0231d23968 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, bool *checked);
/*
* Get a PGconn which can be used to execute queries on the remote PostgreSQL
@@ -1832,3 +1836,154 @@ 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.
+ *
+ * checked will be set to true if PQconnCheck() is called at least once.
+ *
+ * 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, bool *checked)
+{
+ HASH_SEQ_STATUS scan;
+ ConnCacheEntry *entry;
+ bool all = !OidIsValid(serverid);
+ bool result = true;
+ StringInfoData str;
+
+ *checked = false;
+
+ 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);
+ }
+
+ /* Set a flag to notify the caller */
+ *checked = true;
+ }
+ }
+
+ /* 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 check had not been done.
+ *
+ * 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;
+ bool result,
+ checked = false;
+
+ /* 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);
+
+ result = verify_cached_connections(server->serverid, &checked);
+
+ /* Return the result if checking function was called, otherwise NULL */
+ if (checked)
+ PG_RETURN_BOOL(result);
+ else
+ PG_RETURN_NULL();
+}
+
+/*
+ * 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)
+{
+ bool result,
+ checked = false;
+
+ /* quick exit if connection cache has not been initialized yet. */
+ if (!ConnectionHash)
+ PG_RETURN_NULL();
+
+ result = verify_cached_connections(InvalidOid, &checked);
+
+ /* Return the result if checking function was called, otherwise NULL */
+ if (checked)
+ PG_RETURN_BOOL(result);
+ else
+ PG_RETURN_NULL();
+}
+
+Datum
+postgres_fdw_can_verify_connection_states(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_BOOL(PQconnCheckable());
+}
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..d598228b62 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -826,6 +826,82 @@ 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 a connection to the
+ specified foreign server has not been established yet. If no foreign
+ server with the given name is found, an error is reported. 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] v33-0003-add-test.patch (5.3K, ../../TYAPR01MB586664F5ED2128E572EA95B0F5AA9@TYAPR01MB5866.jpnprd01.prod.outlook.com/4-v33-0003-add-test.patch)
download | inline diff:
From 6a04df56995cfa4a7e586e5cf29569fd42c87f3e Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Fri, 27 Jan 2023 03:17:36 +0000
Subject: [PATCH v33 3/3] 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
^ permalink raw reply [nested|flat] 6+ messages in thread
* RE: [Proposal] Add foreign-server health checks infrastructure
@ 2023-02-22 12:34 Hayato Kuroda (Fujitsu) <[email protected]>
parent: Katsuragi Yuta <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-02-22 12:34 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! New patch set can be available on [1].
> I rethought the pqSocketPoll part. Current interpretation of
> arguments seems a little bit confusing because a specific pattern
> of arguments has a different meaning. What do you think about
> introducing a new argument like `int forConnCheck`? This seems
> straightforward and readable.
I think it may be better, so fixed.
But now we must consider another thing - will we support combination of conncheck
and {read|write} or timeout? Especially about timeout, if someone calls pqSocketPoll()
with forConnCheck = 1 and end_time = -1, the process may be stuck because it waits
till socket peer closed connection.
Currently the forConnCheck can be specified with other request, but timeout must be zero.
> I think there are cases where the given server name does exist,
> however the connection check is not performed (does not pass the
> first if statement above). 1) a case where the server name exist,
> however an open connection to the specified server is not found.
> 2) case where connection for specified server is invalidated.
> 3) case where there is not ConnectionHash entry for the specified
> server. Current implementation returns true in that case, however
> the check is not performed. Suppose the checking mechanism is
> supported on the platform, it does not seem reasonable to return
> true or false when the check is not performed. What do you think?
Thank you for detailed explanation. I agreed your opinion and modified like that.
While making the patch, I come up with idea that postgres_fdw_verify_connection_states*
returns NULL if PQconnCheck() cannot work on this platform. This can be done by
adding PQconnCheckable(). It may be reasonable because the checking is not really
done on this environment. How do you think?
[1]: https://www.postgresql.org/message-id/TYAPR01MB586664F5ED2128E572EA95B0F5AA9%40TYAPR01MB5866.jpnprd0...
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2023-02-22 12:34 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-04 23:16 [PATCH 1/8] introduce bsearch_arg Tomas Vondra <[email protected]>
2023-02-20 06:42 RE: [Proposal] Add foreign-server health checks infrastructure Hayato Kuroda (Fujitsu) <[email protected]>
2023-02-21 07:11 ` Re: [Proposal] Add foreign-server health checks infrastructure Peter Smith <[email protected]>
2023-02-22 12:33 ` RE: [Proposal] Add foreign-server health checks infrastructure Hayato Kuroda (Fujitsu) <[email protected]>
2023-02-21 07:59 ` Re: [Proposal] Add foreign-server health checks infrastructure Katsuragi Yuta <[email protected]>
2023-02-22 12:34 ` 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