Hi,
I am proposing that the default value of
client_connection_check_interval be moved to a non-zero value on
systems where it is supported. I think it would be a nice quality of
life improvement. I have run into a problem where this would have been
useful before with regard to pgbench not currently handling SIGINT
corrently[0]. I basically picked 10s out of thin air and am happy to
change it to what others feel would be more appropriate. This doesn't
need to be a check that happens often because it should just be a
backstop for unusual scenarios or poorly programmed clients.
The original thread where Thomas committed these changes seemed to
indicate no performance impact[1]. The only reason that I can think of
this being turned off by default is that it isn't available on all
systems that Postgres supports. When this was committed however, the
only system that seemed to support EPOLLRDHUP was Linux. Seems like in
recent years this story has changed given the description of the
parameter.
> This option relies on kernel events exposed by Linux, macOS, illumos and
> the BSD family of operating systems, and is not currently available on
> other systems.
[0]: https://www.postgresql.org/message-id/CSSWBAX56CVY.291H6ZNNHK7EO@c3po
[1]:
https://www.postgresql.org/message-id/ca+hukg++kitznuoxw2-kob1pkwd2cyuqa9vlj5bf0g_i7l1...@mail.gmail.com
--
Tristan Partin
Neon (https://neon.tech)
From d38cc95dceda5fcf98c8aae6379085e9c38a3155 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tris...@neon.tech>
Date: Fri, 16 Jun 2023 12:47:26 -0500
Subject: [PATCH v1] Default client_connection_check_interval to 10s on
supported systems
client_connection_check_interval is an inherently valuable parameter
when supported. It can help cancel long running queries server-side when
a connection drops client-side.
---
configure.ac | 3 +++
doc/src/sgml/config.sgml | 10 ++++++----
meson.build | 2 ++
src/backend/utils/misc/guc_tables.c | 7 ++++++-
src/include/pg_config.h.in | 6 ++++++
src/tools/msvc/Solution.pm | 2 ++
6 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/configure.ac b/configure.ac
index 09558ada0f..3df7bbeb4f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1846,6 +1846,9 @@ AC_CHECK_DECLS([strlcat, strlcpy, strnlen])
AC_CHECK_DECLS([preadv], [], [AC_LIBOBJ(preadv)], [#include <sys/uio.h>])
AC_CHECK_DECLS([pwritev], [], [AC_LIBOBJ(pwritev)], [#include <sys/uio.h>])
+AC_CHECK_DECLS(EPOLLHUP, [], [], [#include <epoll.h>])
+AC_CHECK_DECLS(EPOLLRDHUP, [], [], [#include <epoll.h>])
+
# This is probably only present on macOS, but may as well check always
AC_CHECK_DECLS(F_FULLFSYNC, [], [], [#include <fcntl.h>])
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 6262cb7bb2..24f69ebd15 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1044,10 +1044,12 @@ include_dir 'conf.d'
</para>
<para>
If the value is specified without units, it is taken as milliseconds.
- The default value is <literal>0</literal>, which disables connection
- checks. Without connection checks, the server will detect the loss of
- the connection only at the next interaction with the socket, when it
- waits for, receives or sends data.
+ The default value is <literal>0</literal> on systems not previously
+ mentioned, such as Windows, which disables connection checks, and a
+ default value of <literal>10000</literal>, otherwise. Without
+ connection checks, the server will detect the loss of the connection
+ only at the next interaction with the socket, when it waits for,
+ receives or sends data.
</para>
<para>
For the kernel itself to detect lost TCP connections reliably and within
diff --git a/meson.build b/meson.build
index 82f2782673..8de6a7a7f1 100644
--- a/meson.build
+++ b/meson.build
@@ -2163,6 +2163,8 @@ endforeach
decl_checks = [
+ ['EPOLLHUP', 'sys/epoll.h'],
+ ['EPOLLRDHUP', 'sys/epoll.h'],
['F_FULLFSYNC', 'fcntl.h'],
['fdatasync', 'unistd.h'],
['posix_fadvise', 'fcntl.h'],
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 71e27f8eb0..8cfd6b61bb 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -3478,7 +3478,12 @@ struct config_int ConfigureNamesInt[] =
GUC_UNIT_MS
},
&client_connection_check_interval,
- 0, 0, INT_MAX,
+#if defined(HAVE_DECL_EPOLLHUP) || defined(HAVE_DECL_EPOLLRDHUP)
+ 10000,
+#else
+ 0,
+#endif
+ 0, INT_MAX,
check_client_connection_check_interval, NULL, NULL
},
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 6d572c3820..7ce5004550 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -91,6 +91,12 @@
/* Define to 1 if you have the `CRYPTO_lock' function. */
#undef HAVE_CRYPTO_LOCK
+/* Define to 1 if you have the EPOLLHUP flag */
+#undef HAVE_DECL_EPOLLHUP
+
+/* Define to 1 if you have the EPOLLRDHUP flag */
+#undef HAVE_DECL_EPOLLRDHUP
+
/* Define to 1 if you have the declaration of `fdatasync', and to 0 if you
don't. */
#undef HAVE_DECL_FDATASYNC
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index b6d31c3583..14c43fefb0 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -232,6 +232,8 @@ sub GenerateFiles
HAVE_COPYFILE_H => undef,
HAVE_CRTDEFS_H => undef,
HAVE_CRYPTO_LOCK => undef,
+ HAVE_DECL_EPOLLHUP => 0,
+ HAVE_DECL_EPOLLRDHUP => 0,
HAVE_DECL_FDATASYNC => 0,
HAVE_DECL_F_FULLFSYNC => 0,
HAVE_DECL_LLVMCREATEGDBREGISTRATIONLISTENER => 0,
--
Tristan Partin
Neon (https://neon.tech)
