From: Mark Andrews Date: Fri, 16 Feb 2024 00:40:26 +0000 (+1100) Subject: Use a new memory context when flushing the cache X-Git-Tag: v9.20.0~19^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5e77edd07482a69fdf578e7680a495e00ace8b7b;p=thirdparty%2Fbind9.git Use a new memory context when flushing the cache When the cache's memory context was in over memory state when the cache was flushed it resulted in LRU cleaning removing newly entered data in the new cache straight away until the old cache had been destroyed enough to take it out of over memory state. When flushing the cache create a new memory context for the new db to prevent this. --- diff --git a/lib/dns/cache.c b/lib/dns/cache.c index acff97edae0..28bb6d297c0 100644 --- a/lib/dns/cache.c +++ b/lib/dns/cache.c @@ -68,6 +68,7 @@ struct dns_cache { isc_mutex_t lock; isc_mem_t *mctx; /* Main cache memory */ isc_mem_t *hmctx; /* Heap memory */ + isc_mem_t *tmctx; /* Tree memory */ isc_loop_t *loop; char *name; isc_refcount_t references; @@ -86,33 +87,60 @@ struct dns_cache { ***/ static isc_result_t -cache_create_db(dns_cache_t *cache, dns_db_t **db) { +cache_create_db(dns_cache_t *cache, dns_db_t **dbp, isc_mem_t **tmctxp, + isc_mem_t **hmctxp) { isc_result_t result; char *argv[1] = { 0 }; + dns_db_t *db = NULL; + isc_mem_t *tmctx = NULL, *hmctx = NULL; + + /* + * This will be the cache memory context, which is subject + * to cleaning when the configured memory limits are exceeded. + */ + isc_mem_create(&tmctx); + isc_mem_setname(tmctx, "cache"); + + /* + * This will be passed to RBTDB to use for heaps. This is separate + * from the main cache memory because it can grow quite large under + * heavy load and could otherwise cause the cache to be cleaned too + * aggressively. + */ + isc_mem_create(&hmctx); + isc_mem_setname(hmctx, "cache_heap"); /* * For databases of type "qpcache" or "rbt" (which are the * only cache implementations currently in existence) we pass * hmctx to dns_db_create() via argv[0]. */ - argv[0] = (char *)cache->hmctx; - result = dns_db_create(cache->mctx, CACHEDB_DEFAULT, dns_rootname, - dns_dbtype_cache, cache->rdclass, 1, argv, db); + argv[0] = (char *)hmctx; + result = dns_db_create(tmctx, CACHEDB_DEFAULT, dns_rootname, + dns_dbtype_cache, cache->rdclass, 1, argv, &db); if (result != ISC_R_SUCCESS) { - return (result); + goto cleanup_mctx; } - - dns_db_setservestalettl(*db, cache->serve_stale_ttl); - dns_db_setservestalerefresh(*db, cache->serve_stale_refresh); - dns_db_setloop(*db, cache->loop); - - result = dns_db_setcachestats(*db, cache->stats); + result = dns_db_setcachestats(db, cache->stats); if (result != ISC_R_SUCCESS) { - dns_db_detach(db); - return (result); + goto cleanup_db; } + dns_db_setservestalettl(db, cache->serve_stale_ttl); + dns_db_setservestalerefresh(db, cache->serve_stale_refresh); + dns_db_setloop(db, cache->loop); + *dbp = db; + *hmctxp = hmctx; + *tmctxp = tmctx; return (ISC_R_SUCCESS); + +cleanup_db: + dns_db_detach(&db); +cleanup_mctx: + isc_mem_detach(&hmctx); + isc_mem_detach(&tmctx); + + return (result); } static void @@ -121,7 +149,12 @@ cache_destroy(dns_cache_t *cache) { isc_mutex_destroy(&cache->lock); isc_mem_free(cache->mctx, cache->name); isc_loop_detach(&cache->loop); - isc_mem_detach(&cache->hmctx); + if (cache->hmctx != NULL) { + isc_mem_detach(&cache->hmctx); + } + if (cache->tmctx != NULL) { + isc_mem_detach(&cache->tmctx); + } isc_mem_putanddetach(&cache->mctx, cache, sizeof(*cache)); } @@ -130,32 +163,21 @@ dns_cache_create(isc_loopmgr_t *loopmgr, dns_rdataclass_t rdclass, const char *cachename, dns_cache_t **cachep) { isc_result_t result; dns_cache_t *cache = NULL; - isc_mem_t *mctx = NULL, *hmctx = NULL; + isc_mem_t *mctx = NULL; REQUIRE(loopmgr != NULL); REQUIRE(cachename != NULL); REQUIRE(cachep != NULL && *cachep == NULL); /* - * This will be the main cache memory context, which is subject - * to cleaning when the configured memory limits are exceeded. + * Self. */ isc_mem_create(&mctx); - isc_mem_setname(mctx, "cache"); - - /* - * This will be passed to RBTDB to use for heaps. This is separate - * from the main cache memory because it can grow quite large under - * heavy load and could otherwise cause the cache to be cleaned too - * aggressively. - */ - isc_mem_create(&hmctx); - isc_mem_setname(hmctx, "cache_heap"); + isc_mem_setname(mctx, "cache-self"); cache = isc_mem_get(mctx, sizeof(*cache)); *cache = (dns_cache_t){ .mctx = mctx, - .hmctx = hmctx, .rdclass = rdclass, .name = isc_mem_strdup(mctx, cachename), .loop = isc_loop_ref(isc_loop_main(loopmgr)), @@ -170,7 +192,8 @@ dns_cache_create(isc_loopmgr_t *loopmgr, dns_rdataclass_t rdclass, /* * Create the database */ - result = cache_create_db(cache, &cache->db); + result = cache_create_db(cache, &cache->db, &cache->tmctx, + &cache->hmctx); if (result != ISC_R_SUCCESS) { goto cleanup; } @@ -190,7 +213,7 @@ cache_cleanup(dns_cache_t *cache) { isc_refcount_destroy(&cache->references); cache->magic = 0; - isc_mem_clearwater(cache->mctx); + isc_mem_clearwater(cache->tmctx); dns_db_detach(&cache->db); cache_destroy(cache); @@ -220,6 +243,17 @@ dns_cache_getname(dns_cache_t *cache) { return (cache->name); } +static void +updatewater(dns_cache_t *cache) { + size_t hi = cache->size - (cache->size >> 3); /* ~ 7/8ths. */ + size_t lo = cache->size - (cache->size >> 2); /* ~ 3/4ths. */ + if (cache->size == 0U || hi == 0U || lo == 0U) { + isc_mem_clearwater(cache->tmctx); + } else { + isc_mem_setwater(cache->tmctx, hi, lo); + } +} + void dns_cache_setcachesize(dns_cache_t *cache, size_t size) { REQUIRE(VALID_CACHE(cache)); @@ -234,15 +268,8 @@ dns_cache_setcachesize(dns_cache_t *cache, size_t size) { LOCK(&cache->lock); cache->size = size; + updatewater(cache); UNLOCK(&cache->lock); - - size_t hi = size - (size >> 3); /* Approximately 7/8ths. */ - size_t lo = size - (size >> 2); /* Approximately 3/4ths. */ - if (size == 0U || hi == 0U || lo == 0U) { - isc_mem_clearwater(cache->mctx); - } else { - isc_mem_setwater(cache->mctx, hi, lo); - } } size_t @@ -309,19 +336,29 @@ dns_cache_getservestalerefresh(dns_cache_t *cache) { isc_result_t dns_cache_flush(dns_cache_t *cache) { dns_db_t *db = NULL, *olddb; + isc_mem_t *tmctx = NULL, *oldtmctx; + isc_mem_t *hmctx = NULL, *oldhmctx; isc_result_t result; - result = cache_create_db(cache, &db); + result = cache_create_db(cache, &db, &tmctx, &hmctx); if (result != ISC_R_SUCCESS) { return (result); } LOCK(&cache->lock); + isc_mem_clearwater(cache->tmctx); + oldhmctx = cache->hmctx; + cache->hmctx = hmctx; + oldtmctx = cache->tmctx; + cache->tmctx = tmctx; + updatewater(cache); olddb = cache->db; cache->db = db; UNLOCK(&cache->lock); dns_db_detach(&olddb); + isc_mem_detach(&oldhmctx); + isc_mem_detach(&oldtmctx); return (ISC_R_SUCCESS); } @@ -583,7 +620,7 @@ dns_cache_dumpstats(dns_cache_t *cache, FILE *fp) { fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)dns_db_hashsize(cache->db), "cache database hash buckets"); - fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)isc_mem_inuse(cache->mctx), + fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)isc_mem_inuse(cache->tmctx), "cache tree memory in use"); fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)isc_mem_inuse(cache->hmctx), @@ -643,7 +680,7 @@ dns_cache_renderxml(dns_cache_t *cache, void *writer0) { dns_db_nodecount(cache->db, dns_dbtree_nsec), writer)); TRY0(renderstat("CacheBuckets", dns_db_hashsize(cache->db), writer)); - TRY0(renderstat("TreeMemInUse", isc_mem_inuse(cache->mctx), writer)); + TRY0(renderstat("TreeMemInUse", isc_mem_inuse(cache->tmctx), writer)); TRY0(renderstat("HeapMemInUse", isc_mem_inuse(cache->hmctx), writer)); error: @@ -715,7 +752,7 @@ dns_cache_renderjson(dns_cache_t *cache, void *cstats0) { CHECKMEM(obj); json_object_object_add(cstats, "CacheBuckets", obj); - obj = json_object_new_int64(isc_mem_inuse(cache->mctx)); + obj = json_object_new_int64(isc_mem_inuse(cache->tmctx)); CHECKMEM(obj); json_object_object_add(cstats, "TreeMemInUse", obj);