From: Evan Hunt Date: Tue, 6 Feb 2024 00:11:16 +0000 (-0800) Subject: add setup/commit functions to rdatacallbacks X-Git-Tag: v9.19.22~4^2~5 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=3512cf565456e5c5dc3f5de8132f69af327c86b3;p=thirdparty%2Fbind9.git add setup/commit functions to rdatacallbacks because dns_qpmulti_commit() can be time consuming, it's inefficient to open and commit a qpmulti transaction for each rdataset being loaded into a database. we can improve load time by opening a qpmulti transaction before adding a group of rdatasets and then committing it afterward. this commit adds 'setup' and 'commit' functions to dns_rdatacallbacks_t, which can be called before and after the loops in which 'add' is called in dns_master_load() and axfr_apply(). --- diff --git a/lib/dns/callbacks.c b/lib/dns/callbacks.c index 34ad8fdf924..e034155efc9 100644 --- a/lib/dns/callbacks.c +++ b/lib/dns/callbacks.c @@ -78,13 +78,9 @@ static void dns_rdatacallbacks_initcommon(dns_rdatacallbacks_t *callbacks) { REQUIRE(callbacks != NULL); - callbacks->magic = DNS_CALLBACK_MAGIC; - callbacks->add = NULL; - callbacks->rawdata = NULL; - callbacks->zone = NULL; - callbacks->add_private = NULL; - callbacks->error_private = NULL; - callbacks->warn_private = NULL; + *callbacks = (dns_rdatacallbacks_t){ + .magic = DNS_CALLBACK_MAGIC, + }; } /* diff --git a/lib/dns/diff.c b/lib/dns/diff.c index afbf87c5e8d..efefbaa2a0b 100644 --- a/lib/dns/diff.c +++ b/lib/dns/diff.c @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -506,13 +507,16 @@ dns_diff_applysilently(dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver) { /* XXX this duplicates lots of code in diff_apply(). */ isc_result_t -dns_diff_load(dns_diff_t *diff, dns_addrdatasetfunc_t addfunc, - void *add_private) { +dns_diff_load(dns_diff_t *diff, dns_rdatacallbacks_t *callbacks) { dns_difftuple_t *t; isc_result_t result; REQUIRE(DNS_DIFF_VALID(diff)); + if (callbacks->setup != NULL) { + callbacks->setup(callbacks->add_private); + } + t = ISC_LIST_HEAD(diff->tuples); while (t != NULL) { dns_name_t *name; @@ -551,8 +555,8 @@ dns_diff_load(dns_diff_t *diff, dns_addrdatasetfunc_t addfunc, rds.trust = dns_trust_ultimate; INSIST(op == DNS_DIFFOP_ADD); - result = (*addfunc)(add_private, name, - &rds DNS__DB_FILELINE); + result = callbacks->add(callbacks->add_private, name, + &rds DNS__DB_FILELINE); if (result == DNS_R_UNCHANGED) { isc_log_write(DIFF_COMMON_LOGARGS, ISC_LOG_WARNING, @@ -570,7 +574,11 @@ dns_diff_load(dns_diff_t *diff, dns_addrdatasetfunc_t addfunc, } } result = ISC_R_SUCCESS; + failure: + if (callbacks->commit != NULL) { + callbacks->commit(callbacks->add_private); + } return (result); } diff --git a/lib/dns/include/dns/callbacks.h b/lib/dns/include/dns/callbacks.h index e908a9f0488..da15ec9255e 100644 --- a/lib/dns/include/dns/callbacks.h +++ b/lib/dns/include/dns/callbacks.h @@ -37,9 +37,20 @@ struct dns_rdatacallbacks { unsigned int magic; /*% - * dns_load_master calls this when it has rdatasets to commit. + * dns_load_master calls 'add' when it has an rdataset to add + * to the database. If defined, it calls 'setup' before and + * 'commit' after adding rdatasets. + * + * Some database implementations will commit each rdataset as + * soon as it's added, in which case 'setup' and 'commit' need + * not be defined. However, other implementations can be + * optimized by grouping rdatasets into a transaction; the + * setup and commit functions allow this transaction to be + * opened and committed. */ dns_addrdatasetfunc_t add; + dns_transactionfunc_t setup; + dns_transactionfunc_t commit; /*% * dns_master_load*() call this when loading a raw zonefile, diff --git a/lib/dns/include/dns/diff.h b/lib/dns/include/dns/diff.h index 841e796004a..3bda5644d6e 100644 --- a/lib/dns/include/dns/diff.h +++ b/lib/dns/include/dns/diff.h @@ -243,19 +243,15 @@ dns_diff_applysilently(dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver); */ isc_result_t -dns_diff_load(dns_diff_t *diff, dns_addrdatasetfunc_t addfunc, - void *add_private); +dns_diff_load(dns_diff_t *diff, dns_rdatacallbacks_t *callbacks); /*%< * Like dns_diff_apply, but for use when loading a new database * instead of modifying an existing one. This bypasses the * database transaction mechanisms. * * Requires: - *\li 'addfunc' is a valid dns_addradatasetfunc_t obtained from - * dns_db_beginload() - * - *\li 'add_private' points to a corresponding dns_dbload_t * - * (XXX why is it a void pointer, then?) + *\li 'callbacks' points to a dns_rdatacallbacks_t structure obtained + * from dns_db_beginload() */ isc_result_t diff --git a/lib/dns/include/dns/types.h b/lib/dns/include/dns/types.h index b938d13752b..852ed865667 100644 --- a/lib/dns/include/dns/types.h +++ b/lib/dns/include/dns/types.h @@ -436,6 +436,7 @@ typedef void (*dns_rawdatafunc_t)(dns_zone_t *, dns_masterrawheader_t *); typedef isc_result_t (*dns_addrdatasetfunc_t)(void *arg, const dns_name_t *name, dns_rdataset_t *rdataset DNS__DB_FLARG); +typedef void (*dns_transactionfunc_t)(void *arg); typedef isc_result_t (*dns_additionaldatafunc_t)( void *arg, const dns_name_t *name, dns_rdatatype_t type, diff --git a/lib/dns/master.c b/lib/dns/master.c index 81cc999b00b..a9e7960d963 100644 --- a/lib/dns/master.c +++ b/lib/dns/master.c @@ -1007,7 +1007,7 @@ load_text(dns_loadctx_t *lctx) { dns_rdataclass_t rdclass; dns_rdatatype_t type, covers; uint32_t ttl_offset = 0; - dns_name_t *new_name; + dns_name_t *new_name = NULL; bool current_has_delegation = false; bool finish_origin = false; bool finish_include = false; @@ -1018,9 +1018,9 @@ load_text(dns_loadctx_t *lctx) { isc_result_t result = ISC_R_UNEXPECTED; rdatalist_head_t glue_list; rdatalist_head_t current_list; - dns_rdatalist_t *this; + dns_rdatalist_t *this = NULL; dns_rdatalist_t *rdatalist = NULL; - dns_rdatalist_t *new_rdatalist; + dns_rdatalist_t *new_rdatalist = NULL; int rdlcount = 0; int rdlcount_save = 0; int rdatalist_size = 0; @@ -1029,21 +1029,21 @@ load_text(dns_loadctx_t *lctx) { isc_buffer_t target_ft; isc_buffer_t target_save; dns_rdata_t *rdata = NULL; - dns_rdata_t *new_rdata; + dns_rdata_t *new_rdata = NULL; int rdcount = 0; int rdcount_save = 0; int rdata_size = 0; unsigned char *target_mem = NULL; int target_size = TSIZ; int new_in_use; - isc_mem_t *mctx; - dns_rdatacallbacks_t *callbacks; - dns_incctx_t *ictx; + isc_mem_t *mctx = NULL; + dns_rdatacallbacks_t *callbacks = NULL; + dns_incctx_t *ictx = NULL; char *range = NULL; char *lhs = NULL; char *gtype = NULL; char *rhs = NULL; - const char *source; + const char *source = NULL; unsigned long line = 0; bool explicit_ttl; char classname1[DNS_RDATACLASS_FORMATSIZE]; @@ -1066,6 +1066,11 @@ load_text(dns_loadctx_t *lctx) { isc_buffer_init(&target, target_mem, target_size); target_save = target; + /* open a database transaction */ + if (callbacks->setup != NULL) { + callbacks->setup(callbacks->add_private); + } + if ((lctx->options & DNS_MASTER_CHECKNAMES) != 0) { options |= DNS_RDATA_CHECKNAMES; } @@ -2116,6 +2121,11 @@ insist_and_cleanup: INSIST(result != ISC_R_SUCCESS); cleanup: + /* commit the database transaction */ + if (callbacks->commit != NULL) { + callbacks->commit(callbacks->add_private); + } + while ((this = ISC_LIST_HEAD(current_list)) != NULL) { ISC_LIST_UNLINK(current_list, this, link); } @@ -2357,6 +2367,11 @@ load_raw(dns_loadctx_t *lctx) { name = dns_fixedname_initname(&fixed); + /* open a database transaction */ + if (callbacks->setup != NULL) { + callbacks->setup(callbacks->add_private); + } + /* * In the following loop, we regard any error fatal regardless of * whether "MANYERRORS" is set in the context option. This is because @@ -2598,6 +2613,11 @@ load_raw(dns_loadctx_t *lctx) { } cleanup: + /* commit the database transaction */ + if (callbacks->commit != NULL) { + callbacks->commit(callbacks->add_private); + } + if (rdata != NULL) { isc_mem_cput(mctx, rdata, rdata_size, sizeof(*rdata)); } @@ -2888,17 +2908,14 @@ commit(dns_rdatacallbacks_t *callbacks, dns_loadctx_t *lctx, unsigned int line) { dns_rdatalist_t *this; dns_rdataset_t dataset; - isc_result_t result; + isc_result_t result = ISC_R_SUCCESS; char namebuf[DNS_NAME_FORMATSIZE]; void (*error)(struct dns_rdatacallbacks *, const char *, ...); this = ISC_LIST_HEAD(*head); error = callbacks->error; - if (this == NULL) { - return (ISC_R_SUCCESS); - } - do { + while (this != NULL) { dns_rdataset_init(&dataset); dns_rdatalist_tordataset(this, &dataset); dataset.trust = dns_trust_ultimate; @@ -2911,8 +2928,8 @@ commit(dns_rdatacallbacks_t *callbacks, dns_loadctx_t *lctx, dataset.attributes |= DNS_RDATASETATTR_RESIGN; dataset.resign = resign_fromlist(this, lctx); } - result = ((*callbacks->add)(callbacks->add_private, owner, - &dataset DNS__DB_FILELINE)); + result = callbacks->add(callbacks->add_private, owner, + &dataset DNS__DB_FILELINE); if (result == ISC_R_NOMEMORY) { (*error)(callbacks, "dns_master_load: %s", isc_result_totext(result)); @@ -2931,12 +2948,13 @@ commit(dns_rdatacallbacks_t *callbacks, dns_loadctx_t *lctx, if (MANYERRS(lctx, result)) { SETRESULT(lctx, result); } else if (result != ISC_R_SUCCESS) { - return (result); + break; } ISC_LIST_UNLINK(*head, this, link); this = ISC_LIST_HEAD(*head); - } while (this != NULL); - return (ISC_R_SUCCESS); + } + + return (result); } /* diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index 1ec6690a11a..bf0d1992088 100644 --- a/lib/dns/qpzone.c +++ b/lib/dns/qpzone.c @@ -1682,32 +1682,30 @@ delegating_type(qpzonedb_t *qpdb, qpdata_t *node, dns_typepair_t type) { } static void -loading_addnode(qpzonedb_t *qpdb, const dns_name_t *name, dns_rdatatype_t type, - dns_rdatatype_t covers, qpdata_t **nodep) { +loading_addnode(qpdb_load_t *loadctx, const dns_name_t *name, + dns_rdatatype_t type, dns_rdatatype_t covers, + qpdata_t **nodep) { + qpzonedb_t *qpdb = (qpzonedb_t *)loadctx->db; isc_result_t result; qpdata_t *node = NULL, *nsecnode = NULL; - dns_qp_t *qp = NULL, *nsec = NULL; if (type == dns_rdatatype_nsec3 || covers == dns_rdatatype_nsec3) { - dns_qpmulti_write(qpdb->nsec3, &qp); - result = dns_qp_getname(qp, name, (void **)&node, NULL); + result = dns_qp_getname(loadctx->nsec3, name, (void **)&node, + NULL); if (result == ISC_R_SUCCESS) { *nodep = node; } else { node = new_qpdata(qpdb, name); - result = dns_qp_insert(qp, node, 0); + result = dns_qp_insert(loadctx->nsec3, node, 0); INSIST(result == ISC_R_SUCCESS); node->nsec = DNS_DB_NSEC_NSEC3; *nodep = node; qpdata_detach(&node); } - dns_qp_compact(qp, DNS_QPGC_MAYBE); - dns_qpmulti_commit(qpdb->nsec3, &qp); return; } - dns_qpmulti_write(qpdb->tree, &qp); - result = dns_qp_getname(qp, name, (void **)&node, NULL); + result = dns_qp_getname(loadctx->tree, name, (void **)&node, NULL); if (result == ISC_R_SUCCESS) { if (type == dns_rdatatype_nsec && node->nsec == DNS_DB_NSEC_HAS_NSEC) @@ -1717,7 +1715,7 @@ loading_addnode(qpzonedb_t *qpdb, const dns_name_t *name, dns_rdatatype_t type, } else { INSIST(node == NULL); node = new_qpdata(qpdb, name); - result = dns_qp_insert(qp, node, 0); + result = dns_qp_insert(loadctx->tree, node, 0); INSIST(result == ISC_R_SUCCESS); qpdata_unref(node); } @@ -1730,9 +1728,8 @@ loading_addnode(qpzonedb_t *qpdb, const dns_name_t *name, dns_rdatatype_t type, * too. This tree speeds searches for closest NSECs that would * otherwise need to examine many irrelevant nodes in large TLDs. */ - dns_qpmulti_write(qpdb->nsec, &nsec); nsecnode = new_qpdata(qpdb, name); - result = dns_qp_insert(nsec, nsecnode, 0); + result = dns_qp_insert(loadctx->nsec, nsecnode, 0); node->nsec = DNS_DB_NSEC_HAS_NSEC; if (result == ISC_R_SUCCESS) { nsecnode->nsec = DNS_DB_NSEC_NSEC; @@ -1740,13 +1737,6 @@ loading_addnode(qpzonedb_t *qpdb, const dns_name_t *name, dns_rdatatype_t type, qpdata_detach(&nsecnode); done: - if (nsec != NULL) { - dns_qp_compact(nsec, DNS_QPGC_MAYBE); - dns_qpmulti_commit(qpdb->nsec, &nsec); - } - - dns_qp_compact(qp, DNS_QPGC_MAYBE); - dns_qpmulti_commit(qpdb->tree, &qp); *nodep = node; } @@ -2167,7 +2157,6 @@ loading_addrdataset(void *arg, const dns_name_t *name, isc_region_t region; dns_slabheader_t *newheader = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; - dns_qp_t *qp = NULL; REQUIRE(rdataset->rdclass == qpdb->common.rdclass); @@ -2180,11 +2169,10 @@ loading_addrdataset(void *arg, const dns_name_t *name, return (DNS_R_NOTZONETOP); } - dns_qpmulti_write(qpdb->tree, &qp); if (rdataset->type != dns_rdatatype_nsec3 && rdataset->covers != dns_rdatatype_nsec3) { - addwildcards(qpdb, qp, name, false); + addwildcards(qpdb, loadctx->tree, name, false); } if (dns_name_iswildcard(name)) { @@ -2192,23 +2180,20 @@ loading_addrdataset(void *arg, const dns_name_t *name, /* * NS owners cannot legally be wild cards. */ - result = DNS_R_INVALIDNS; - } else if (rdataset->type == dns_rdatatype_nsec3) { + return (DNS_R_INVALIDNS); + } + + if (rdataset->type == dns_rdatatype_nsec3) { /* * NSEC3 owners cannot legally be wild cards. */ - result = DNS_R_INVALIDNSEC3; - } else { - wildcardmagic(qpdb, qp, name, false); + return (DNS_R_INVALIDNSEC3); } - } - dns_qp_compact(qp, DNS_QPGC_MAYBE); - dns_qpmulti_commit(qpdb->tree, &qp); - if (result != ISC_R_SUCCESS) { - return (result); + + wildcardmagic(qpdb, loadctx->tree, name, false); } - loading_addnode(qpdb, name, rdataset->type, rdataset->covers, &node); + loading_addnode(loadctx, name, rdataset->type, rdataset->covers, &node); result = dns_rdataslab_fromrdataset(rdataset, qpdb->common.mctx, ®ion, sizeof(dns_slabheader_t)); if (result != ISC_R_SUCCESS) { @@ -2252,6 +2237,35 @@ loading_addrdataset(void *arg, const dns_name_t *name, return (result); } +static void +loading_setup(void *arg) { + qpdb_load_t *loadctx = arg; + qpzonedb_t *qpdb = (qpzonedb_t *)loadctx->db; + + dns_qpmulti_write(qpdb->tree, &loadctx->tree); + dns_qpmulti_write(qpdb->nsec, &loadctx->nsec); + dns_qpmulti_write(qpdb->nsec3, &loadctx->nsec3); +} + +static void +loading_commit(void *arg) { + qpdb_load_t *loadctx = arg; + qpzonedb_t *qpdb = (qpzonedb_t *)loadctx->db; + + if (loadctx->tree != NULL) { + dns_qp_compact(loadctx->tree, DNS_QPGC_MAYBE); + dns_qpmulti_commit(qpdb->tree, &loadctx->tree); + } + if (loadctx->nsec != NULL) { + dns_qp_compact(loadctx->nsec, DNS_QPGC_MAYBE); + dns_qpmulti_commit(qpdb->nsec, &loadctx->nsec); + } + if (loadctx->nsec3 != NULL) { + dns_qp_compact(loadctx->nsec3, DNS_QPGC_MAYBE); + dns_qpmulti_commit(qpdb->nsec3, &loadctx->nsec3); + } +} + static isc_result_t beginload(dns_db_t *db, dns_rdatacallbacks_t *callbacks) { qpdb_load_t *loadctx = NULL; @@ -2262,9 +2276,7 @@ beginload(dns_db_t *db, dns_rdatacallbacks_t *callbacks) { REQUIRE(VALID_QPZONE(qpdb)); loadctx = isc_mem_get(qpdb->common.mctx, sizeof(*loadctx)); - - loadctx->db = db; - loadctx->now = 0; + *loadctx = (qpdb_load_t){ .db = db }; RWLOCK(&qpdb->lock, isc_rwlocktype_write); @@ -2275,6 +2287,8 @@ beginload(dns_db_t *db, dns_rdatacallbacks_t *callbacks) { RWUNLOCK(&qpdb->lock, isc_rwlocktype_write); callbacks->add = loading_addrdataset; + callbacks->setup = loading_setup; + callbacks->commit = loading_commit; callbacks->add_private = loadctx; return (ISC_R_SUCCESS); @@ -2308,6 +2322,8 @@ endload(dns_db_t *db, dns_rdatacallbacks_t *callbacks) { } callbacks->add = NULL; + callbacks->setup = NULL; + callbacks->commit = NULL; callbacks->add_private = NULL; isc_mem_put(qpdb->common.mctx, loadctx, sizeof(*loadctx)); diff --git a/lib/dns/xfrin.c b/lib/dns/xfrin.c index 2c6e7b0f2ce..0edf52497f3 100644 --- a/lib/dns/xfrin.c +++ b/lib/dns/xfrin.c @@ -343,7 +343,7 @@ axfr_apply(void *arg) { goto failure; } - CHECK(dns_diff_load(&xfr->diff, xfr->axfr.add, xfr->axfr.add_private)); + CHECK(dns_diff_load(&xfr->diff, &xfr->axfr)); if (xfr->maxrecords != 0U) { result = dns_db_getsize(xfr->db, xfr->ver, &records, NULL); if (result == ISC_R_SUCCESS && records > xfr->maxrecords) {