From 09e4fb2ffaeaccf6c74c5aeac7ffa65fc6743b1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ayd=C4=B1n=20Mercan?= Date: Tue, 30 Apr 2024 14:37:26 +0300 Subject: [PATCH 1/3] Return the old counter value in `isc_stats_increment` Returning the value allows for better high-water tracking without running into edge cases like the following: 0. The counter is at value X 1. Increment the value (X+1) 2. The value is decreased multiple times in another threads (X+1-Y) 3. Get the value (X+1-Y) 4. Update-if-greater misses the X+1 value which should have been the high-water --- lib/isc/include/isc/stats.h | 4 ++-- lib/isc/stats.c | 4 ++-- lib/ns/include/ns/stats.h | 2 +- lib/ns/stats.c | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/isc/include/isc/stats.h b/lib/isc/include/isc/stats.h index 23a076eba9..f6520969c8 100644 --- a/lib/isc/include/isc/stats.h +++ b/lib/isc/include/isc/stats.h @@ -137,10 +137,10 @@ isc_stats_ncounters(isc_stats_t *stats); * */ -void +isc_statscounter_t isc_stats_increment(isc_stats_t *stats, isc_statscounter_t counter); /*%< - * Increment the counter-th counter of stats. + * Increment the counter-th counter of stats and return the old value. * * Requires: *\li 'stats' is a valid isc_stats_t. diff --git a/lib/isc/stats.c b/lib/isc/stats.c index 07a88fb6c9..2f7e8bcde1 100644 --- a/lib/isc/stats.c +++ b/lib/isc/stats.c @@ -97,12 +97,12 @@ isc_stats_create(isc_mem_t *mctx, isc_stats_t **statsp, int ncounters) { *statsp = stats; } -void +isc_statscounter_t isc_stats_increment(isc_stats_t *stats, isc_statscounter_t counter) { REQUIRE(ISC_STATS_VALID(stats)); REQUIRE(counter < stats->ncounters); - atomic_fetch_add_relaxed(&stats->counters[counter], 1); + return (atomic_fetch_add_relaxed(&stats->counters[counter], 1)); } void diff --git a/lib/ns/include/ns/stats.h b/lib/ns/include/ns/stats.h index d7e79443cf..c904d7f38c 100644 --- a/lib/ns/include/ns/stats.h +++ b/lib/ns/include/ns/stats.h @@ -124,7 +124,7 @@ ns_stats_detach(ns_stats_t **statsp); void ns_stats_create(isc_mem_t *mctx, int ncounters, ns_stats_t **statsp); -void +isc_statscounter_t ns_stats_increment(ns_stats_t *stats, isc_statscounter_t counter); void diff --git a/lib/ns/stats.c b/lib/ns/stats.c index 84dd0ab029..791c08a756 100644 --- a/lib/ns/stats.c +++ b/lib/ns/stats.c @@ -78,11 +78,11 @@ ns_stats_create(isc_mem_t *mctx, int ncounters, ns_stats_t **statsp) { /*% * Increment/Decrement methods */ -void +isc_statscounter_t ns_stats_increment(ns_stats_t *stats, isc_statscounter_t counter) { REQUIRE(NS_STATS_VALID(stats)); - isc_stats_increment(stats->counters, counter); + return (isc_stats_increment(stats->counters, counter)); } void From e037520b923b4d711128b095e14ec61a39669cb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ayd=C4=B1n=20Mercan?= Date: Mon, 22 Apr 2024 17:01:16 +0300 Subject: [PATCH 2/3] Keep track of the recursive clients highwater The high-water allows administrators to better tune the recursive clients limit without having to to poll the statistics channel in high rates to get this number. --- bin/named/server.c | 6 ++++++ bin/named/statschannel.c | 2 ++ lib/ns/include/ns/stats.h | 4 +++- lib/ns/query.c | 9 +++++++-- 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 908eb4de5d..f972dce906 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -12184,6 +12184,12 @@ named_server_status(named_server_t *server, isc_buffer_t **text) { isc_quota_getmax(&server->sctx->recursionquota)); CHECK(putstr(text, line)); + snprintf(line, sizeof(line), "recursive high-water: %u\n", + (unsigned int)ns_stats_get_counter( + server->sctx->nsstats, + ns_statscounter_recurshighwater)); + CHECK(putstr(text, line)); + snprintf(line, sizeof(line), "tcp clients: %u/%u\n", isc_quota_getused(&server->sctx->tcpquota), isc_quota_getmax(&server->sctx->tcpquota)); diff --git a/bin/named/statschannel.c b/bin/named/statschannel.c index 494b3c6c82..b9b85e0594 100644 --- a/bin/named/statschannel.c +++ b/bin/named/statschannel.c @@ -327,6 +327,8 @@ init_desc(void) { SET_NSSTATDESC(updatebadprereq, "updates rejected due to prerequisite failure", "UpdateBadPrereq"); + SET_NSSTATDESC(recurshighwater, "Recursive clients high-water", + "RecursHighwater"); SET_NSSTATDESC(recursclients, "recursing clients", "RecursClients"); SET_NSSTATDESC(dns64, "queries answered by DNS64", "DNS64"); SET_NSSTATDESC(ratedropped, "responses dropped for rate limits", diff --git a/lib/ns/include/ns/stats.h b/lib/ns/include/ns/stats.h index c904d7f38c..766ba37a56 100644 --- a/lib/ns/include/ns/stats.h +++ b/lib/ns/include/ns/stats.h @@ -112,7 +112,9 @@ enum { ns_statscounter_updatequota = 67, - ns_statscounter_max = 68, + ns_statscounter_recurshighwater = 68, + + ns_statscounter_max = 69, }; void diff --git a/lib/ns/query.c b/lib/ns/query.c index ee88495dfb..2827fc0e0b 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -2483,6 +2483,7 @@ free_fresp(ns_client_t *client, dns_fetchresponse_t **frespp) { static isc_result_t recursionquotatype_attach(ns_client_t *client, bool soft_limit) { + isc_statscounter_t recurscount; isc_result_t result; result = isc_quota_acquire(&client->manager->sctx->recursionquota); @@ -2505,8 +2506,12 @@ recursionquotatype_attach(ns_client_t *client, bool soft_limit) { return (result); } - ns_stats_increment(client->manager->sctx->nsstats, - ns_statscounter_recursclients); + recurscount = ns_stats_increment(client->manager->sctx->nsstats, + ns_statscounter_recursclients); + + ns_stats_update_if_greater(client->manager->sctx->nsstats, + ns_statscounter_recurshighwater, + recurscount + 1); return (result); } From d6dd51bb1b5a66303dd7c46bc487cee97097962e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ayd=C4=B1n=20Mercan?= Date: Wed, 8 May 2024 14:58:57 +0300 Subject: [PATCH 3/3] Added CHANGES and release note for [GL #4668] --- CHANGES | 5 +++++ doc/notes/notes-current.rst | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index 873c1b948d..a3432c8c50 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +6387. [func] Added a new statistics variable "recursive high-water" + that reports the maximum number of simultaneous + recursive clients BIND has handled while running. + [GL #4668] + 6386. [bug] When shutting down catzs->view could point to freed memory. Obtain a reference to the view to prevent this. [GL #4502] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 133d75c2a3..0b57512d6e 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -20,7 +20,9 @@ Security Fixes New Features ~~~~~~~~~~~~ -- None. +- Added a new statistics variable ``recursive high-water`` that reports + the maximum number of simultaneous recursive clients BIND has handled + while running. :gl:`#4668` Removed Features ~~~~~~~~~~~~~~~~