public inbox for [email protected]
help / color / mirror / Atom feedFrom: [email protected] <[email protected]>
To: 'Shinya Kato' <[email protected]>
To: 'Zhihong Yu' <[email protected]>
Cc: [email protected] <[email protected]>
Subject: RE: [Proposal] Add foreign-server health checks infrastructure
Date: Mon, 29 Nov 2021 09:57:22 +0000
Message-ID: <TYAPR01MB5866E0A4F9365002D8A7736FF5669@TYAPR01MB5866.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <TYAPR01MB586615734CA7C814364CBA55F5619@TYAPR01MB5866.jpnprd01.prod.outlook.com>
References: <TYAPR01MB58662809E678253B90E82CE5F5889@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<[email protected]>
<TYAPR01MB58662CD4FD98AA475B3D10F9F59B9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<[email protected]>
<TYAPR01MB58661992162BA7708191EC79F5619@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<TYAPR01MB586615734CA7C814364CBA55F5619@TYAPR01MB5866.jpnprd01.prod.outlook.com>
Dear Hackers,
I attached new version that is based from Zhihong's comments.
And I newly attached a doc patch. I think the infrastructure part is no longer WIP.
Could you review them?
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/x-zip-compressed] 2021129.zip (7.2K, ../TYAPR01MB5866E0A4F9365002D8A7736FF5669@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-2021129.zip)
download
[application/octet-stream] v04_0001_add_checking_infrastracture.patch (11.9K, ../TYAPR01MB5866E0A4F9365002D8A7736FF5669@TYAPR01MB5866.jpnprd01.prod.outlook.com/3-v04_0001_add_checking_infrastracture.patch)
download | inline diff:
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index aac10165ec..0c4d61a34c 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -269,6 +269,7 @@ CopyGetData(CopyFromState cstate, void *databuf, int minread, int maxread)
readmessage:
HOLD_CANCEL_INTERRUPTS();
+ HOLD_CHECKING_REMOTE_SERVERS_INTERRUPTS();
pq_startmsgread();
mtype = pq_getbyte();
if (mtype == EOF)
@@ -300,6 +301,7 @@ CopyGetData(CopyFromState cstate, void *databuf, int minread, int maxread)
ereport(ERROR,
(errcode(ERRCODE_CONNECTION_FAILURE),
errmsg("unexpected EOF on client connection with an open transaction")));
+ RESUME_CHECKING_REMOTE_SERVERS_INTERRUPTS();
RESUME_CANCEL_INTERRUPTS();
/* ... and process it */
switch (mtype)
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index e07cc57431..eb9c4f3f05 100644
--- a/src/backend/foreign/foreign.c
+++ b/src/backend/foreign/foreign.c
@@ -26,7 +26,11 @@
#include "utils/memutils.h"
#include "utils/rel.h"
#include "utils/syscache.h"
+#include "utils/timeout.h"
+/* for checking remote servers */
+int remote_servers_connection_check_interval = 0;
+static CheckingRemoteServersCallbackItem *fdw_callbacks = NULL;
/*
* GetForeignDataWrapper - look up the foreign-data wrapper by OID.
@@ -836,3 +840,51 @@ GetExistingLocalJoinPath(RelOptInfo *joinrel)
}
return NULL;
}
+
+/*
+ * Register callbacks for checking remote servers.
+ *
+ * This function is intended for use by FDW extensions.
+ * The checking timeout will be fired after registering the first callback.
+ */
+void
+RegisterCheckingRemoteServersCallback(CheckingRemoteServersCallback callback, void *arg)
+{
+ CheckingRemoteServersCallbackItem *item;
+ /* should we start checking timeout? */
+ bool first_exec = HaveCheckingRemoteServersCallbacks();
+
+ item = (CheckingRemoteServersCallbackItem *)
+ MemoryContextAlloc(TopMemoryContext, sizeof(CheckingRemoteServersCallbackItem));
+ item->callback = callback;
+ item->arg = arg;
+ item->next = fdw_callbacks;
+ fdw_callbacks = item;
+
+ if (first_exec && remote_servers_connection_check_interval > 0)
+ enable_timeout_after(CHECKING_REMOTE_SERVERS_TIMEOUT, remote_servers_connection_check_interval);
+}
+
+
+/*
+ * Call callbacks for checking remote servers.
+ *
+ * Note that this function will not return anything.
+ * Callback functions must throw ereport(ERROR) if disconnection has been detected.
+ */
+void
+CallCheckingRemoteServersCallbacks(void)
+{
+ CheckingRemoteServersCallbackItem *item;
+ for (item = fdw_callbacks; item; item = item->next)
+ item->callback(item->arg);
+}
+
+/*
+ * Check whether any callbacks has been registered.
+ */
+bool
+HaveCheckingRemoteServersCallbacks(void)
+{
+ return fdw_callbacks != NULL;
+}
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 9ebba025cc..5130a8340b 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -1171,7 +1171,6 @@ pq_startmsgread(void)
ereport(FATAL,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("terminating connection because protocol synchronization was lost")));
-
PqCommReadingMsg = true;
}
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 0775abe35d..ca5d50236b 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -43,6 +43,7 @@
#include "commands/async.h"
#include "commands/prepare.h"
#include "executor/spi.h"
+#include "foreign/foreign.h"
#include "jit/jit.h"
#include "libpq/libpq.h"
#include "libpq/pqformat.h"
@@ -349,6 +350,7 @@ SocketBackend(StringInfo inBuf)
* Get message type code from the frontend.
*/
HOLD_CANCEL_INTERRUPTS();
+ HOLD_CHECKING_REMOTE_SERVERS_INTERRUPTS();
pq_startmsgread();
qtype = pq_getbyte();
@@ -455,6 +457,7 @@ SocketBackend(StringInfo inBuf)
*/
if (pq_getmessage(inBuf, maxmsglen))
return EOF; /* suitable message already logged */
+ RESUME_CHECKING_REMOTE_SERVERS_INTERRUPTS();
RESUME_CANCEL_INTERRUPTS();
return qtype;
@@ -2708,6 +2711,13 @@ start_xact_command(void)
!get_timeout_active(CLIENT_CONNECTION_CHECK_TIMEOUT))
enable_timeout_after(CLIENT_CONNECTION_CHECK_TIMEOUT,
client_connection_check_interval);
+ if (remote_servers_connection_check_interval > 0 &&
+ IsUnderPostmaster &&
+ MyProcPort &&
+ HaveCheckingRemoteServersCallbacks() &&
+ !get_timeout_active(CHECKING_REMOTE_SERVERS_TIMEOUT))
+ enable_timeout_after(CHECKING_REMOTE_SERVERS_TIMEOUT,
+ remote_servers_connection_check_interval);
}
static void
@@ -3213,6 +3223,27 @@ ProcessInterrupts(void)
}
}
+ if (CheckingRemoteServersTimeoutPending)
+ {
+ if (CheckingRemoteServersHoldoffCount != 0)
+ {
+ /*
+ * Skip checking foreign servers while reading messages.
+ */
+ InterruptPending = true;
+ }
+ else
+ {
+ CheckingRemoteServersTimeoutPending = false;
+
+ CallCheckingRemoteServersCallbacks();
+
+ if (remote_servers_connection_check_interval > 0)
+ enable_timeout_after(CHECKING_REMOTE_SERVERS_TIMEOUT,
+ remote_servers_connection_check_interval);
+ }
+ }
+
if (ClientConnectionLost)
{
QueryCancelPending = false; /* lost connection trumps QueryCancel */
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 381d9e548d..15b0c2727b 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -36,9 +36,11 @@ volatile sig_atomic_t IdleInTransactionSessionTimeoutPending = false;
volatile sig_atomic_t IdleSessionTimeoutPending = false;
volatile sig_atomic_t ProcSignalBarrierPending = false;
volatile sig_atomic_t LogMemoryContextPending = false;
+volatile sig_atomic_t CheckingRemoteServersTimeoutPending = false;
volatile uint32 InterruptHoldoffCount = 0;
volatile uint32 QueryCancelHoldoffCount = 0;
volatile uint32 CritSectionCount = 0;
+volatile uint32 CheckingRemoteServersHoldoffCount = 0;
int MyProcPid;
pg_time_t MyStartTime;
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 646126edee..5962b1e395 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -73,6 +73,7 @@ static void LockTimeoutHandler(void);
static void IdleInTransactionSessionTimeoutHandler(void);
static void IdleSessionTimeoutHandler(void);
static void ClientCheckTimeoutHandler(void);
+static void CheckingRemoteServersTimeoutHandler(void);
static bool ThereIsAtLeastOneRole(void);
static void process_startup_options(Port *port, bool am_superuser);
static void process_settings(Oid databaseid, Oid roleid);
@@ -621,6 +622,7 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
IdleInTransactionSessionTimeoutHandler);
RegisterTimeout(IDLE_SESSION_TIMEOUT, IdleSessionTimeoutHandler);
RegisterTimeout(CLIENT_CONNECTION_CHECK_TIMEOUT, ClientCheckTimeoutHandler);
+ RegisterTimeout(CHECKING_REMOTE_SERVERS_TIMEOUT, CheckingRemoteServersTimeoutHandler);
}
/*
@@ -1250,6 +1252,14 @@ ClientCheckTimeoutHandler(void)
SetLatch(MyLatch);
}
+static void
+CheckingRemoteServersTimeoutHandler(void)
+{
+ CheckingRemoteServersTimeoutPending = true;
+ InterruptPending = true;
+ SetLatch(MyLatch);
+}
+
/*
* Returns true if at least one role is defined in this database cluster.
*/
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index e91d5a3cfd..1faeb59b87 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -51,6 +51,7 @@
#include "commands/vacuum.h"
#include "commands/variable.h"
#include "common/string.h"
+#include "foreign/foreign.h"
#include "funcapi.h"
#include "jit/jit.h"
#include "libpq/auth.h"
@@ -105,6 +106,7 @@
#include "utils/queryjumble.h"
#include "utils/rls.h"
#include "utils/snapmgr.h"
+#include "utils/timeout.h"
#include "utils/tzparser.h"
#include "utils/inval.h"
#include "utils/varlena.h"
@@ -3583,6 +3585,18 @@ static struct config_int ConfigureNamesInt[] =
NULL, NULL, NULL
},
+
+ {
+ {"remote_servers_connection_check_interval", PGC_USERSET, CONN_AUTH_SETTINGS,
+ gettext_noop("Sets the time interval between checks for disconnection of remote servers."),
+ NULL,
+ GUC_UNIT_MS
+ },
+ &remote_servers_connection_check_interval,
+ 0, 0, INT_MAX,
+ NULL, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL, NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 1cbc9feeb6..9b70761d9c 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -90,6 +90,10 @@
# disconnection while running queries;
# 0 for never
+#remote_servers_connection_check_interval = 0 # time between time between checks for
+ # foreign server disconnection;
+ # 0 for never
+
# - Authentication -
#authentication_timeout = 1min # 1s-600s
diff --git a/src/include/foreign/foreign.h b/src/include/foreign/foreign.h
index 8169eb76b1..98f59c1153 100644
--- a/src/include/foreign/foreign.h
+++ b/src/include/foreign/foreign.h
@@ -81,4 +81,20 @@ extern List *GetForeignColumnOptions(Oid relid, AttrNumber attnum);
extern Oid get_foreign_data_wrapper_oid(const char *fdwname, bool missing_ok);
extern Oid get_foreign_server_oid(const char *servername, bool missing_ok);
+
+/* functions and variables for fdw checking. */
+typedef void (*CheckingRemoteServersCallback) (void *arg);
+typedef struct CheckingRemoteServersCallbackItem
+{
+ struct CheckingRemoteServersCallbackItem *next;
+ CheckingRemoteServersCallback callback;
+ void *arg;
+} CheckingRemoteServersCallbackItem;
+
+extern void RegisterCheckingRemoteServersCallback(CheckingRemoteServersCallback callback, void *arg);
+extern void CallCheckingRemoteServersCallbacks(void);
+extern bool HaveCheckingRemoteServersCallbacks(void);
+
+extern int remote_servers_connection_check_interval;
+
#endif /* FOREIGN_H */
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 90a3016065..377fc68aaa 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -98,10 +98,13 @@ extern PGDLLIMPORT volatile sig_atomic_t LogMemoryContextPending;
extern PGDLLIMPORT volatile sig_atomic_t CheckClientConnectionPending;
extern PGDLLIMPORT volatile sig_atomic_t ClientConnectionLost;
+extern PGDLLIMPORT volatile sig_atomic_t CheckingRemoteServersTimeoutPending;
+
/* these are marked volatile because they are examined by signal handlers: */
extern PGDLLIMPORT volatile uint32 InterruptHoldoffCount;
extern PGDLLIMPORT volatile uint32 QueryCancelHoldoffCount;
extern PGDLLIMPORT volatile uint32 CritSectionCount;
+extern PGDLLIMPORT volatile uint32 CheckingRemoteServersHoldoffCount;
/* in tcop/postgres.c */
extern void ProcessInterrupts(void);
@@ -126,7 +129,7 @@ do { \
/* Is ProcessInterrupts() guaranteed to clear InterruptPending? */
#define INTERRUPTS_CAN_BE_PROCESSED() \
(InterruptHoldoffCount == 0 && CritSectionCount == 0 && \
- QueryCancelHoldoffCount == 0)
+ QueryCancelHoldoffCount == 0 && CheckingRemoteServersHoldoffCount == 0)
#define HOLD_INTERRUPTS() (InterruptHoldoffCount++)
@@ -152,6 +155,13 @@ do { \
CritSectionCount--; \
} while(0)
+#define HOLD_CHECKING_REMOTE_SERVERS_INTERRUPTS() (CheckingRemoteServersHoldoffCount++)
+
+#define RESUME_CHECKING_REMOTE_SERVERS_INTERRUPTS() \
+do { \
+ Assert(CheckingRemoteServersHoldoffCount > 0); \
+ CheckingRemoteServersHoldoffCount--; \
+} while(0)
/*****************************************************************************
* globals.h -- *
diff --git a/src/include/utils/timeout.h b/src/include/utils/timeout.h
index 2cbc5de4d9..ceb6b1c12c 100644
--- a/src/include/utils/timeout.h
+++ b/src/include/utils/timeout.h
@@ -34,6 +34,7 @@ typedef enum TimeoutId
IDLE_SESSION_TIMEOUT,
CLIENT_CONNECTION_CHECK_TIMEOUT,
STARTUP_PROGRESS_TIMEOUT,
+ CHECKING_REMOTE_SERVERS_TIMEOUT,
/* First user-definable timeout reason */
USER_TIMEOUT,
/* Maximum number of timeout reasons */
[application/octet-stream] v04_0002_add_doc.patch (2.4K, ../TYAPR01MB5866E0A4F9365002D8A7736FF5669@TYAPR01MB5866.jpnprd01.prod.outlook.com/4-v04_0002_add_doc.patch)
download | inline diff:
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index ab617c7b86..a93c73da2f 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1035,6 +1035,25 @@ include_dir 'conf.d'
</listitem>
</varlistentry>
+ <varlistentry id="guc-remote-servers-connection-check-interval" xreflabel="remote_servers_connection_check_interval">
+ <term><varname>remote_servers_connection_check_interval</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>remote_servers_connection_check_interval</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Sets the time interval between optional checks that the remote servers
+ are still alive. If this parameter is set to a valid value, registered
+ callback functions are executed at the specified time interval.
+ If your FDW extension does not implement a callback function, nothing
+ happens after the specified amount of time. If the value is specified
+ without units, it is taken as milliseconds. The default value is
+ <literal>0</literal>, this means the feature is disabled.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect2>
diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
index dbf5dd3d15..df52c53ae6 100644
--- a/doc/src/sgml/fdwhandler.sgml
+++ b/doc/src/sgml/fdwhandler.sgml
@@ -2136,4 +2136,26 @@ GetForeignServerByName(const char *name, bool missing_ok);
</sect1>
+ <sect1 id="fdw-checking-remote-servers">
+ <title>Checking the Health of Remote Servers via Foreign Data Wrapper</title>
+
+ <para>
+ For verifying the health of remote servers, callback functions can be
+ registered that runs periodically. The time interval is controlled by
+ <xref linkend="guc-remote-servers-connection-check-interval"/>.
+ The API for registering functions is:
+ <programlisting>
+void
+RegisterCheckingRemoteServersCallback(CheckingRemoteServersCallback callback, void *arg);
+ </programlisting>
+ Only functions that return void can be registered as callback functions.
+ Please set to void* arg a pointer if you want to pass an argument.
+ If you have problems with the health check results and want to
+ abort the transaction, please execute <function>ereport(ERROR)</function>
+ in the callback function.
+ </para>
+
+ </sect1>
+
+
</chapter>
view thread (39+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: RE: [Proposal] Add foreign-server health checks infrastructure
In-Reply-To: <TYAPR01MB5866E0A4F9365002D8A7736FF5669@TYAPR01MB5866.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox