From 85e35d4c27ff02157296be9df1922e3970a3a365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 3 Nov 2022 17:42:12 +0100 Subject: [PATCH 1/2] Propagate the shutdown event to the recursing ns_client(s) Send the ns_query_cancel() on the recursing clients when we initiate the named shutdown for faster shutdown. When we are shutting down the resolver, we cancel all the outstanding fetches, and the ISC_R_CANCEL events doesn't propagate to the ns_client callback. In the future, the better solution how to fix this would be to look at the shutdown paths and let them all propagate from bottom (loopmgr) to top (f.e. ns_client). (cherry picked from commit d861d403bb9a7912e29a06aba6caf6d502839f1b) --- lib/ns/client.c | 28 +++++++++++++--------------- lib/ns/include/ns/client.h | 12 +++++++++--- lib/ns/interfacemgr.c | 6 +++++- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/lib/ns/client.c b/lib/ns/client.c index 2b3da21b21..39ac87f540 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -121,8 +121,6 @@ atomic_uint_fast64_t ns_client_requests = 0; static void clientmgr_attach(ns_clientmgr_t *source, ns_clientmgr_t **targetp); static void -clientmgr_detach(ns_clientmgr_t **mp); -static void clientmgr_destroy(ns_clientmgr_t *manager); static void ns_client_endrequest(ns_client_t *client); @@ -1665,7 +1663,7 @@ ns__client_put_cb(void *client0) { dns_message_detach(&client->message); if (client->manager != NULL) { - clientmgr_detach(&client->manager); + ns_clientmgr_detach(&client->manager); } /* @@ -2408,7 +2406,7 @@ cleanup: } if (client->manager != NULL) { - clientmgr_detach(&client->manager); + ns_clientmgr_detach(&client->manager); } isc_mem_detach(&client->mctx); if (client->sctx != NULL) { @@ -2442,8 +2440,8 @@ clientmgr_attach(ns_clientmgr_t *source, ns_clientmgr_t **targetp) { *targetp = source; } -static void -clientmgr_detach(ns_clientmgr_t **mp) { +void +ns_clientmgr_detach(ns_clientmgr_t **mp) { int32_t oldrefs; ns_clientmgr_t *mgr = *mp; *mp = NULL; @@ -2517,20 +2515,20 @@ ns_clientmgr_create(ns_server_t *sctx, isc_taskmgr_t *taskmgr, } void -ns_clientmgr_destroy(ns_clientmgr_t **managerp) { - ns_clientmgr_t *manager; +ns_clientmgr_shutdown(ns_clientmgr_t *manager) { + ns_client_t *client; - REQUIRE(managerp != NULL); - REQUIRE(VALID_MANAGER(*managerp)); - - manager = *managerp; - *managerp = NULL; + REQUIRE(VALID_MANAGER(manager)); MTRACE("destroy"); - if (isc_refcount_decrement(&manager->references) == 1) { - clientmgr_destroy(manager); + LOCK(&manager->reclock); + for (client = ISC_LIST_HEAD(manager->recursing); client != NULL; + client = ISC_LIST_NEXT(client, rlink)) + { + ns_query_cancel(client); } + UNLOCK(&manager->reclock); } isc_sockaddr_t * diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h index 8e1d5cfb10..e937f0acec 100644 --- a/lib/ns/include/ns/client.h +++ b/lib/ns/include/ns/client.h @@ -348,10 +348,16 @@ ns_clientmgr_create(ns_server_t *sctx, isc_taskmgr_t *taskmgr, */ void -ns_clientmgr_destroy(ns_clientmgr_t **managerp); +ns_clientmgr_shutdown(ns_clientmgr_t *manager); /*%< - * Destroy a client manager and all ns_client_t objects - * managed by it. + * Shutdown a client manager and all ns_client_t objects + * managed by it + */ + +void +ns_clientmgr_detach(ns_clientmgr_t **managerp); +/*%< + * Detach from a client manager. */ isc_sockaddr_t * diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c index 06f23628d0..be98841cea 100644 --- a/lib/ns/interfacemgr.c +++ b/lib/ns/interfacemgr.c @@ -386,7 +386,7 @@ ns_interfacemgr_destroy(ns_interfacemgr_t *mgr) { clearlistenon(mgr); isc_mutex_destroy(&mgr->lock); for (size_t i = 0; i < (size_t)mgr->ncpus; i++) { - ns_clientmgr_destroy(&mgr->clientmgrs[i]); + ns_clientmgr_detach(&mgr->clientmgrs[i]); } isc_mem_put(mgr->mctx, mgr->clientmgrs, mgr->ncpus * sizeof(mgr->clientmgrs[0])); @@ -455,6 +455,10 @@ ns_interfacemgr_shutdown(ns_interfacemgr_t *mgr) { if (mgr->route != NULL) { isc_nm_cancelread(mgr->route); } + + for (size_t i = 0; i < (size_t)mgr->ncpus; i++) { + ns_clientmgr_shutdown(mgr->clientmgrs[i]); + } } static void From 2cb02a417a014ef35db0f438af1c05c5a144f41a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 3 Nov 2022 18:01:22 +0100 Subject: [PATCH 2/2] Add CHANGES and release notes for [GL #3183] (cherry picked from commit d3f1639c16e7777a52d66c2dccd8b43a08a0750b) --- CHANGES | 4 ++++ doc/notes/notes-current.rst | 3 +++ 2 files changed, 7 insertions(+) diff --git a/CHANGES b/CHANGES index 953ad25c7b..d0d278bfc8 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +6040. [bug] Speed up the named shutdown time by explicitly + canceling all recursing ns_client objects for + each ns_clientmgr. [GL #3183] + 6039. [bug] Removing a catalog zone from catalog-zones without also removing the referenced zone could leave a dangling pointer. [GL #3683] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 45d6fa9b0d..9383fdc4d7 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -70,6 +70,9 @@ Bug Fixes cases a dangling pointer could cause a :iscman:`named` process crash. This has been fixed. :gl:`#3683` +- The ``named`` would wait for some outstanding recursing queries + to finish before shutting down. This has been fixed. :gl:`#3183` + Known Issues ~~~~~~~~~~~~