From e7927a5b1811b8db30ca0632b32c774ad3680222 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 6 Mar 2024 13:26:04 +0100 Subject: [PATCH] Move the task creation into cache_create_db() The dns_cache_flush() drops the old database and creates a new one, but it forgets to create the task(s) that runs the node pruning and cleaning the rbtdb when flushing it next time. This causes the cleaning to skip cleaning the parent nodes (with .down == NULL) leading to increased memory usage over time until the database is unable to keep up and just stays overmem all the time. (cherry picked from commit 79040a669ca26b75b06c25d27418cee7ab658013) --- lib/dns/cache.c | 76 +++++++++++++++++++++++++------------ lib/dns/include/dns/cache.h | 23 +++++------ 2 files changed, 60 insertions(+), 39 deletions(-) diff --git a/lib/dns/cache.c b/lib/dns/cache.c index fd944bd46f..2a6344e51f 100644 --- a/lib/dns/cache.c +++ b/lib/dns/cache.c @@ -129,6 +129,7 @@ struct dns_cache { isc_mutex_t lock; isc_mem_t *mctx; /* Main cache memory */ isc_mem_t *hmctx; /* Heap memory */ + isc_taskmgr_t *taskmgr; char *name; isc_refcount_t references; isc_refcount_t live_tasks; @@ -169,13 +170,47 @@ water(void *arg, int mark); static isc_result_t cache_create_db(dns_cache_t *cache, dns_db_t **db) { isc_result_t result; + isc_task_t *dbtask = NULL; + isc_task_t *prunetask = NULL; + result = dns_db_create(cache->mctx, cache->db_type, dns_rootname, dns_dbtype_cache, cache->rdclass, cache->db_argc, cache->db_argv, db); - if (result == ISC_R_SUCCESS) { - dns_db_setservestalettl(*db, cache->serve_stale_ttl); - dns_db_setservestalerefresh(*db, cache->serve_stale_refresh); + if (result != ISC_R_SUCCESS) { + return (result); } + + dns_db_setservestalettl(*db, cache->serve_stale_ttl); + dns_db_setservestalerefresh(*db, cache->serve_stale_refresh); + + if (cache->taskmgr == NULL) { + return (ISC_R_SUCCESS); + } + + result = isc_task_create(cache->taskmgr, 1, &dbtask); + if (result != ISC_R_SUCCESS) { + goto cleanup_db; + } + isc_task_setname(dbtask, "cache_dbtask", NULL); + + result = isc_task_create(cache->taskmgr, UINT_MAX, &prunetask); + if (result != ISC_R_SUCCESS) { + goto cleanup_dbtask; + } + isc_task_setname(prunetask, "cache_prunetask", NULL); + + dns_db_settask(*db, dbtask, prunetask); + + isc_task_detach(&prunetask); + isc_task_detach(&dbtask); + + return (ISC_R_SUCCESS); + +cleanup_dbtask: + isc_task_detach(&dbtask); +cleanup_db: + dns_db_detach(db); + return (result); } @@ -192,6 +227,7 @@ dns_cache_create(isc_mem_t *cmctx, isc_mem_t *hmctx, isc_taskmgr_t *taskmgr, REQUIRE(*cachep == NULL); REQUIRE(cmctx != NULL); REQUIRE(hmctx != NULL); + REQUIRE(taskmgr != NULL || strcmp(db_type, "rbt") != 0); REQUIRE(cachename != NULL); cache = isc_mem_get(cmctx, sizeof(*cache)); @@ -200,6 +236,11 @@ dns_cache_create(isc_mem_t *cmctx, isc_mem_t *hmctx, isc_taskmgr_t *taskmgr, isc_mem_attach(cmctx, &cache->mctx); isc_mem_attach(hmctx, &cache->hmctx); + cache->taskmgr = NULL; + if (taskmgr != NULL) { + isc_taskmgr_attach(taskmgr, &cache->taskmgr); + } + cache->name = NULL; if (cachename != NULL) { cache->name = isc_mem_strdup(cmctx, cachename); @@ -256,28 +297,6 @@ dns_cache_create(isc_mem_t *cmctx, isc_mem_t *hmctx, isc_taskmgr_t *taskmgr, if (result != ISC_R_SUCCESS) { goto cleanup_dbargv; } - if (taskmgr != NULL) { - isc_task_t *dbtask = NULL; - isc_task_t *prunetask = NULL; - - result = isc_task_create(taskmgr, 1, &dbtask); - if (result != ISC_R_SUCCESS) { - goto cleanup_db; - } - isc_task_setname(dbtask, "cache_dbtask", NULL); - - result = isc_task_create(taskmgr, UINT_MAX, &prunetask); - if (result != ISC_R_SUCCESS) { - isc_task_detach(&dbtask); - goto cleanup_db; - } - isc_task_setname(prunetask, "cache_prunetask", NULL); - - dns_db_settask(cache->db, dbtask, prunetask); - isc_task_detach(&dbtask); - isc_task_detach(&prunetask); - } - cache->magic = CACHE_MAGIC; /* @@ -321,6 +340,9 @@ cleanup_lock: if (cache->name != NULL) { isc_mem_free(cmctx, cache->name); } + if (cache->taskmgr != NULL) { + isc_taskmgr_detach(&cache->taskmgr); + } isc_mem_detach(&cache->hmctx); isc_mem_putanddetach(&cache->mctx, cache, sizeof(*cache)); return (result); @@ -387,6 +409,10 @@ cache_free(dns_cache_t *cache) { isc_stats_detach(&cache->stats); } + if (cache->taskmgr != NULL) { + isc_taskmgr_detach(&cache->taskmgr); + } + isc_mutex_destroy(&cache->lock); cache->magic = 0; diff --git a/lib/dns/include/dns/cache.h b/lib/dns/include/dns/cache.h index 8fc96574d8..880c1ffd10 100644 --- a/lib/dns/include/dns/cache.h +++ b/lib/dns/include/dns/cache.h @@ -61,25 +61,20 @@ dns_cache_create(isc_mem_t *cmctx, isc_mem_t *hmctx, isc_taskmgr_t *taskmgr, const char *cachename, const char *db_type, unsigned int db_argc, char **db_argv, dns_cache_t **cachep); /*%< - * Create a new DNS cache. - * - * dns_cache_create2() will create a named cache. - * - * dns_cache_create3() will create a named cache using two separate memory - * contexts, one for cache data which can be cleaned and a separate one for - * memory allocated for the heap (which can grow without an upper limit and - * has no mechanism for shrinking). - * - * dns_cache_create() is a backward compatible version that internally - * specifies an empty cache name and a single memory context. + * Create a new named DNS cache using two separate memory contexts, one for + * cache data which can be cleaned and a separate one for memory allocated for + * the heap (which can grow without an upper limit and has no mechanism for + * shrinking). * * Requires: * - *\li 'cmctx' (and 'hmctx' if applicable) is a valid memory context. + *\li 'cmctx' and 'hmctx' are valid memory contexts. + * + *\li 'taskmgr' is a valid task manager (if 'db_type' is "rbt"). * *\li 'taskmgr' is a valid task manager and 'timermgr' is a valid timer - * manager, or both are NULL. If NULL, no periodic cleaning of the - * cache will take place. + * manager, or both are NULL (if 'db_type' is not "rbt"). If NULL, no + * periodic cleaning of the cache will take place. * *\li 'cachename' is a valid string. This must not be NULL. *