when committing a new qpzone version, delete dead nodes

if all data has been deleted from a node in the qpzone
database, delete the node too.

diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c
index 210c2abbbb9c924ee2717a64374e13c3ebce86ce..c5ea5a1e31c1b0b3b447737c56a54572d007aac0 100644 (file)
--- a/lib/dns/qpcache.c
+++ b/lib/dns/qpcache.c
@@ -202,10 +202,9 @@ struct qpcnode {
        uint8_t       : 0;
 
        /*%
-        * Used for dead nodes cleaning.  This linked list is used to mark nodes
-        * which have no data any longer, but we cannot unlink at that exact
-        * moment because we did not or could not obtain a write lock on the
-        * tree.
+        * Used for dead node cleaning. The deadnodes queue is used
+        * for nodes that have no data any longer, but we can't unlink
+        * yet because we don't have a tree lock.
         */
        isc_queue_node_t deadlink;
 };
@@ -216,9 +215,8 @@ struct qpcnode {
  * to reduce contention between threads.
  */
 typedef struct qpcache_bucket {
-       /*%
-        * Temporary storage for stale cache nodes and dynamically
-        * deleted nodes that await being cleaned up.
+       /*
+        * Temporary storage for cache nodes that need to be deleted.
         */
        isc_queue_t deadnodes;
 

diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c
index e772ee11be70aaaf58463b6e3a96a21011d70717..d1bb246d2ca4655b3de21ef5f5fd1fba5d405a2e 100644 (file)
--- a/lib/dns/qpzone.c
+++ b/lib/dns/qpzone.c
@@ -874,6 +874,7 @@ clean_zone_node(qpznode_t *node, uint32_t least_serial) {
                }
                top_prev = current;
        }
+
        if (!still_dirty) {
                node->dirty = false;
        }
@@ -1518,10 +1519,8 @@ closeversion(dns_db_t *db, dns_dbversion_t **versionp,
                NODE_UNLOCK(nlock, &nlocktype);
        }
 
-       if (EMPTY(cleanup_list)) {
-               *versionp = NULL;
-               return;
-       }
+       dns_qp_t *tree = NULL, *nsec = NULL, *nsec3 = NULL;
+       bool need_tree = false, need_nsec = false, need_nsec3 = false;
 
        for (changed = HEAD(cleanup_list); changed != NULL;
             changed = next_changed)
@@ -1537,14 +1536,100 @@ closeversion(dns_db_t *db, dns_dbversion_t **versionp,
                if (rollback) {
                        rollback_node(node, serial);
                }
+
+               qpznode_ref(node);
                qpznode_release(qpdb, node, least_serial,
                                &nlocktype DNS__DB_FILELINE);
 
+               /* If the node is now empty, we can delete it. */
+               if (commit && node->data == NULL) {
+                       switch ((int)node->nsec) {
+                       case DNS_DB_NSEC_HAS_NSEC:
+                               /*
+                                * Delete the matching node from the NSEC tree
+                                * first, then fall through to the main tree.
+                                */
+                               if (nsec == NULL) {
+                                       need_nsec = true;
+                                       next_changed = changed;
+                               } else {
+                                       dns_qp_deletename(nsec, &node->name,
+                                                         NULL, NULL);
+                               }
+                               FALLTHROUGH;
+                       case DNS_DB_NSEC_NORMAL:
+                               if (tree == NULL) {
+                                       need_tree = true;
+                                       next_changed = changed;
+                               } else {
+                                       dns_qp_deletename(tree, &node->name,
+                                                         NULL, NULL);
+                               }
+                               break;
+                       case DNS_DB_NSEC_NSEC:
+                               if (nsec == NULL) {
+                                       need_nsec = true;
+                                       next_changed = changed;
+                               } else {
+                                       dns_qp_deletename(nsec, &node->name,
+                                                         NULL, NULL);
+                               }
+                               break;
+                       case DNS_DB_NSEC_NSEC3:
+                               if (nsec3 == NULL) {
+                                       need_nsec3 = true;
+                                       next_changed = changed;
+                               } else {
+                                       dns_qp_deletename(nsec3, &node->name,
+                                                         NULL, NULL);
+                               }
+                               break;
+                       default:
+                               UNREACHABLE();
+                       }
+               }
+
+               qpznode_detach(&node);
+
                NODE_UNLOCK(nlock, &nlocktype);
 
+               if (next_changed == changed) {
+                       /*
+                        * We found a node to delete but didn't have a
+                        * QP writer open, so we open one now, then go
+                        * back to delete the node. If there's a next
+                        * time, we'll already have the writer open,
+                        * so we won't need this extra step.
+                        */
+                       if (need_tree && tree == NULL) {
+                               dns_qpmulti_write(qpdb->tree, &tree);
+                       }
+                       if (need_nsec && nsec == NULL) {
+                               dns_qpmulti_write(qpdb->nsec, &nsec);
+                       }
+                       if (need_nsec3 && nsec3 == NULL) {
+                               dns_qpmulti_write(qpdb->nsec3, &nsec3);
+                       }
+
+                       continue;
+               }
+
                isc_mem_put(qpdb->common.mctx, changed, sizeof(*changed));
        }
 
+       if (tree != NULL) {
+               dns_qp_compact(tree, DNS_QPGC_MAYBE);
+               dns_qpmulti_commit(qpdb->tree, &tree);
+       }
+       if (nsec != NULL) {
+               dns_qp_compact(nsec, DNS_QPGC_MAYBE);
+               dns_qpmulti_commit(qpdb->nsec, &nsec);
+       }
+       if (nsec3 != NULL) {
+               dns_qp_compact(nsec3, DNS_QPGC_MAYBE);
+               dns_qpmulti_commit(qpdb->nsec3, &nsec3);
+       }
+
        *versionp = NULL;
 }