pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: s.cherkashin@postgrespro.ru
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Add client connection check during the execution of the query
Date: Fri, 08 Feb 2019 20:16:15 +0300
Message-ID: <e2575e5bd3dbbf7c81d36ffc89a82cb9@postgrespro.ru> (raw )
In-Reply-To: <19003.1547420739@sss.pgh.pa.us >
References: <77def86b27e41f0efcba411460e929ae@postgrespro.ru >
<19003.1547420739@sss.pgh.pa.us >
The purpose of this patch is to stop the execution of continuous
requests in case of a disconnection from the client. In most cases, the
client must wait for a response from the server before sending new data
- which means there should not remain unread data on the socket and we
will be able to determine a broken connection.
Perhaps exceptions are possible, but I could not think of such a use
case (except COPY). I would be grateful if someone could offer such
cases or their solutions.
I added a test for the GUC variable when the client connects via SSL,
but I'm not sure that this test is really necessary.
Best regards,
Sergey Cherkashin.
Attachments:
Add_client_connection_check_v2.patch (text/x-diff, 13.1K) download
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index b6f5822b84..36d031df3c 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8259,6 +8259,26 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
</listitem>
</varlistentry>
+ <varlistentry id="guc-client-connection-check-interval" xreflabel="client_connection_check_interval">
+ <term><varname>client_connection_check_interval</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>client_connection_check_interval</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Sets a time interval, in milliseconds, between periodic
+ verification of client-server connection during query execution.
+ If the client aborts the connection, the query is terminated.
+ </para>
+ <para>
+ Default value is <literal>zero</literal> - it disables connection
+ checks, so the backend will detect client disconnection only when trying
+ to send a response to the query.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect2>
</sect1>
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index c39617a430..7e43734845 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -120,6 +120,7 @@
*/
int Unix_socket_permissions;
char *Unix_socket_group;
+int client_connection_check_interval;
/* Where the Unix socket files are (list of palloc'd strings) */
static List *sock_paths = NIL;
@@ -1926,3 +1927,33 @@ pq_setkeepalivescount(int count, Port *port)
return STATUS_OK;
}
+
+/* --------------------------------
+ * pq_check_client_connection - check if client connected to socket or not
+ * --------------------------------
+ */
+void pq_check_client_connection(void)
+{
+ CheckClientConnectionPending = false;
+ if (IsUnderPostmaster &&
+ MyProcPort != NULL && !PqCommReadingMsg && !PqCommBusy)
+ {
+ char nextbyte;
+ int r;
+
+#ifdef WIN32
+ pgwin32_noblock = 1;
+#endif
+ r = recv(MyProcPort->sock, &nextbyte, 1, MSG_PEEK);
+#ifdef WIN32
+ pgwin32_noblock = 0;
+#endif
+
+ if (r == 0 || (r == -1 &&
+ errno != EAGAIN && errno != EWOULDBLOCK && errno != EINTR))
+ {
+ ClientConnectionLost = true;
+ InterruptPending = true;
+ }
+ }
+}
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index e773f20d9f..0230a0fdd0 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3030,6 +3030,8 @@ ProcessInterrupts(void)
(errcode(ERRCODE_ADMIN_SHUTDOWN),
errmsg("terminating connection due to administrator command")));
}
+ if (CheckClientConnectionPending)
+ pq_check_client_connection();
if (ClientConnectionLost)
{
QueryCancelPending = false; /* lost connection trumps QueryCancel */
@@ -4208,6 +4210,9 @@ PostgresMain(int argc, char *argv[],
*/
CHECK_FOR_INTERRUPTS();
DoingCommandRead = false;
+ if (client_connection_check_interval)
+ enable_timeout_after(SKIP_CLIENT_CHECK_TIMEOUT,
+ client_connection_check_interval);
/*
* (5) turn off the idle-in-transaction timeout
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index fd51934aaf..c15aef3793 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -30,6 +30,7 @@ ProtocolVersion FrontendProtocol;
volatile sig_atomic_t InterruptPending = false;
volatile sig_atomic_t QueryCancelPending = false;
volatile sig_atomic_t ProcDiePending = false;
+volatile sig_atomic_t CheckClientConnectionPending = false;
volatile sig_atomic_t ClientConnectionLost = false;
volatile sig_atomic_t IdleInTransactionSessionTimeoutPending = false;
volatile sig_atomic_t ConfigReloadPending = false;
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index c0b6231458..45bc8babbb 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -34,6 +34,7 @@
#include "catalog/pg_db_role_setting.h"
#include "catalog/pg_tablespace.h"
#include "libpq/auth.h"
+#include "libpq/libpq.h"
#include "libpq/libpq-be.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
@@ -72,6 +73,7 @@ static void ShutdownPostgres(int code, Datum arg);
static void StatementTimeoutHandler(void);
static void LockTimeoutHandler(void);
static void IdleInTransactionSessionTimeoutHandler(void);
+static void ClientCheckTimeoutHandler(void);
static bool ThereIsAtLeastOneRole(void);
static void process_startup_options(Port *port, bool am_superuser);
static void process_settings(Oid databaseid, Oid roleid);
@@ -628,6 +630,7 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
RegisterTimeout(LOCK_TIMEOUT, LockTimeoutHandler);
RegisterTimeout(IDLE_IN_TRANSACTION_SESSION_TIMEOUT,
IdleInTransactionSessionTimeoutHandler);
+ RegisterTimeout(SKIP_CLIENT_CHECK_TIMEOUT, ClientCheckTimeoutHandler);
}
/*
@@ -1239,6 +1242,16 @@ IdleInTransactionSessionTimeoutHandler(void)
SetLatch(MyLatch);
}
+static void
+ClientCheckTimeoutHandler(void)
+{
+ CheckClientConnectionPending = true;
+ InterruptPending = true;
+ if (client_connection_check_interval > 0)
+ enable_timeout_after(SKIP_CLIENT_CHECK_TIMEOUT,
+ client_connection_check_interval);
+}
+
/*
* 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 c216ed0922..4a1431477d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3142,6 +3142,16 @@ static struct config_int ConfigureNamesInt[] =
NULL, NULL, NULL
},
+ {
+ {"client_connection_check_interval", PGC_USERSET, CLIENT_CONN_OTHER,
+ gettext_noop("Sets the time interval for checking connection with the client."),
+ gettext_noop("A value of -1 disables this feature. Zero selects a suitable default value."),
+ GUC_UNIT_MS
+ },
+ &client_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 a21865a77f..c682dcc467 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -674,6 +674,9 @@
#dynamic_library_path = '$libdir'
+#client_connection_check_interval = 1000 # set time interval between
+ # connection checks, in ms
+ # 0 is disabled
#------------------------------------------------------------------------------
# LOCK MANAGEMENT
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index 755819cc58..0048a9ee79 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -71,6 +71,7 @@ extern int pq_getbyte(void);
extern int pq_peekbyte(void);
extern int pq_getbyte_if_available(unsigned char *c);
extern int pq_putbytes(const char *s, size_t len);
+extern void pq_check_client_connection(void);
/*
* prototypes for functions in be-secure.c
@@ -102,6 +103,7 @@ extern WaitEventSet *FeBeWaitSet;
extern char *SSLCipherSuites;
extern char *SSLECDHCurve;
extern bool SSLPreferServerCiphers;
+extern int client_connection_check_interval;
extern int ssl_min_protocol_version;
extern int ssl_max_protocol_version;
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index c9e35003a5..8b7f72d0d0 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -83,7 +83,7 @@ extern PGDLLIMPORT volatile sig_atomic_t QueryCancelPending;
extern PGDLLIMPORT volatile sig_atomic_t ProcDiePending;
extern PGDLLIMPORT volatile sig_atomic_t IdleInTransactionSessionTimeoutPending;
extern PGDLLIMPORT volatile sig_atomic_t ConfigReloadPending;
-
+extern PGDLLIMPORT volatile sig_atomic_t CheckClientConnectionPending;
extern PGDLLIMPORT volatile sig_atomic_t ClientConnectionLost;
/* these are marked volatile because they are examined by signal handlers: */
diff --git a/src/include/utils/timeout.h b/src/include/utils/timeout.h
index 9244a2a7b7..acd26bb39c 100644
--- a/src/include/utils/timeout.h
+++ b/src/include/utils/timeout.h
@@ -31,6 +31,7 @@ typedef enum TimeoutId
STANDBY_TIMEOUT,
STANDBY_LOCK_TIMEOUT,
IDLE_IN_TRANSACTION_SESSION_TIMEOUT,
+ SKIP_CLIENT_CHECK_TIMEOUT,
/* First user-definable timeout reason */
USER_TIMEOUT,
/* Maximum number of timeout reasons */
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 19d60a506e..526a5149cf 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -7,6 +7,7 @@ include $(top_builddir)/src/Makefile.global
SUBDIRS = \
brin \
commit_ts \
+ connection \
dummy_seclabel \
snapshot_too_old \
test_bloomfilter \
diff --git a/src/test/modules/connection/Makefile b/src/test/modules/connection/Makefile
new file mode 100644
index 0000000000..5c44d7ad40
--- /dev/null
+++ b/src/test/modules/connection/Makefile
@@ -0,0 +1,16 @@
+# src/test/modules/connection/Makefile
+
+subdir = src/test/modules/connection
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+
+export with_openssl
+
+check:
+ $(prove_check)
+
+installcheck:
+ $(prove_installcheck)
+
+clean distclean maintainer-clean:
+ rm -rf tmp_check
diff --git a/src/test/modules/connection/t/001_close_connection.pl b/src/test/modules/connection/t/001_close_connection.pl
new file mode 100644
index 0000000000..7f727833ec
--- /dev/null
+++ b/src/test/modules/connection/t/001_close_connection.pl
@@ -0,0 +1,107 @@
+# Check if backend stopped after client disconnection
+use strict;
+use warnings;
+use PostgresNode;
+use TestLib;
+use Test::More;
+use File::Copy;
+
+if ($ENV{with_openssl} eq 'yes')
+{
+ plan tests => 3;
+}
+else
+{
+ plan tests => 2;
+}
+
+my $long_query = q{
+DO
+$$
+DECLARE row_data RECORD;
+BEGIN
+EXECUTE 'CREATE TABLE IF NOT EXISTS keep_alive_test AS SELECT generate_series(0,100000) AS tt';
+FOR row_data IN
+ SELECT tt
+ FROM keep_alive_test
+LOOP
+ EXECUTE 'SELECT count(*) FROM keep_alive_test';
+END LOOP;
+END$$;
+};
+my $set_guc_on = q{
+ SET client_connection_check_interval = 1000;
+};
+my $set_guc_off = q{
+ SET client_connection_check_interval = 0;
+};
+my ($pid, $timed_out);
+
+my $node = get_new_node('node');
+$node->init;
+$node->start;
+
+#########################################################
+# TEST 1: GUC client_connection_check_interval: enabled #
+#########################################################
+
+# Set GUC options, get backend pid and run a long time query
+$node->psql('postgres', "$set_guc_on SELECT pg_backend_pid(); $long_query",
+ stdout => \$pid, timeout => 2, timed_out => \$timed_out);
+
+# Give time to the backend to detect client disconnected
+sleep 3;
+# Check if backend is still alive
+my $is_alive = $node->safe_psql('postgres', "SELECT count(*) FROM pg_stat_activity where pid = $pid;");
+is($is_alive, '0', 'Test: client_connection_check_interval enable');
+$node->stop;
+
+##########################################################
+# TEST 2: GUC client_connection_check_interval: disabled #
+##########################################################
+
+$node->start;
+$node->psql('postgres', "$set_guc_off SELECT pg_backend_pid(); $long_query",
+ stdout => \$pid, timeout => 2, timed_out => \$timed_out);
+# Give time to the client to disconnect
+sleep 3;
+# Check if backend is still alive
+$is_alive = $node->safe_psql('postgres', "SELECT count(*) FROM pg_stat_activity where pid = $pid;");
+is($is_alive, '1', 'Test: client_connection_check_interval disable');
+$node->stop;
+
+##########################################################
+# TEST 3: Using client_connection_check_interval when #
+# client connected using SSL #
+##########################################################
+
+if ($ENV{with_openssl} eq 'yes')
+{
+ # The client's private key must not be world-readable, so take a copy
+ # of the key stored in the code tree and update its permissions.
+ copy("../../ssl/ssl/client.key", "../../ssl/ssl/client_tmp.key");
+ chmod 0600, "../../ssl/ssl/client_tmp.key";
+ copy("../../ssl/ssl/client-revoked.key", "../../ssl/ssl/client-revoked_tmp.key");
+ chmod 0600, "../../ssl/ssl/client-revoked_tmp.key";
+ $ENV{PGHOST} = $node->host;
+ $ENV{PGPORT} = $node->port;
+
+ open my $sslconf, '>', $node->data_dir . "/sslconfig.conf";
+ print $sslconf "ssl=on\n";
+ print $sslconf "ssl_cert_file='server-cn-only.crt'\n";
+ print $sslconf "ssl_key_file='server-password.key'\n";
+ print $sslconf "ssl_passphrase_command='echo secret1'\n";
+ close $sslconf;
+
+ $node->start;
+ $node->psql('postgres', "$set_guc_on SELECT pg_backend_pid(); $long_query",
+ stdout => \$pid, timeout => 2, timed_out => \$timed_out,
+ sslmode => 'require');
+
+ # Give time to the backend to detect client disconnected
+ sleep 3;
+ # Check if backend is still alive
+ my $is_alive = $node->safe_psql('postgres', "SELECT count(*) FROM pg_stat_activity where pid = $pid;");
+ is($is_alive, '0', 'Test: client_connection_check_interval enabled, SSL');
+ $node->stop;
+}
view thread (54+ messages) latest in thread
Message-ID: <e2575e5bd3dbbf7c81d36ffc89a82cb9@postgrespro.ru>
Permalink: ../e2575e5bd3dbbf7c81d36ffc89a82cb9@postgrespro.ru/
Also on: postgresql.org/message-id/e2575e5bd3dbbf7c81d36ffc89a82cb9@postgrespro.ru
copy link · copy postgr.es · copy git am
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: pgsql-hackers@postgresql.org
Cc: s.cherkashin@postgrespro.ru, tgl@sss.pgh.pa.us
Subject: Re: Add client connection check during the execution of the query
In-Reply-To: <e2575e5bd3dbbf7c81d36ffc89a82cb9@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox