]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
improve calculation of database size
authorEvan Hunt <each@isc.org>
Mon, 9 Mar 2020 18:32:47 +0000 (11:32 -0700)
committerEvan Hunt <each@isc.org>
Thu, 12 Mar 2020 07:38:37 +0000 (00:38 -0700)
"max-journal-size" is set by default to twice the size of the zone
database. however, the calculation of zone database size was flawed.

- change the size calculations in dns_db_getsize() to more accurately
  represent the space needed for a journal file or *XFR message to
  contain the data in the database. previously we returned the sizes
  of all rdataslabs, including header overhead and offset tables,
  which resulted in the database size being reported as much larger
  than the equivalent journal transactions would have been.
- map files caused a particular problem here: the full name can't be
  determined from the node while a file is being deserialized, because
  the uppernode pointers aren't set yet. so we store "full name length"
  in the dns_rbtnode structure while serializing, and clear it after
  deserialization is complete.

CHANGES
lib/dns/include/dns/db.h
lib/dns/include/dns/rbt.h
lib/dns/include/dns/rdataslab.h
lib/dns/mapapi
lib/dns/rbt.c
lib/dns/rbtdb.c
lib/dns/rdataslab.c
lib/dns/tests/rbt_test.c
lib/dns/win32/libdns.def.in

diff --git a/CHANGES b/CHANGES
index 98810a60f65016193bce12890033f60598c2c299..e6f68af9cb65d34c47bcde9675e6ae73ea31677f 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,7 @@
+5367.  [bug]           Fixed a flaw in the calculation of zone database
+                       size so that "max-journal-size default" will use
+                       the correct limit. [GL #1661]
+
 5366.  [bug]           Fix a race condition with the keymgr when the same
                        zone plus dnssec-policy is configured in multiple
                        views. [GL #1653]
index a092600274ce286451233333567e707f9a79c75d..fa1bb52240ff2bea9ec8003b80e238cbd5e96ee4 100644 (file)
@@ -1470,8 +1470,12 @@ isc_result_t
 dns_db_getsize(dns_db_t *db, dns_dbversion_t *version, uint64_t *records,
               uint64_t *bytes);
 /*%<
- * Get the number of records in the given version of the database as well
- * as the number bytes used to store those records.
+ * On success if 'records' is not NULL, it is set to the number of records
+ * in the given version of the database. If 'bytes' is not NULL, it is
+ * set to the approximate number of bytes needed to transfer the records,
+ * counting name, TTL, type, class, and rdata for each RR.  (This is meant
+ * to be a rough approximation of the size of a full zone transfer, though
+ * it does not take into account DNS message overhead or name compression.)
  *
  * Requires:
  * \li 'db' is a valid zone database.
index aaf5f58ca54e7fd3164ee10f5254d7ce5a73ca35..8c2139eb1434194dc4572021ec3d6c8c44662502 100644 (file)
@@ -105,6 +105,13 @@ struct dns_rbtnode {
        unsigned int down_is_relative : 1;
        unsigned int data_is_relative : 1;
 
+       /*
+        * full name length; set during serialization, and used
+        * during deserialization to calculate database size.
+        * should be cleared after use.
+        */
+       unsigned int fullnamelen : 8; /*%< range is 1..255 */
+
        /* node needs to be cleaned from rpz */
        unsigned int rpz : 1;
        unsigned int : 0; /* end of bitfields c/o tree lock */
@@ -152,9 +159,8 @@ struct dns_rbtnode {
        uint8_t : 0; /* start of bitfields c/o node lock */
        uint8_t dirty : 1;
        uint8_t wild : 1;
-       uint8_t : 0;      /* end of bitfields c/o node lock */
-       uint16_t locknum; /* note that this is not in the bitfield
-                          * */
+       uint8_t : 0;            /* end of bitfields c/o node lock */
+       uint16_t       locknum; /* note that this is not in the bitfield */
        isc_refcount_t references;
        /*@}*/
 };
@@ -1029,12 +1035,12 @@ dns_rbtnodechain_nextflat(dns_rbtnodechain_t *chain, dns_name_t *name);
  * Find the next node at the current depth in DNSSEC order.
  */
 
-void
-dns_rbtnode_nodename(dns_rbtnode_t *node, dns_name_t *name);
-
-dns_rbtnode_t *
-dns_rbt_root(dns_rbt_t *rbt);
-
+unsigned int
+dns__rbtnode_namelen(dns_rbtnode_t *node);
+/*%<
+ * Returns the length of the full name of the node. Used only internally
+ * and in unit tests.
+ */
 ISC_LANG_ENDDECLS
 
 #endif /* DNS_RBT_H */
index dfcf96081719e680ad4cd5655dc2cf38eb8d1b73..81b3494e8dea024143646ce139dbc107bb882506 100644 (file)
@@ -96,6 +96,15 @@ dns_rdataslab_size(unsigned char *slab, unsigned int reservelen);
  *\li  The number of bytes in the slab, including the reservelen.
  */
 
+unsigned int
+dns_rdataslab_rdatasize(unsigned char *slab, unsigned int reservelen);
+/*%<
+ * Return the size of the rdata in an rdataslab.
+ *
+ * Requires:
+ *\li  'slab' points to a slab.
+ */
+
 unsigned int
 dns_rdataslab_count(unsigned char *slab, unsigned int reservelen);
 /*%<
index 3a710613e9e03affb52f6c7e483ee94a7d982327..d7a692f964590be241e1a9fc57f63c7a06dd1236 100644 (file)
@@ -13,4 +13,4 @@
 # Whenever releasing a new major release of BIND9, set this value
 # back to 1.0 when releasing the first alpha.  Map files are *never*
 # compatible across major releases.
-MAPAPI=1.0
+MAPAPI=2.0
index ef6441b37c434bca35d870846df74723f3f079bc..da38f76155d878331cab7f615381a1ff23e0ac02 100644 (file)
@@ -281,21 +281,6 @@ NODENAME(dns_rbtnode_t *node, dns_name_t *name) {
        name->attributes |= DNS_NAMEATTR_READONLY;
 }
 
-void
-dns_rbtnode_nodename(dns_rbtnode_t *node, dns_name_t *name) {
-       name->length = NAMELEN(node);
-       name->labels = OFFSETLEN(node);
-       name->ndata = NAME(node);
-       name->offsets = OFFSETS(node);
-       name->attributes = ATTRS(node);
-       name->attributes |= DNS_NAMEATTR_READONLY;
-}
-
-dns_rbtnode_t *
-dns_rbt_root(dns_rbt_t *rbt) {
-       return (rbt->root);
-}
-
 #ifdef DEBUG
 #define inline
 /*
@@ -527,18 +512,39 @@ match_header_version(file_header_t *header) {
        return (true);
 }
 
+unsigned int
+dns__rbtnode_namelen(dns_rbtnode_t *node) {
+       dns_name_t current;
+       unsigned int len = 0;
+
+       REQUIRE(DNS_RBTNODE_VALID(node));
+
+       dns_name_init(&current, NULL);
+
+       do {
+               if (node != NULL) {
+                       NODENAME(node, &current);
+                       len += current.length;
+               } else {
+                       len += 1;
+                       break;
+               }
+
+               node = get_upper_node(node);
+       } while (!dns_name_isabsolute(&current));
+
+       return (len);
+}
+
 static isc_result_t
 serialize_node(FILE *file, dns_rbtnode_t *node, uintptr_t left, uintptr_t right,
               uintptr_t down, uintptr_t parent, uintptr_t data,
               uint64_t *crc) {
+       isc_result_t result;
        dns_rbtnode_t temp_node;
        off_t file_position;
-       unsigned char *node_data;
+       unsigned char *node_data = NULL;
        size_t datasize;
-       isc_result_t result;
-#ifdef DEBUG
-       dns_name_t nodename;
-#endif /* ifdef DEBUG */
 
        INSIST(node != NULL);
 
@@ -581,6 +587,8 @@ serialize_node(FILE *file, dns_rbtnode_t *node, uintptr_t left, uintptr_t right,
                temp_node.data_is_relative = 1;
        }
 
+       temp_node.fullnamelen = dns__rbtnode_namelen(node);
+
        node_data = (unsigned char *)node + sizeof(dns_rbtnode_t);
        datasize = NODE_SIZE(node) - sizeof(dns_rbtnode_t);
 
@@ -589,10 +597,8 @@ serialize_node(FILE *file, dns_rbtnode_t *node, uintptr_t left, uintptr_t right,
        CHECK(isc_stdio_write(node_data, 1, datasize, file, NULL));
 
 #ifdef DEBUG
-       dns_name_init(&nodename, NULL);
-       NODENAME(node, &nodename);
        fprintf(stderr, "serialize ");
-       dns_name_print(&nodename, stderr);
+       dns_name_print(name, stderr);
        fprintf(stderr, "\n");
        hexdump("node header", (unsigned char *)&temp_node,
                sizeof(dns_rbtnode_t));
index 7a935918ac3caa6aed61a7c022a0f76877bc9fa0..f379f81f7de2e7d6c2cc801db6c7df13ba6c9fc4 100644 (file)
@@ -5927,19 +5927,27 @@ resign_delete(dns_rbtdb_t *rbtdb, rbtdb_version_t *version,
        }
 }
 
+static inline uint64_t
+recordsize(rdatasetheader_t *header, unsigned int namelen) {
+       return (dns_rdataslab_rdatasize((unsigned char *)header,
+                                       sizeof(*header)) +
+               sizeof(dns_ttl_t) + sizeof(dns_rdatatype_t) +
+               sizeof(dns_rdataclass_t) + namelen);
+}
+
 static void
 update_recordsandbytes(bool add, rbtdb_version_t *rbtversion,
-                      rdatasetheader_t *header) {
+                      rdatasetheader_t *header, unsigned int namelen) {
        unsigned char *hdr = (unsigned char *)header;
        size_t hdrsize = sizeof(*header);
 
        RWLOCK(&rbtversion->rwlock, isc_rwlocktype_write);
        if (add) {
                rbtversion->records += dns_rdataslab_count(hdr, hdrsize);
-               rbtversion->bytes += dns_rdataslab_size(hdr, hdrsize);
+               rbtversion->bytes += recordsize(header, namelen);
        } else {
                rbtversion->records -= dns_rdataslab_count(hdr, hdrsize);
-               rbtversion->bytes -= dns_rdataslab_size(hdr, hdrsize);
+               rbtversion->bytes -= recordsize(header, namelen);
        }
        RWUNLOCK(&rbtversion->rwlock, isc_rwlocktype_write);
 }
@@ -5950,6 +5958,8 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion,
       dns_rdataset_t *addedrdataset, isc_stdtime_t now) {
        rbtdb_changed_t *changed = NULL;
        rdatasetheader_t *topheader, *topheader_prev, *header, *sigheader;
+       dns_fixedname_t fname;
+       dns_name_t *nodename = dns_fixedname_initname(&fname);
        unsigned char *merged;
        isc_result_t result;
        bool header_nx;
@@ -5968,6 +5978,8 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion,
         * Caller must be holding the node lock.
         */
 
+       dns_rbt_fullnamefromnode(rbtnode, nodename);
+
        if ((options & DNS_DBADD_MERGE) != 0) {
                REQUIRE(rbtversion != NULL);
                merge = true;
@@ -6343,7 +6355,8 @@ find_header:
                        newheader->next = topheader->next;
                        if (rbtversion != NULL && !header_nx) {
                                update_recordsandbytes(false, rbtversion,
-                                                      header);
+                                                      header,
+                                                      nodename->length);
                        }
                        free_rdataset(rbtdb, rbtdb->common.mctx, header);
                } else {
@@ -6395,7 +6408,8 @@ find_header:
                        }
                        if (rbtversion != NULL && !header_nx) {
                                update_recordsandbytes(false, rbtversion,
-                                                      header);
+                                                      header,
+                                                      nodename->length);
                        }
                }
        } else {
@@ -6472,7 +6486,8 @@ find_header:
        }
 
        if (rbtversion != NULL && !newheader_nx) {
-               update_recordsandbytes(true, rbtversion, newheader);
+               update_recordsandbytes(true, rbtversion, newheader,
+                                      nodename->length);
        }
 
        /*
@@ -6836,6 +6851,8 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
        dns_rbtdb_t *rbtdb = (dns_rbtdb_t *)db;
        dns_rbtnode_t *rbtnode = (dns_rbtnode_t *)node;
        rbtdb_version_t *rbtversion = version;
+       dns_fixedname_t fname;
+       dns_name_t *nodename = dns_fixedname_initname(&fname);
        rdatasetheader_t *topheader, *topheader_prev, *header, *newheader;
        unsigned char *subresult;
        isc_region_t region;
@@ -6845,6 +6862,8 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
        REQUIRE(VALID_RBTDB(rbtdb));
        REQUIRE(rbtversion != NULL && rbtversion->rbtdb == rbtdb);
 
+       dns_rbt_fullnamefromnode(rbtnode, nodename);
+
        if (rbtdb->common.methods == &zone_methods) {
                RWLOCK(&rbtdb->tree_lock, isc_rwlocktype_read);
                REQUIRE(((rbtnode->nsec == DNS_RBT_NSEC_NSEC3 &&
@@ -6960,7 +6979,8 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
                         * to additional info.  We need to clear these fields
                         * to avoid having duplicated references.
                         */
-                       update_recordsandbytes(true, rbtversion, newheader);
+                       update_recordsandbytes(true, rbtversion, newheader,
+                                              nodename->length);
                } else if (result == DNS_R_NXRRSET) {
                        /*
                         * This subtraction would remove all of the rdata;
@@ -6995,7 +7015,8 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
                 * topheader.
                 */
                INSIST(rbtversion->serial >= topheader->serial);
-               update_recordsandbytes(false, rbtversion, header);
+               update_recordsandbytes(false, rbtversion, header,
+                                      nodename->length);
                if (topheader_prev != NULL) {
                        topheader_prev->next = newheader;
                } else {
@@ -7362,9 +7383,14 @@ rbt_datafixer(dns_rbtnode_t *rbtnode, void *base, size_t filesize, void *arg,
                                return (ISC_R_INVALIDFILE);
                        }
                }
-               update_recordsandbytes(true, rbtdb->current_version, header);
+
+               update_recordsandbytes(true, rbtdb->current_version, header,
+                                      rbtnode->fullnamelen);
        }
 
+       /* We're done deserializing; clear fullnamelen */
+       rbtnode->fullnamelen = 0;
+
        return (ISC_R_SUCCESS);
 }
 
index b3487de0ccccaab36afccc3ec9ad16c761e4100c..bd7deb2fe1bafdbd4a3a393fba99c568c0bde6b1 100644 (file)
@@ -361,6 +361,34 @@ dns_rdataslab_size(unsigned char *slab, unsigned int reservelen) {
        return ((unsigned int)(current - slab));
 }
 
+unsigned int
+dns_rdataslab_rdatasize(unsigned char *slab, unsigned int reservelen) {
+       unsigned int count, length, rdatalen = 0;
+       unsigned char *current;
+
+       REQUIRE(slab != NULL);
+
+       current = slab + reservelen;
+       count = *current++ * 256;
+       count += *current++;
+#if DNS_RDATASET_FIXED
+       current += (4 * count);
+#endif /* if DNS_RDATASET_FIXED */
+       while (count > 0) {
+               count--;
+               length = *current++ * 256;
+               length += *current++;
+               rdatalen += length;
+#if DNS_RDATASET_FIXED
+               current += length + 2;
+#else  /* if DNS_RDATASET_FIXED */
+               current += length;
+#endif /* if DNS_RDATASET_FIXED */
+       }
+
+       return (rdatalen);
+}
+
 unsigned int
 dns_rdataslab_count(unsigned char *slab, unsigned int reservelen) {
        unsigned int count;
index 2157db624f2910de3734a6fe5e7afd25c4277457..a88790e655d5e198e0220c848e3526a5938f41b4 100644 (file)
@@ -1184,6 +1184,47 @@ rbt_nodechain(void **state) {
        test_context_teardown(ctx);
 }
 
+/* Test addname return values */
+static void
+rbtnode_namelen(void **state) {
+       isc_result_t result;
+       test_context_t *ctx = NULL;
+       dns_rbtnode_t *node;
+       unsigned int len;
+
+       UNUSED(state);
+
+       isc_mem_debugging = ISC_MEM_DEBUGRECORD;
+
+       ctx = test_context_setup();
+
+       node = NULL;
+       result = insert_helper(ctx->rbt, ".", &node);
+       len = dns__rbtnode_namelen(node);
+       assert_int_equal(result, ISC_R_EXISTS);
+       assert_int_equal(len, 1);
+       node = NULL;
+
+       result = insert_helper(ctx->rbt, "a.b.c.d.e.f.g.h.i.j.k.l.m", &node);
+       len = dns__rbtnode_namelen(node);
+       assert_int_equal(result, ISC_R_SUCCESS);
+       assert_int_equal(len, 27);
+
+       node = NULL;
+       result = insert_helper(ctx->rbt, "isc.org", &node);
+       len = dns__rbtnode_namelen(node);
+       assert_int_equal(result, ISC_R_SUCCESS);
+       assert_int_equal(len, 9);
+
+       node = NULL;
+       result = insert_helper(ctx->rbt, "example.com", &node);
+       len = dns__rbtnode_namelen(node);
+       assert_int_equal(result, ISC_R_SUCCESS);
+       assert_int_equal(len, 13);
+
+       test_context_teardown(ctx);
+}
+
 #if defined(DNS_BENCHMARK_TESTS) && !defined(__SANITIZE_THREAD__)
 
 /*
@@ -1324,6 +1365,8 @@ main(void) {
                                                _teardown),
                cmocka_unit_test_setup_teardown(rbt_nodechain, _setup,
                                                _teardown),
+               cmocka_unit_test_setup_teardown(rbtnode_namelen, _setup,
+                                               _teardown),
 #if defined(DNS_BENCHMARK_TESTS) && !defined(__SANITIZE_THREAD__)
                cmocka_unit_test_setup_teardown(benchmark, _setup, _teardown),
 #endif /* defined(DNS_BENCHMARK_TESTS) && !defined(__SANITIZE_THREAD__) */
index 83d750cb0341d0c0c0a079bd6956877efa6c7125..fec0a6dd832d4877b4379db8b88c1c03f51c9fbd 100644 (file)
@@ -7,6 +7,7 @@ EXPORTS
 dns__rbt_checkproperties
 dns__rbt_getheight
 dns__rbtnode_getdistance
+dns__rbtnode_namelen
 dns__zone_findkeys
 dns__zone_loadpending
 dns__zone_updatesigs
@@ -766,10 +767,8 @@ dns_rbt_nodecount
 dns_rbt_printdot
 dns_rbt_printnodeinfo
 dns_rbt_printtext
-dns_rbt_root
 dns_rbt_serialize_align
 dns_rbt_serialize_tree
-dns_rbtnode_nodename
 dns_rbtnodechain_current
 dns_rbtnodechain_down
 dns_rbtnodechain_first
@@ -871,6 +870,7 @@ dns_rdataslab_equal
 dns_rdataslab_equalx
 dns_rdataslab_fromrdataset
 dns_rdataslab_merge
+dns_rdataslab_rdatasize
 dns_rdataslab_size
 dns_rdataslab_subtract
 dns_rdatatype_atcname