v3-0002-Use-WL_SOCKET_CLOSED-for-client_connection_check_.patch
text/x-patch
Filename: v3-0002-Use-WL_SOCKET_CLOSED-for-client_connection_check_.patch
Type: text/x-patch
Part: 1
Patch
Format: format-patch
Series: patch v3-0002
Subject: Use WL_SOCKET_CLOSED for client_connection_check_interval.
| File | + | − |
|---|---|---|
| doc/src/sgml/config.sgml | 3 | 3 |
| src/backend/libpq/pqcomm.c | 29 | 15 |
| src/backend/utils/misc/guc.c | 2 | 5 |
From 2d191c0d3bbc240e125633d51f9f1e003630b866 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Fri, 30 Apr 2021 10:48:32 +1200
Subject: [PATCH v3 2/2] Use WL_SOCKET_CLOSED for
client_connection_check_interval.
Previously we used poll() directly to check for a POLLRDHUP event.
Instead, use WaitEventSet to poll the socket for WL_SOCKET_CLOSED, which
knows how to detect equivalent events on many more operating systems.
Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Reviewed-by: Maksim Milyutin <milyutinma@gmail.com>
Discussion: https://postgr.es/m/77def86b27e41f0efcba411460e929ae%40postgrespro.ru
---
doc/src/sgml/config.sgml | 6 ++---
src/backend/libpq/pqcomm.c | 44 ++++++++++++++++++++++++------------
src/backend/utils/misc/guc.c | 7 ++----
3 files changed, 34 insertions(+), 23 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 4ac617615c..ab52d4bd2c 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1012,9 +1012,9 @@ include_dir 'conf.d'
the kernel reports that the connection is closed.
</para>
<para>
- This option 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 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.
</para>
<para>
If the value is specified without units, it is taken as milliseconds.
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 4c37df09cf..c235a682c2 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -1959,22 +1959,36 @@ pq_settcpusertimeout(int timeout, Port *port)
bool
pq_check_connection(void)
{
-#if defined(POLLRDHUP)
- /*
- * POLLRDHUP is a Linux extension to poll(2) to detect sockets closed by
- * the other end. We don't have a portable way to do that without
- * actually trying to read or write data on other systems. We don't want
- * to read because that would be confused by pipelined queries and COPY
- * data. Perhaps in future we'll try to write a heartbeat message instead.
- */
- struct pollfd pollfd;
+ WaitEvent event;
int rc;
- pollfd.fd = MyProcPort->sock;
- pollfd.events = POLLOUT | POLLIN | POLLRDHUP;
- pollfd.revents = 0;
+ /*
+ * Temporarily ignore the latch, so that we can poll for just the one
+ * event we care about.
+ */
+ ModifyWaitEvent(FeBeWaitSet, FeBeWaitSetLatchPos, WL_LATCH_SET, NULL);
- rc = poll(&pollfd, 1, 0);
+ PG_TRY();
+ {
+ /*
+ * It's OK to clobber the socket event to report only the event we're
+ * interested in without restoring the previous state afterwards,
+ * because every FeBeWaitSet wait site does the same.
+ */
+ ModifyWaitEvent(FeBeWaitSet, FeBeWaitSetSocketPos, WL_SOCKET_CLOSED,
+ NULL);
+ rc = WaitEventSetWait(FeBeWaitSet, 0, &event, 1, 0);
+ }
+ PG_FINALLY();
+ {
+ /*
+ * Restore the latch, so we can't leave FeBeWaitSet in a broken state
+ * that ignores latches.
+ */
+ ModifyWaitEvent(FeBeWaitSet, FeBeWaitSetLatchPos, WL_LATCH_SET,
+ MyLatch);
+ }
+ PG_END_TRY();
if (rc < 0)
{
@@ -1983,9 +1997,9 @@ pq_check_connection(void)
errmsg("could not poll socket: %m")));
return false;
}
- else if (rc == 1 && (pollfd.revents & (POLLHUP | POLLRDHUP)))
+
+ if (rc == 1 && (event.events & WL_SOCKET_CLOSED))
return false;
-#endif
return true;
}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index ee6a838b3a..ca45b36754 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -12169,14 +12169,11 @@ check_huge_page_size(int *newval, void **extra, GucSource source)
static bool
check_client_connection_check_interval(int *newval, void **extra, GucSource source)
{
-#ifndef POLLRDHUP
- /* Linux only, for now. See pq_check_connection(). */
- if (*newval != 0)
+ if (!WaitEventSetCanReportClosed() && *newval != 0)
{
- GUC_check_errdetail("client_connection_check_interval must be set to 0 on platforms that lack POLLRDHUP.");
+ GUC_check_errdetail("client_connection_check_interval must be set to 0 on this platform");
return false;
}
-#endif
return true;
}
--
2.33.1