From: Aram Sargsyan Date: Thu, 15 Jan 2026 14:46:06 +0000 (+0000) Subject: Replace the outgoing queries RTT histogram code with isc_histomulti X-Git-Tag: v9.21.20~24^2~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e41fbea8432e987d1004c38c30a88d6bd6183c11;p=thirdparty%2Fbind9.git Replace the outgoing queries RTT histogram code with isc_histomulti The granularity of the simple histogram with fixed number of ranges sometimes isn't good enough. As there's a need to implement a new histogram statistics for the incoming query times (RTT), it was decided to also update the existing RTT statistics of the outgoing queries so that they look similar and use common code. Remove the old histogram code from the resolver and from the statistics channel. Reimplement the outgoing queries RTT histogram using the isc_histomulti module, and prepare the necessary base for implementing the incoming queries RTT histogram. The statistics channel will be updated to expose the new histograms in an upcoming commit. --- diff --git a/bin/named/server.c b/bin/named/server.c index 85e8c686526..160e3d5990e 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -3678,6 +3678,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, const cfg_obj_t *disablelist = NULL; isc_stats_t *resstats = NULL; dns_stats_t *resquerystats = NULL; + isc_histomulti_t *resqueryinrttstats = NULL; + isc_histomulti_t *resqueryoutrttstats = NULL; bool auto_root = false; named_cache_t *nsc = NULL; bool zero_no_soattl; @@ -4259,6 +4261,9 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, dns_resolver_getstats(pview->resolver, &resstats); dns_resolver_getquerystats(pview->resolver, &resquerystats); + dns_resolver_getqueryrttstats(pview->resolver, + &resqueryinrttstats, + &resqueryoutrttstats); dns_view_detach(&pview); } } @@ -4319,11 +4324,23 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, isc_stats_create(mctx, &resstats, dns_resstatscounter_max); } dns_resolver_setstats(view->resolver, resstats); + if (resquerystats == NULL) { dns_rdatatypestats_create(mctx, &resquerystats); } dns_resolver_setquerystats(view->resolver, resquerystats); + if (resqueryinrttstats == NULL) { + isc_histomulti_create(mctx, DNS_RTTHISTO_SIGBITS, + &resqueryinrttstats); + } + if (resqueryoutrttstats == NULL) { + isc_histomulti_create(mctx, DNS_RTTHISTO_SIGBITS, + &resqueryoutrttstats); + } + dns_resolver_setqueryrttstats(view->resolver, resqueryinrttstats, + resqueryoutrttstats); + /* * Set the ADB cache size to 1/8th of the max-cache-size or * MAX_ADB_SIZE_FOR_CACHESHARE when the cache is shared. @@ -5487,6 +5504,12 @@ cleanup: if (resquerystats != NULL) { dns_stats_detach(&resquerystats); } + if (resqueryinrttstats != NULL) { + isc_histomulti_detach(&resqueryinrttstats); + } + if (resqueryoutrttstats != NULL) { + isc_histomulti_detach(&resqueryoutrttstats); + } if (order != NULL) { dns_order_detach(&order); } diff --git a/bin/named/statschannel.c b/bin/named/statschannel.c index 51184ffe441..f720d7c80eb 100644 --- a/bin/named/statschannel.c +++ b/bin/named/statschannel.c @@ -441,28 +441,6 @@ init_desc(void) { SET_RESSTATDESC(valnegsuccess, "DNSSEC NX validation succeeded", "ValNegOk"); SET_RESSTATDESC(valfail, "DNSSEC validation failed", "ValFail"); - SET_RESSTATDESC(queryrtt0, - "queries with RTT < " DNS_RESOLVER_QRYRTTCLASS0STR "ms", - "QryRTT" DNS_RESOLVER_QRYRTTCLASS0STR); - SET_RESSTATDESC(queryrtt1, - "queries with RTT " DNS_RESOLVER_QRYRTTCLASS0STR - "-" DNS_RESOLVER_QRYRTTCLASS1STR "ms", - "QryRTT" DNS_RESOLVER_QRYRTTCLASS1STR); - SET_RESSTATDESC(queryrtt2, - "queries with RTT " DNS_RESOLVER_QRYRTTCLASS1STR - "-" DNS_RESOLVER_QRYRTTCLASS2STR "ms", - "QryRTT" DNS_RESOLVER_QRYRTTCLASS2STR); - SET_RESSTATDESC(queryrtt3, - "queries with RTT " DNS_RESOLVER_QRYRTTCLASS2STR - "-" DNS_RESOLVER_QRYRTTCLASS3STR "ms", - "QryRTT" DNS_RESOLVER_QRYRTTCLASS3STR); - SET_RESSTATDESC(queryrtt4, - "queries with RTT " DNS_RESOLVER_QRYRTTCLASS3STR - "-" DNS_RESOLVER_QRYRTTCLASS4STR "ms", - "QryRTT" DNS_RESOLVER_QRYRTTCLASS4STR); - SET_RESSTATDESC(queryrtt5, - "queries with RTT > " DNS_RESOLVER_QRYRTTCLASS4STR "ms", - "QryRTT" DNS_RESOLVER_QRYRTTCLASS4STR "+"); SET_RESSTATDESC(nfetch, "active fetches", "NumFetch"); SET_RESSTATDESC(buckets, "bucket size", "BucketSize"); SET_RESSTATDESC(refused, "REFUSED received", "REFUSED"); diff --git a/lib/dns/include/dns/resolver.h b/lib/dns/include/dns/resolver.h index 5c56d5d86d0..6ec13fc631e 100644 --- a/lib/dns/include/dns/resolver.h +++ b/lib/dns/include/dns/resolver.h @@ -49,6 +49,7 @@ #include #include +#include #include #include #include @@ -143,21 +144,6 @@ enum { DNS_FETCHOPT_EDNSVERSIONMASK = 0xff000000, }; -/* - * Upper bounds of class of query RTT (ms). Corresponds to - * dns_resstatscounter_queryrttX statistics counters. - */ -#define DNS_RESOLVER_QRYRTTCLASS0 10 -#define DNS_RESOLVER_QRYRTTCLASS0STR "10" -#define DNS_RESOLVER_QRYRTTCLASS1 100 -#define DNS_RESOLVER_QRYRTTCLASS1STR "100" -#define DNS_RESOLVER_QRYRTTCLASS2 500 -#define DNS_RESOLVER_QRYRTTCLASS2STR "500" -#define DNS_RESOLVER_QRYRTTCLASS3 800 -#define DNS_RESOLVER_QRYRTTCLASS3STR "800" -#define DNS_RESOLVER_QRYRTTCLASS4 1600 -#define DNS_RESOLVER_QRYRTTCLASS4STR "1600" - /* * XXXRTH Should this API be made semi-private? (I.e. * _dns_resolver_create()). @@ -586,8 +572,7 @@ dns_resolver_setstats(dns_resolver_t *res, isc_stats_t *stats); * * Requires: * \li 'res' is valid. - * - *\li stats is a valid statistics supporting resolver statistics counters + * \li stats is a valid statistics supporting resolver statistics counters * (see dns/stats.h). */ @@ -600,8 +585,7 @@ dns_resolver_getstats(dns_resolver_t *res, isc_stats_t **statsp); * * Requires: * \li 'res' is valid. - * - *\li 'statsp' != NULL && '*statsp' != NULL + * \li 'statsp' != NULL && '*statsp' != NULL */ void @@ -623,8 +607,7 @@ dns_resolver_setquerystats(dns_resolver_t *res, dns_stats_t *stats); * * Requires: * \li 'res' is valid. - * - *\li stats is a valid statistics created by dns_rdatatypestats_create(). + * \li 'stats' is a valid statistics created by dns_rdatatypestats_create(). */ void @@ -636,8 +619,36 @@ dns_resolver_getquerystats(dns_resolver_t *res, dns_stats_t **statsp); * * Requires: * \li 'res' is valid. + * \li 'statsp' != NULL && '*statsp' != NULL + */ + +void +dns_resolver_setqueryrttstats(dns_resolver_t *res, isc_histomulti_t *hmin, + isc_histomulti_t *hmout); +/*%< + * Set a query RTT statistics histograms 'hmin' and 'hmout' for 'res'. Once the + * statistic histograms are installed, the resolver will start updating them + * with the incoming and outgoing queries' RTT values accordingly. + * + * Requires: + * \li 'res' is valid. + * \li 'stats' is a valid statistics created by dns_rdatatypestats_create(). + * \li 'hmin' is a valid isc_histomulti_t object. + * \li 'hmout' is a valid isc_histomulti_t object. + */ + +void +dns_resolver_getqueryrttstats(dns_resolver_t *res, isc_histomulti_t **hmpin, + isc_histomulti_t **hmpout); +/*%< + * Get the query RTT statistics histograms for 'res'. If the histograms are set + * then the corresponding '*hmp{in,out}' are attached to them; otherwise, + * '*hmp{in,out}' are untouched. * - *\li 'statsp' != NULL && '*statsp' != NULL + * Requires: + * \li 'res' is valid. + * \li 'hmpin' == NULL || '*hmpin' == NULL + * \li 'hmpout' == NULL || '*hmpout' == NULL */ void diff --git a/lib/dns/include/dns/stats.h b/lib/dns/include/dns/stats.h index b40f5bc9991..0bbdde4638c 100644 --- a/lib/dns/include/dns/stats.h +++ b/lib/dns/include/dns/stats.h @@ -19,6 +19,7 @@ #include +#include #include /*% @@ -52,30 +53,24 @@ enum { dns_resstatscounter_dispabort = 21, dns_resstatscounter_dispsockfail = 22, dns_resstatscounter_querytimeout = 23, - dns_resstatscounter_queryrtt0 = 24, - dns_resstatscounter_queryrtt1 = 25, - dns_resstatscounter_queryrtt2 = 26, - dns_resstatscounter_queryrtt3 = 27, - dns_resstatscounter_queryrtt4 = 28, - dns_resstatscounter_queryrtt5 = 29, - dns_resstatscounter_nfetch = 30, - dns_resstatscounter_disprequdp = 31, - dns_resstatscounter_dispreqtcp = 32, - dns_resstatscounter_buckets = 33, - dns_resstatscounter_refused = 34, - dns_resstatscounter_cookienew = 35, - dns_resstatscounter_cookieout = 36, - dns_resstatscounter_cookiein = 37, - dns_resstatscounter_cookieok = 38, - dns_resstatscounter_badvers = 39, - dns_resstatscounter_badcookie = 40, - dns_resstatscounter_zonequota = 41, - dns_resstatscounter_serverquota = 42, - dns_resstatscounter_clientquota = 43, - dns_resstatscounter_nextitem = 44, - dns_resstatscounter_priming = 45, - dns_resstatscounter_forwardonlyfail = 46, - dns_resstatscounter_max = 47, + dns_resstatscounter_nfetch = 24, + dns_resstatscounter_disprequdp = 25, + dns_resstatscounter_dispreqtcp = 26, + dns_resstatscounter_buckets = 27, + dns_resstatscounter_refused = 28, + dns_resstatscounter_cookienew = 29, + dns_resstatscounter_cookieout = 30, + dns_resstatscounter_cookiein = 31, + dns_resstatscounter_cookieok = 32, + dns_resstatscounter_badvers = 33, + dns_resstatscounter_badcookie = 34, + dns_resstatscounter_zonequota = 35, + dns_resstatscounter_serverquota = 36, + dns_resstatscounter_clientquota = 37, + dns_resstatscounter_nextitem = 38, + dns_resstatscounter_priming = 39, + dns_resstatscounter_forwardonlyfail = 40, + dns_resstatscounter_max = 41, /* * DNSSEC stats. @@ -203,6 +198,24 @@ enum { dns_sizecounter_out_max = DNS_SIZEHISTO_MAXOUT + 1, }; +/* + * This gives enough amount of buckets in the lower end of the values (which + * are the values up to about 30000 milliseconds (MAXIMUM_QUERY_TIMEOUT) to make + * sense for the resolver) to be useful and not to be too many. E.g. bucket + * number 30 covers values between 56 and 59, while bucket number 102 covers the + * values between 28672 and 30719. + */ +#define DNS_RTTHISTO_SIGBITS 3 +#define DNS_RTTHISTO_MAX 102 + +/* + * For consistency with other stats counters + */ +enum { + dns_queryrttcounter_in_max = DNS_RTTHISTO_MAX + 1, + dns_queryrttcounter_out_max = DNS_RTTHISTO_MAX + 1, +}; + /*% * Attributes for statistics counters of RRset and Rdatatype types. * diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 68efd7580f9..3dc51cdb8bf 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -207,7 +208,11 @@ #define DEFAULT_QUERY_TIMEOUT (MAX_SINGLE_QUERY_TIMEOUT + 1000U) #endif /* ifndef DEFAULT_QUERY_TIMEOUT */ -/* The maximum time in seconds for the whole query to live. */ +/* + * The maximum time in seconds for the whole query to live. + * + * Note: if increased, DNS_RTTHISTO_MAX should also be updated. + */ #ifndef MAXIMUM_QUERY_TIMEOUT #define MAXIMUM_QUERY_TIMEOUT 30000 #endif /* ifndef MAXIMUM_QUERY_TIMEOUT */ @@ -598,6 +603,8 @@ struct dns_resolver { isc_result_t quotaresp[2]; isc_stats_t *stats; dns_stats_t *querystats; + isc_histomulti_t *queryinrttstats; + isc_histomulti_t *queryoutrttstats; /* Additions for serve-stale feature. */ unsigned int retryinterval; /* in milliseconds */ @@ -1255,31 +1262,17 @@ fctx_cancelquery(resquery_t **queryp, isc_time_t *finish, bool no_response, * We have both the start and finish times for this * packet, so we can compute a real RTT. */ - unsigned int rttms; - rtt = (unsigned int)isc_time_microdiff(finish, &query->start); - rttms = rtt / US_PER_MS; factor = DNS_ADB_RTTADJDEFAULT; - if (rttms < DNS_RESOLVER_QRYRTTCLASS0) { - inc_stats(fctx->res, - dns_resstatscounter_queryrtt0); - } else if (rttms < DNS_RESOLVER_QRYRTTCLASS1) { - inc_stats(fctx->res, - dns_resstatscounter_queryrtt1); - } else if (rttms < DNS_RESOLVER_QRYRTTCLASS2) { - inc_stats(fctx->res, - dns_resstatscounter_queryrtt2); - } else if (rttms < DNS_RESOLVER_QRYRTTCLASS3) { - inc_stats(fctx->res, - dns_resstatscounter_queryrtt3); - } else if (rttms < DNS_RESOLVER_QRYRTTCLASS4) { - inc_stats(fctx->res, - dns_resstatscounter_queryrtt4); - } else { - inc_stats(fctx->res, - dns_resstatscounter_queryrtt5); + if (fctx->res->queryoutrttstats != NULL) { + const unsigned int rttms = rtt / US_PER_MS; + isc_histomulti_inc( + fctx->res->queryoutrttstats, + rttms < MAXIMUM_QUERY_TIMEOUT + ? rttms + : MAXIMUM_QUERY_TIMEOUT); } } else { uint32_t value; @@ -9642,6 +9635,12 @@ dns_resolver__destroy(dns_resolver_t *res) { dns_nametree_detach(&res->algorithms); dns_nametree_detach(&res->digests); + if (res->queryoutrttstats != NULL) { + isc_histomulti_detach(&res->queryoutrttstats); + } + if (res->queryinrttstats != NULL) { + isc_histomulti_detach(&res->queryinrttstats); + } if (res->querystats != NULL) { dns_stats_detach(&res->querystats); } @@ -10884,6 +10883,32 @@ dns_resolver_getquerystats(dns_resolver_t *res, dns_stats_t **statsp) { } } +void +dns_resolver_setqueryrttstats(dns_resolver_t *res, isc_histomulti_t *hmin, + isc_histomulti_t *hmout) { + REQUIRE(VALID_RESOLVER(res)); + REQUIRE(res->queryinrttstats == NULL); + REQUIRE(res->queryoutrttstats == NULL); + + isc_histomulti_attach(hmin, &res->queryinrttstats); + isc_histomulti_attach(hmout, &res->queryoutrttstats); +} + +void +dns_resolver_getqueryrttstats(dns_resolver_t *res, isc_histomulti_t **hmpin, + isc_histomulti_t **hmpout) { + REQUIRE(VALID_RESOLVER(res)); + REQUIRE(hmpin == NULL || *hmpin == NULL); + REQUIRE(hmpout == NULL || *hmpout == NULL); + + if (hmpin != NULL && res->queryinrttstats != NULL) { + isc_histomulti_attach(res->queryinrttstats, hmpin); + } + if (hmpout != NULL && res->queryoutrttstats != NULL) { + isc_histomulti_attach(res->queryoutrttstats, hmpout); + } +} + void dns_resolver_freefresp(dns_fetchresponse_t **frespp) { REQUIRE(frespp != NULL);