Merge branch '3881-catz-offload' into 'main'

Resolve "Run the catalog zone update as an offloaded work"

Closes #3881

See merge request isc-projects/bind9!7560

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

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
 ~~~~~~~~~
 

lib/dns/catz.c

@@ -26,6 +26,7 @@
 #include <isc/parseint.h>
 #include <isc/result.h>
 #include <isc/util.h>
+#include <isc/work.h>
 
 #include <dns/catz.h>
 #include <dns/dbiterator.h>
@@ -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

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);
 /*%<