Create a second pruning task for rbtdb with unlimited quantum

Previously, rbtdb->task had quantum of 1 because it was originally used
just for freeing RBTDB contents, which can happen on a "best effort"
basis (does not need to be prioritized).  However, when tree pruning was
implemented, it also started sending events to that task, enabling the
latter to become clogged up with a significant event backlog because it
only pruned a single RBTDB node per event.

To prioritize tree pruning (as it is necessary for enforcing the
configured memory use limit for the cache memory context), create a
second task with a virtually unlimited quantum (UINT_MAX) and send the
tree-pruning events to this new task, to ensure that all nodes scheduled
for pruning will be processed before further nodes are queued in a
similar fashion.

This change enables dropping the prunenodes list and restoring the
originally-used logic that allocates and sends a separate event for each
node to prune.

(cherry picked from commit 231b2375e5b9b98096711f5e883911134adb6392)

diff --git a/bin/tests/system/dyndb/driver/db.c b/bin/tests/system/dyndb/driver/db.c
index bed7d3e3a0f435db397146058ff2f815d683bc14..5f01185dd88a258a72e088f465b00927afc52308 100644 (file)
--- a/bin/tests/system/dyndb/driver/db.c
+++ b/bin/tests/system/dyndb/driver/db.c
@@ -429,12 +429,12 @@ overmem(dns_db_t *db, bool over) {
 }
 
 static void
-settask(dns_db_t *db, isc_task_t *task) {
+settask(dns_db_t *db, isc_task_t *task, isc_task_t *prunetask) {
        sampledb_t *sampledb = (sampledb_t *)db;
 
        REQUIRE(VALID_SAMPLEDB(sampledb));
 
-       dns_db_settask(sampledb->rbtdb, task);
+       dns_db_settask(sampledb->rbtdb, task, prunetask);
 }
 
 static isc_result_t

diff --git a/lib/dns/cache.c b/lib/dns/cache.c
index 417cdaff56989ea88b1601d67cfe8276fb5687c5..f69b2f912b25e4f33ba5039b269919292c92ece5 100644 (file)
--- a/lib/dns/cache.c
+++ b/lib/dns/cache.c
@@ -189,7 +189,6 @@ dns_cache_create(isc_mem_t *cmctx, isc_mem_t *hmctx, isc_taskmgr_t *taskmgr,
        isc_result_t result;
        dns_cache_t *cache;
        int i, extra = 0;
-       isc_task_t *dbtask;
 
        REQUIRE(cachep != NULL);
        REQUIRE(*cachep == NULL);
@@ -261,15 +260,25 @@ dns_cache_create(isc_mem_t *cmctx, isc_mem_t *hmctx, isc_taskmgr_t *taskmgr,
                goto cleanup_dbargv;
        }
        if (taskmgr != NULL) {
-               dbtask = 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);
-               dns_db_settask(cache->db, dbtask);
+
+               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->filename = NULL;

diff --git a/lib/dns/db.c b/lib/dns/db.c
index 41dbaa3d688a0d3e4a2a9093628bdab050556f8c..f90add170e7f29f70ff302d3e4607cea4b1f386a 100644 (file)
--- a/lib/dns/db.c
+++ b/lib/dns/db.c
@@ -847,10 +847,10 @@ dns_db_adjusthashsize(dns_db_t *db, size_t size) {
 }
 
 void
-dns_db_settask(dns_db_t *db, isc_task_t *task) {
+dns_db_settask(dns_db_t *db, isc_task_t *task, isc_task_t *prunetask) {
        REQUIRE(DNS_DB_VALID(db));
 
-       (db->methods->settask)(db, task);
+       (db->methods->settask)(db, task, prunetask);
 }
 
 isc_result_t

diff --git a/lib/dns/include/dns/db.h b/lib/dns/include/dns/db.h
index 84b1cb10a08f6bd2a1b1a907752927a0ec3d94aa..ae276393c2c48355ef699bf31619d1a9e729eaf6 100644 (file)
--- a/lib/dns/include/dns/db.h
+++ b/lib/dns/include/dns/db.h
@@ -137,7 +137,7 @@ typedef struct dns_dbmethods {
        unsigned int (*nodecount)(dns_db_t *db);
        bool (*ispersistent)(dns_db_t *db);
        void (*overmem)(dns_db_t *db, bool overmem);
-       void (*settask)(dns_db_t *db, isc_task_t *);
+       void (*settask)(dns_db_t *db, isc_task_t *, isc_task_t *);
        isc_result_t (*getoriginnode)(dns_db_t *db, dns_dbnode_t **nodep);
        void (*transfernode)(dns_db_t *db, dns_dbnode_t **sourcep,
                             dns_dbnode_t **targetp);
@@ -1428,13 +1428,14 @@ dns_db_adjusthashsize(dns_db_t *db, size_t size);
  */
 
 void
-dns_db_settask(dns_db_t *db, isc_task_t *task);
+dns_db_settask(dns_db_t *db, isc_task_t *task, isc_task_t *prunetask);
 /*%<
  * If task is set then the final detach maybe performed asynchronously.
  *
  * Requires:
  * \li 'db' is a valid database.
- * \li 'task' to be valid or NULL.
+ * \li 'task' to be valid or NULL (default task to send events to).
+ * \li 'prunetask' to be valid or NULL (task to send tree-pruning events to).
  */
 
 bool

diff --git a/lib/dns/include/dns/rbt.h b/lib/dns/include/dns/rbt.h
index 6cfd40ffddfc95b000795dff65a2578bcf8cbcc0..4a6b0781c78d9bdcb1637be0e7d611a0e025d8ae 100644 (file)
--- a/lib/dns/include/dns/rbt.h
+++ b/lib/dns/include/dns/rbt.h
@@ -140,12 +140,6 @@ struct dns_rbtnode {
         */
        ISC_LINK(dns_rbtnode_t) deadlink;
 
-       /*%
-        * This linked list is used to store nodes from which tree pruning can
-        * be started.
-        */
-       ISC_LINK(dns_rbtnode_t) prunelink;
-
        /*@{*/
        /*!
         * These values are used in the RBT DB implementation.  The appropriate

diff --git a/lib/dns/rbt.c b/lib/dns/rbt.c
index 780a950a48bae70a9aecd73f49a4c83598ff7178..d5d18b836eb8dfc429f573fd89b1ce76e0c57023 100644 (file)
--- a/lib/dns/rbt.c
+++ b/lib/dns/rbt.c
@@ -2307,7 +2307,6 @@ create_node(isc_mem_t *mctx, const dns_name_t *name, dns_rbtnode_t **nodep) {
        HASHVAL(node) = 0;
 
        ISC_LINK_INIT(node, deadlink);
-       ISC_LINK_INIT(node, prunelink);
 
        LOCKNUM(node) = 0;
        WILD(node) = 0;

diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
index b924ff4b964a5701fe371376ad5e8a977c722516..66d8a62b3f4643b5ca168e18bb665c2b6106360c 100644 (file)
--- a/lib/dns/rbtdb.c
+++ b/lib/dns/rbtdb.c
@@ -500,6 +500,7 @@ struct dns_rbtdb {
        rbtdb_version_t *future_version;
        rbtdb_versionlist_t open_versions;
        isc_task_t *task;
+       isc_task_t *prunetask;
        dns_dbnode_t *soanode;
        dns_dbnode_t *nsnode;
 
@@ -529,10 +530,6 @@ struct dns_rbtdb {
         */
        rbtnodelist_t *deadnodes;
 
-       /* List of nodes from which recursive tree pruning can be started from.
-        * Locked by tree_lock. */
-       rbtnodelist_t *prunenodes;
-
        /*
         * Heaps.  These are used for TTL based expiry in a cache,
         * or for zone resigning in a zone DB.  hmctx is the memory
@@ -1140,14 +1137,6 @@ free_rbtdb(dns_rbtdb_t *rbtdb, bool log, isc_event_t *event) {
                }
        }
 
-       for (i = 0; i < rbtdb->node_lock_count; i++) {
-               node = ISC_LIST_HEAD(rbtdb->prunenodes[i]);
-               while (node != NULL) {
-                       ISC_LIST_UNLINK(rbtdb->prunenodes[i], node, prunelink);
-                       node = ISC_LIST_HEAD(rbtdb->prunenodes[i]);
-               }
-       }
-
        if (event == NULL) {
                rbtdb->quantum = (rbtdb->task != NULL) ? 100 : 0;
        }
@@ -1234,17 +1223,6 @@ free_rbtdb(dns_rbtdb_t *rbtdb, bool log, isc_event_t *event) {
                isc_mem_put(rbtdb->common.mctx, rbtdb->deadnodes,
                            rbtdb->node_lock_count * sizeof(rbtnodelist_t));
        }
-       /*
-        * Clean up prune node buckets.
-        */
-       if (rbtdb->prunenodes != NULL) {
-               for (i = 0; i < rbtdb->node_lock_count; i++) {
-                       INSIST(ISC_LIST_EMPTY(rbtdb->prunenodes[i]));
-               }
-               isc_mem_put(rbtdb->common.mctx, rbtdb->prunenodes,
-                           rbtdb->node_lock_count *
-                                   sizeof(rbtdb->prunenodes[i]));
-       }
        /*
         * Clean up heap objects.
         */
@@ -1273,6 +1251,9 @@ free_rbtdb(dns_rbtdb_t *rbtdb, bool log, isc_event_t *event) {
        if (rbtdb->task != NULL) {
                isc_task_detach(&rbtdb->task);
        }
+       if (rbtdb->prunetask != NULL) {
+               isc_task_detach(&rbtdb->prunetask);
+       }
 
        RBTDB_DESTROYLOCK(&rbtdb->lock);
        rbtdb->common.magic = 0;
@@ -1897,7 +1878,6 @@ delete_node(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node) {
        isc_result_t result = ISC_R_UNEXPECTED;
 
        INSIST(!ISC_LINK_LINKED(node, deadlink));
-       INSIST(!ISC_LINK_LINKED(node, prunelink));
 
        if (isc_log_wouldlog(dns_lctx, ISC_LOG_DEBUG(1))) {
                char printname[DNS_NAME_FORMATSIZE];
@@ -1995,10 +1975,6 @@ is_last_node_on_its_level(dns_rbtnode_t *node) {
                node->left == NULL && node->right == NULL);
 }
 
-/*%
- * The tree lock must be held when this function is called as it reads and
- * updates rbtdb->prunenodes.
- */
 static void
 send_to_prune_tree(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node,
                   isc_rwlocktype_t nlocktype) {
@@ -8243,7 +8219,7 @@ adjusthashsize(dns_db_t *db, size_t size) {
 }
 
 static void
-settask(dns_db_t *db, isc_task_t *task) {
+settask(dns_db_t *db, isc_task_t *task, isc_task_t *prunetask) {
        dns_rbtdb_t *rbtdb;
 
        rbtdb = (dns_rbtdb_t *)db;
@@ -8257,6 +8233,12 @@ settask(dns_db_t *db, isc_task_t *task) {
        if (task != NULL) {
                isc_task_attach(task, &rbtdb->task);
        }
+       if (rbtdb->prunetask != NULL) {
+               isc_task_detach(&rbtdb->prunetask);
+       }
+       if (prunetask != NULL) {
+               isc_task_attach(prunetask, &rbtdb->prunetask);
+       }
        RBTDB_UNLOCK(&rbtdb->lock, isc_rwlocktype_write);
 }
 
@@ -8831,12 +8813,6 @@ dns_rbtdb_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type,
                ISC_LIST_INIT(rbtdb->deadnodes[i]);
        }
 
-       rbtdb->prunenodes = isc_mem_get(
-               mctx, rbtdb->node_lock_count * sizeof(rbtdb->prunenodes[0]));
-       for (i = 0; i < (int)rbtdb->node_lock_count; i++) {
-               ISC_LIST_INIT(rbtdb->prunenodes[i]);
-       }
-
        rbtdb->active = rbtdb->node_lock_count;
 
        for (i = 0; i < (int)(rbtdb->node_lock_count); i++) {
@@ -8944,6 +8920,7 @@ dns_rbtdb_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type,
        isc_refcount_init(&rbtdb->references, 1);
        rbtdb->attributes = 0;
        rbtdb->task = NULL;
+       rbtdb->prunetask = NULL;
        rbtdb->serve_stale_ttl = 0;
 
        /*

diff --git a/lib/dns/sdb.c b/lib/dns/sdb.c
index 2c6802ff9cc30301863bece611798b3818695381..7822d6ef14398a2a9028a9b52e8088f2c3447b9d 100644 (file)
--- a/lib/dns/sdb.c
+++ b/lib/dns/sdb.c
@@ -1262,9 +1262,10 @@ overmem(dns_db_t *db, bool over) {
 }
 
 static void
-settask(dns_db_t *db, isc_task_t *task) {
+settask(dns_db_t *db, isc_task_t *task, isc_task_t *prunetask) {
        UNUSED(db);
        UNUSED(task);
+       UNUSED(prunetask);
 }
 
 static dns_dbmethods_t sdb_methods = {

diff --git a/lib/dns/sdlz.c b/lib/dns/sdlz.c
index 8ba7aaeda4c9064e77e0cb15fa9ea929b2e36996..106e24ba704ca58ea6ff8707f5f4c219d24d3c07 100644 (file)
--- a/lib/dns/sdlz.c
+++ b/lib/dns/sdlz.c
@@ -1212,9 +1212,10 @@ overmem(dns_db_t *db, bool over) {
 }
 
 static void
-settask(dns_db_t *db, isc_task_t *task) {
+settask(dns_db_t *db, isc_task_t *task, isc_task_t *prunetask) {
        UNUSED(db);
        UNUSED(task);
+       UNUSED(prunetask);
 }
 
 /*

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index ea0245664804dcf49046e1a8094bab324e1bd44e..b457095907b196c5f2add7c411571954192a3812 100644 (file)
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -2316,7 +2316,7 @@ zone_load(dns_zone_t *zone, unsigned int flags, bool locked) {
                              isc_result_totext(result));
                goto cleanup;
        }
-       dns_db_settask(db, zone->task);
+       dns_db_settask(db, zone->task, zone->task);
 
        if (zone->type == dns_zone_primary ||
            zone->type == dns_zone_secondary || zone->type == dns_zone_mirror)
@@ -14725,7 +14725,7 @@ ns_query(dns_zone_t *zone, dns_rdataset_t *soardataset, dns_stub_t *stub) {
                                             dns_result_totext(result));
                                goto cleanup;
                        }
-                       dns_db_settask(stub->db, zone->task);
+                       dns_db_settask(stub->db, zone->task, zone->task);
                }
 
                result = dns_db_newversion(stub->db, &stub->version);
@@ -16183,7 +16183,7 @@ dns_zone_settask(dns_zone_t *zone, isc_task_t *task) {
        isc_task_attach(task, &zone->task);
        ZONEDB_LOCK(&zone->dblock, isc_rwlocktype_read);
        if (zone->db != NULL) {
-               dns_db_settask(zone->db, zone->task);
+               dns_db_settask(zone->db, zone->task, zone->task);
        }
        ZONEDB_UNLOCK(&zone->dblock, isc_rwlocktype_read);
        UNLOCK_ZONE(zone);
@@ -17467,7 +17467,7 @@ zone_replacedb(dns_zone_t *zone, dns_db_t *db, bool dump) {
                zone_detachdb(zone);
        }
        zone_attachdb(zone, db);
-       dns_db_settask(zone->db, zone->task);
+       dns_db_settask(zone->db, zone->task, zone->task);
        DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_LOADED | DNS_ZONEFLG_NEEDNOTIFY);
        return (ISC_R_SUCCESS);