From fc9792eae80c34aa1e064d5abf2d2c4f9513bd39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Witold=20Kr=C4=99cicki?= Date: Fri, 28 Feb 2020 13:58:13 +0100 Subject: [PATCH] Limit TCP connection quota logging to 1/s --- lib/isc/netmgr/netmgr.c | 6 +++-- lib/isc/netmgr/tcp.c | 53 +++++++++++++++++++++++++++++------------ 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 0e04dcb8af..9f49dfbd9f 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -719,7 +719,9 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree) { if (!atomic_load(&sock->children[i].destroying)) { nmsocket_cleanup(&sock->children[i], false); if (sock->statsindex != NULL) { - isc__nm_decstats(sock->mgr, sock->statsindex[STATID_ACTIVE]); + isc__nm_decstats( + sock->mgr, + sock->statsindex[STATID_ACTIVE]); } } } @@ -733,7 +735,7 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree) { sock->nchildren = 0; } if (sock->statsindex != NULL) { - isc__nm_decstats(sock->mgr, sock->statsindex[STATID_ACTIVE]); + isc__nm_decstats(sock->mgr, sock->statsindex[STATID_ACTIVE]); } if (sock->tcphandle != NULL) { diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c index 60ca097de0..69eabeb3a1 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -26,12 +26,28 @@ #include #include #include +#include #include #include #include "netmgr-int.h" #include "uv-compat.h" +static atomic_uint_fast32_t last_tcpquota_log = ATOMIC_VAR_INIT(0); + +static bool +can_log_tcp_quota() { + isc_stdtime_t now, last; + + isc_stdtime_get(&now); + last = atomic_exchange_relaxed(&last_tcpquota_log, now); + if (now != last) { + return (true); + } + + return (false); +} + static int tcp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req); @@ -691,10 +707,15 @@ accept_connection(isc_nmsocket_t *ssock) { if (ssock->pquota != NULL) { result = isc_quota_attach(ssock->pquota, "a); + /* - * We share the quota between sockets - we need to have at - * least one active connection here to restart listening - * when the quota is available again (in tcp_close_direct). + * We share the quota between all TCP sockets. Others + * may have used up all the quota slots, in which case + * this socket could starve. So we only fail here if we + * already had at least one active connection on this + * socket. This guarantees that we'll maintain some level + * of service while over quota, and will resume normal + * service when the quota comes back down. */ if (result != ISC_R_SUCCESS) { ssock->overquota++; @@ -705,12 +726,6 @@ accept_connection(isc_nmsocket_t *ssock) { ssock->statsindex[STATID_ACCEPTFAIL]); return (result); } - /* - * We share the quota between sockets - we need to have - * at least one active connection here to restart - * listening when the quota is available again (in - * tcp_close_direct). - */ } } @@ -792,11 +807,14 @@ tcp_connection_cb(uv_stream_t *server, int status) { UNUSED(status); result = accept_connection(ssock); - if (result != ISC_R_SUCCESS) { - isc_log_write(isc_lctx, ISC_LOGCATEGORY_GENERAL, - ISC_LOGMODULE_NETMGR, ISC_LOG_ERROR, - "TCP connection failed: %s", - isc_result_totext(result)); + if (result != ISC_R_SUCCESS && result != ISC_R_NOCONN) { + if ((result != ISC_R_QUOTA && result != ISC_R_SOFTQUOTA) || + can_log_tcp_quota()) { + isc_log_write(isc_lctx, ISC_LOGCATEGORY_GENERAL, + ISC_LOGMODULE_NETMGR, ISC_LOG_ERROR, + "TCP connection failed: %s", + isc_result_totext(result)); + } } } @@ -935,7 +953,12 @@ tcp_close_direct(isc_nmsocket_t *sock) { while (ssock->conns == 0 && ssock->overquota > 0) { ssock->overquota--; isc_result_t result = accept_connection(ssock); - if (result != ISC_R_SUCCESS) { + if (result == ISC_R_SUCCESS || result == ISC_R_NOCONN) { + continue; + } + if ((result != ISC_R_QUOTA && + result != ISC_R_SOFTQUOTA) || + can_log_tcp_quota()) { isc_log_write(isc_lctx, ISC_LOGCATEGORY_GENERAL, ISC_LOGMODULE_NETMGR, ISC_LOG_ERROR,