diff --git a/CHANGES b/CHANGES index 54a2784082..1c33528f4c 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6114. [func] Run the catalog zone update process on the offload + threads. [GL #3881] + 6113. [func] Add shutdown signaling for catalog zones. [GL !7571] 6112. [func] Add reference count tracing for dns_catz_zone_t and diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index af1d4162d2..c6394d0d21 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -61,6 +61,12 @@ Feature Changes failure when receiving multiple UDP messages in a single system call. :gl:`#3840` +- Run catalog zone updates on the specialized "offload" threads to reduce the + amount of time they block query processing on the main networking + threads. This should increase the responsiveness of :iscman:`named` + when catalog zone updates are being applied after a catalog zone has been + successfully transferred. :gl:`#3881` + Bug Fixes ~~~~~~~~~ diff --git a/lib/dns/catz.c b/lib/dns/catz.c index ef1beb7b39..b90fcd277c 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -73,6 +74,7 @@ struct dns_catz_zone { dns_name_t name; dns_catz_zones_t *catzs; dns_rdata_t soa; + uint32_t version; /* key in entries is 'mhash', not domain name! */ isc_ht_t *entries; /* key in coos is domain name */ @@ -85,11 +87,12 @@ struct dns_catz_zone { dns_catz_options_t defoptions; dns_catz_options_t zoneoptions; isc_time_t lastupdated; - bool updatepending; - uint32_t version; - dns_db_t *db; - dns_dbversion_t *dbversion; + bool updatepending; /* there is an update pending */ + bool updaterunning; /* there is an update running */ + isc_result_t updateresult; /* result from the offloaded work */ + dns_db_t *db; /* zones database */ + dns_dbversion_t *dbversion; /* zones database version */ isc_timer_t *updatetimer; @@ -100,11 +103,18 @@ struct dns_catz_zone { isc_refcount_t references; }; +static void +dns__catz_timer_cb(void *); static void dns__catz_timer_start(dns_catz_zone_t *catz); static void dns__catz_timer_stop(void *arg); +static void +dns__catz_update_cb(void *data); +static void +dns__catz_done_cb(void *data); + static isc_result_t catz_process_zones_entry(dns_catz_zone_t *catz, dns_rdataset_t *value, dns_label_t *mhash); @@ -826,7 +836,7 @@ dns__catz_timer_start(dns_catz_zone_t *catz) { catz->loop = isc_loop_current(catz->catzs->loopmgr); - isc_timer_create(catz->loop, dns_catz_update_action, catz, + isc_timer_create(catz->loop, dns__catz_timer_cb, catz, &catz->updatetimer); isc_timer_start(catz->updatetimer, isc_timertype_once, &interval); } @@ -980,6 +990,8 @@ dns__catz_zone_destroy(dns_catz_zone_t *catz) { dns_db_detach(&catz->db); } + INSIST(!catz->updaterunning); + dns_name_free(&catz->name, mctx); dns_catz_options_free(&catz->defoptions, mctx); dns_catz_options_free(&catz->zoneoptions, mctx); @@ -1757,8 +1769,8 @@ catz_process_value(dns_catz_zone_t *catz, dns_name_t *name, } isc_result_t -dns_catz_update_process(dns_catz_zones_t *catzs, dns_catz_zone_t *catz, - const dns_name_t *src_name, dns_rdataset_t *rdataset) { +dns_catz_update_process(dns_catz_zone_t *catz, const dns_name_t *src_name, + dns_rdataset_t *rdataset) { isc_result_t result; int order; unsigned int nlabels; @@ -1767,7 +1779,6 @@ dns_catz_update_process(dns_catz_zones_t *catzs, dns_catz_zone_t *catz, dns_rdata_soa_t soa; dns_name_t prefix; - REQUIRE(DNS_CATZ_ZONES_VALID(catzs)); REQUIRE(DNS_CATZ_ZONE_VALID(catz)); REQUIRE(ISC_MAGIC_VALID(src_name, DNS_NAME_MAGIC)); @@ -2029,21 +2040,41 @@ cleanup: return (result); } -void -dns_catz_update_action(void *arg) { +static void +dns__catz_timer_cb(void *arg) { + char domain[DNS_NAME_FORMATSIZE]; isc_result_t result; - dns_catz_zone_t *catz = arg; + dns_catz_zone_t *catz = (dns_catz_zone_t *)arg; REQUIRE(DNS_CATZ_ZONE_VALID(catz)); + if (atomic_load(&catz->catzs->shuttingdown)) { + return; + } + LOCK(&catz->catzs->lock); + + INSIST(DNS_DB_VALID(catz->db)); + INSIST(catz->dbversion != NULL); + catz->updatepending = false; - dns_catz_update_from_db(catz->db, catz->catzs); - isc_timer_stop(catz->updatetimer); + catz->updaterunning = true; + catz->updateresult = ISC_R_UNSET; + + dns_name_format(&catz->name, domain, DNS_NAME_FORMATSIZE); + isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, + ISC_LOG_INFO, "catz: %s: reload start", domain); + + dns_catz_ref_catzs(catz->catzs); + isc_work_enqueue(catz->loop, dns__catz_update_cb, dns__catz_done_cb, + catz); + isc_timer_destroy(&catz->updatetimer); catz->loop = NULL; + result = isc_time_now(&catz->lastupdated); RUNTIME_CHECK(result == ISC_R_SUCCESS); + UNLOCK(&catz->catzs->lock); } @@ -2074,7 +2105,7 @@ dns_catz_dbupdate_callback(dns_db_t *db, void *fn_arg) { dns_db_detach(&catz->db); /* * We're not registering db update callback, it will be - * registered at the end of update_from_db + * registered at the end of dns__catz_update_cb() */ catz->db_registered = false; } @@ -2082,7 +2113,7 @@ dns_catz_dbupdate_callback(dns_db_t *db, void *fn_arg) { dns_db_attach(db, &catz->db); } - if (!catz->updatepending) { + if (!catz->updatepending && !catz->updaterunning) { catz->updatepending = true; dns_db_currentversion(db, &catz->dbversion); dns__catz_timer_start(catz); @@ -2113,8 +2144,16 @@ catz_rdatatype_is_processable(const dns_rdatatype_t type) { type != dns_rdatatype_cdnskey && type != dns_rdatatype_zonemd); } -void -dns_catz_update_from_db(dns_db_t *db, dns_catz_zones_t *catzs) { +/* + * Process an updated database for a catalog zone. + * It creates a new catz, iterates over database to fill it with content, and + * then merges new catz into old catz. + */ +static void +dns__catz_update_cb(void *data) { + dns_catz_zone_t *catz = (dns_catz_zone_t *)data; + dns_db_t *db = NULL; + dns_catz_zones_t *catzs = NULL; dns_catz_zone_t *oldcatz = NULL, *newcatz = NULL; isc_result_t result; isc_region_t r; @@ -2131,11 +2170,16 @@ dns_catz_update_from_db(dns_db_t *db, dns_catz_zones_t *catzs) { uint32_t vers; uint32_t catz_vers; - REQUIRE(DNS_DB_VALID(db)); - REQUIRE(DNS_CATZ_ZONES_VALID(catzs)); + REQUIRE(DNS_CATZ_ZONE_VALID(catz)); + REQUIRE(DNS_DB_VALID(catz->db)); + REQUIRE(DNS_CATZ_ZONES_VALID(catz->catzs)); + + db = catz->db; + catzs = catz->catzs; if (atomic_load(&catzs->shuttingdown)) { - return; + result = ISC_R_SHUTTINGDOWN; + goto exit; } dns_name_format(&db->origin, bname, DNS_NAME_FORMATSIZE); @@ -2144,13 +2188,15 @@ dns_catz_update_from_db(dns_db_t *db, dns_catz_zones_t *catzs) { * Create a new catz in the same context as current catz. */ dns_name_toregion(&db->origin, &r); + LOCK(&catzs->lock); result = isc_ht_find(catzs->zones, r.base, r.length, (void **)&oldcatz); + UNLOCK(&catzs->lock); if (result != ISC_R_SUCCESS) { /* This can happen if we remove the zone in the meantime. */ isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, "catz: zone '%s' not in config", bname); - return; + goto exit; } result = dns_db_getsoaserial(db, oldcatz->dbversion, &vers); @@ -2160,7 +2206,7 @@ dns_catz_update_from_db(dns_db_t *db, dns_catz_zones_t *catzs) { DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, "catz: zone '%s' has no SOA record (%s)", bname, isc_result_totext(result)); - return; + goto exit; } isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, @@ -2175,7 +2221,7 @@ dns_catz_update_from_db(dns_db_t *db, dns_catz_zones_t *catzs) { DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, "catz: failed to create new zone - %s", isc_result_totext(result)); - return; + goto exit; } result = dns_db_createiterator(db, DNS_DB_NONSEC3, &it); @@ -2186,7 +2232,7 @@ dns_catz_update_from_db(dns_db_t *db, dns_catz_zones_t *catzs) { DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, "catz: failed to create DB iterator - %s", isc_result_totext(result)); - return; + goto exit; } name = dns_fixedname_initname(&fixname); @@ -2205,7 +2251,7 @@ dns_catz_update_from_db(dns_db_t *db, dns_catz_zones_t *catzs) { DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, "catz: failed to create name from string - %s", isc_result_totext(result)); - return; + goto exit; } result = dns_dbiterator_seek(it, name); if (result != ISC_R_SUCCESS) { @@ -2225,6 +2271,11 @@ dns_catz_update_from_db(dns_db_t *db, dns_catz_zones_t *catzs) { * Iterate over database to fill the new zone. */ while (result == ISC_R_SUCCESS) { + if (atomic_load(&catzs->shuttingdown)) { + result = ISC_R_SHUTTINGDOWN; + break; + } + result = dns_dbiterator_current(it, &node, name); if (result != ISC_R_SUCCESS) { isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, @@ -2234,6 +2285,9 @@ dns_catz_update_from_db(dns_db_t *db, dns_catz_zones_t *catzs) { break; } + result = dns_dbiterator_pause(it); + RUNTIME_CHECK(result == ISC_R_SUCCESS); + if (!is_vers_processed) { /* Keep the version node to skip it later in the loop */ vers_node = node; @@ -2270,7 +2324,7 @@ dns_catz_update_from_db(dns_db_t *db, dns_catz_zones_t *catzs) { goto next; } - result = dns_catz_update_process(catzs, newcatz, name, + result = dns_catz_update_process(newcatz, name, &rdataset); if (result != ISC_R_SUCCESS) { char typebuf[DNS_RDATATYPE_FORMATSIZE]; @@ -2312,7 +2366,8 @@ dns_catz_update_from_db(dns_db_t *db, dns_catz_zones_t *catzs) { dns_db_closeversion(db, &oldcatz->dbversion, false); isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_DEBUG(3), - "catz: update_from_db: iteration finished"); + "catz: update_from_db: iteration finished: %s", + isc_result_totext(result)); /* * Check catalog zone version compatibilites. @@ -2345,7 +2400,8 @@ final: "will not be processed", bname); dns_catz_detach_catz(&newcatz); - return; + result = ISC_R_FAILURE; + goto exit; } /* @@ -2359,7 +2415,7 @@ final: "catz: failed merging zones: %s", isc_result_totext(result)); - return; + goto exit; } isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, @@ -2379,6 +2435,35 @@ final: oldcatz->db_registered = true; } } + +exit: + catz->updateresult = result; +} + +static void +dns__catz_done_cb(void *data) { + dns_catz_zone_t *catz = (dns_catz_zone_t *)data; + char dname[DNS_NAME_FORMATSIZE]; + + REQUIRE(DNS_CATZ_ZONE_VALID(catz)); + + LOCK(&catz->catzs->lock); + catz->updaterunning = false; + + dns_name_format(&catz->name, dname, DNS_NAME_FORMATSIZE); + + if (catz->updatepending && !atomic_load(&catz->catzs->shuttingdown)) { + /* Restart the timer */ + dns__catz_timer_start(catz); + } + + UNLOCK(&catz->catzs->lock); + + isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, + ISC_LOG_INFO, "catz: %s: reload done: %s", dname, + isc_result_totext(catz->updateresult)); + + dns_catz_unref_catzs(catz->catzs); } void diff --git a/lib/dns/include/dns/catz.h b/lib/dns/include/dns/catz.h index 67f96e2d21..9c9df5978d 100644 --- a/lib/dns/include/dns/catz.h +++ b/lib/dns/include/dns/catz.h @@ -267,15 +267,14 @@ dns_catz_zones_merge(dns_catz_zone_t *target, dns_catz_zone_t *newzone); */ isc_result_t -dns_catz_update_process(dns_catz_zones_t *catzs, dns_catz_zone_t *zone, - const dns_name_t *src_name, dns_rdataset_t *rdataset); +dns_catz_update_process(dns_catz_zone_t *catz, const dns_name_t *src_name, + dns_rdataset_t *rdataset); /*%< * Process a single rdataset from a catalog zone 'zone' update, src_name is the * record name. * * Requires: - * \li 'catzs' is a valid dns_catz_zones_t. - * \li 'zone' is a valid dns_catz_zone_t. + * \li 'catz' is a valid dns_catz_zone_t. * \li 'src_name' is a valid dns_name_t. * \li 'rdataset' is valid rdataset. */ @@ -385,28 +384,6 @@ dns_catz_dbupdate_callback(dns_db_t *db, void *fn_arg); * \li 'fn_arg' is not NULL (casted to dns_catz_zones_t*). */ -void -dns_catz_update_action(void *arg); -/*%< - * Task that launches dns_catz_update_from_db. - * - * Requires: - * \li 'event' is not NULL. - */ - -void -dns_catz_update_from_db(dns_db_t *db, dns_catz_zones_t *catzs); -/*%< - * Process an updated database for a catalog zone. - * It creates a new catz, iterates over database to fill it with content, and - * then merges new catz into old catz. - * - * Requires: - * \li 'db' is a valid DB. - * \li 'catzs' is a valid dns_catz_zones_t. - * - */ - void dns_catz_prereconfig(dns_catz_zones_t *catzs); /*%<