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 acff97edae07ed33e55631893cafa2bf2c11ede9..28bb6d297c0e18e18c1875f1329a7fa3e66a0bb4 100644 (file)
--- 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);