Merge branch '3628-cleanup-task-from-dns_masterdump' into 'main'

Refactor zone loading and dumping to use offloaded work

Closes #3628

See merge request isc-projects/bind9!6990

CHANGES

@@ -1,3 +1,8 @@
+6006.	[cleanup]	The zone dumping was using isc_task API to launch
+			the zonedump on the offloaded threadpool.  Remove
+			the task and launch the offloaded work directly.
+			[GL #3628]
+
 6005.	[func]		The zone loading has been moved to the offload
 			threadpool instead of doing incremental repeated
 			tasks, so zone loading scheduling is now driven

bin/named/server.c

@@ -265,7 +265,7 @@ struct dumpcontext {
 	dns_dumpctx_t *mdctx;
 	dns_db_t *db;
 	dns_db_t *cache;
-	isc_task_t *task;
+	isc_loop_t *loop;
 	dns_dbversion_t *version;
 };
 
@@ -11555,9 +11555,6 @@ dumpcontext_destroy(struct dumpcontext *dctx) {
 	if (dctx->cache != NULL) {
 		dns_db_detach(&dctx->cache);
 	}
-	if (dctx->task != NULL) {
-		isc_task_detach(&dctx->task);
-	}
 	if (dctx->fp != NULL) {
 		(void)isc_stdio_close(dctx->fp);
 	}
@@ -11613,8 +11610,8 @@ resume:
 				dns_cache_getname(dctx->view->view->cache));
 			result = dns_master_dumptostreamasync(
 				dctx->mctx, dctx->cache, NULL, style, dctx->fp,
-				dctx->task, dumpdone, dctx, &dctx->mdctx);
-			if (result == DNS_R_CONTINUE) {
+				named_g_mainloop, dumpdone, dctx, &dctx->mdctx);
+			if (result == ISC_R_SUCCESS) {
 				return;
 			}
 			if (result == ISC_R_NOTIMPLEMENTED) {
@@ -11673,9 +11670,9 @@ resume:
 			dns_db_currentversion(dctx->db, &dctx->version);
 			result = dns_master_dumptostreamasync(
 				dctx->mctx, dctx->db, dctx->version, style,
-				dctx->fp, dctx->task, dumpdone, dctx,
-				&dctx->mdctx);
-			if (result == DNS_R_CONTINUE) {
+				dctx->fp, dns_zone_getloop(dctx->zone->zone),
+				dumpdone, dctx, &dctx->mdctx);
+			if (result == ISC_R_SUCCESS) {
 				return;
 			}
 			if (result == ISC_R_NOTIMPLEMENTED) {
@@ -11732,25 +11729,14 @@ named_server_dumpdb(named_server_t *server, isc_lex_t *lex,
 	}
 
 	dctx = isc_mem_get(server->mctx, sizeof(*dctx));
-
-	dctx->mctx = server->mctx;
-	dctx->dumpcache = true;
-	dctx->dumpadb = true;
-	dctx->dumpbad = true;
-	dctx->dumpexpired = false;
-	dctx->dumpfail = true;
-	dctx->dumpzones = false;
-	dctx->fp = NULL;
-	ISC_LIST_INIT(dctx->viewlist);
-	dctx->view = NULL;
-	dctx->zone = NULL;
-	dctx->cache = NULL;
-	dctx->mdctx = NULL;
-	dctx->db = NULL;
-	dctx->cache = NULL;
-	dctx->task = NULL;
-	dctx->version = NULL;
-	isc_task_attach(server->task, &dctx->task);
+	*dctx = (struct dumpcontext){
+		.mctx = server->mctx,
+		.dumpcache = true,
+		.dumpadb = true,
+		.dumpbad = true,
+		.dumpfail = true,
+		.viewlist = ISC_LIST_INITIALIZER,
+	};
 
 	CHECKMF(isc_stdio_open(server->dumpfile, "w", &dctx->fp),
 		"could not open dump file", server->dumpfile);

lib/dns/include/dns/masterdump.h

@@ -245,7 +245,7 @@ isc_result_t
 dns_master_dumptostreamasync(isc_mem_t *mctx, dns_db_t *db,
 			     dns_dbversion_t	      *version,
 			     const dns_master_style_t *style, FILE *f,
-			     isc_task_t *task, dns_dumpdonefunc_t done,
+			     isc_loop_t *loop, dns_dumpdonefunc_t done,
 			     void *done_arg, dns_dumpctx_t **dctxp);
 
 isc_result_t
@@ -264,7 +264,6 @@ dns_master_dumptostream(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version,
  * Temporary dynamic memory may be allocated from 'mctx'.
  *
  * Require:
- *\li	'task' to be valid.
  *\li	'done' to be non NULL.
  *\li	'dctxp' to be non NULL && '*dctxp' to be NULL.
  *
@@ -281,7 +280,7 @@ dns_master_dumptostream(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version,
 isc_result_t
 dns_master_dumpasync(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version,
 		     const dns_master_style_t *style, const char *filename,
-		     isc_task_t *task, dns_dumpdonefunc_t done, void *done_arg,
+		     isc_loop_t *loop, dns_dumpdonefunc_t done, void *done_arg,
 		     dns_dumpctx_t **dctxp, dns_masterformat_t format,
 		     dns_masterrawheader_t *header);
 

lib/dns/include/dns/zone.h

@@ -2753,6 +2753,16 @@ dns_zone_gettid(dns_zone_t *zone);
  * \return thread id associated with the zone
  */
 
+isc_loop_t *
+dns_zone_getloop(dns_zone_t *zone);
+/**<
+ * \brief Return loop associated with the zone.
+ *
+ * \param valid dns_zone_t object
+ *
+ * \return loop associated with the zone
+ */
+
 bool
 dns_zone_check_dnskey_nsec3(dns_zone_t *zone, dns_db_t *db,
 			    dns_dbversion_t *ver, dns_diff_t *diff,

lib/dns/masterdump.c

@@ -17,10 +17,12 @@
 #include <stdbool.h>
 #include <stdlib.h>
 
+#include <isc/async.h>
 #include <isc/atomic.h>
 #include <isc/buffer.h>
 #include <isc/event.h>
 #include <isc/file.h>
+#include <isc/loop.h>
 #include <isc/magic.h>
 #include <isc/mem.h>
 #include <isc/print.h>
@@ -28,7 +30,6 @@
 #include <isc/result.h>
 #include <isc/stdio.h>
 #include <isc/string.h>
-#include <isc/task.h>
 #include <isc/time.h>
 #include <isc/types.h>
 #include <isc/util.h>
@@ -261,7 +262,6 @@ struct dns_dumpctx {
 	dns_dbversion_t *version;
 	dns_dbiterator_t *dbiter;
 	dns_totext_ctx_t tctx;
-	isc_task_t *task;
 	dns_dumpdonefunc_t done;
 	void *done_arg;
 	/* dns_master_dumpasync() */
@@ -1071,7 +1071,6 @@ again:
 		sorted[i] = &rdatasets[i];
 	}
 	n = i;
-	INSIST(n <= MAXSORT);
 
 	qsort(sorted, n, sizeof(sorted[0]), dump_order_compare);
 
@@ -1334,9 +1333,6 @@ dumpctx_destroy(dns_dumpctx_t *dctx) {
 		dns_db_closeversion(dctx->db, &dctx->version, false);
 	}
 	dns_db_detach(&dctx->db);
-	if (dctx->task != NULL) {
-		isc_task_detach(&dctx->task);
-	}
 	if (dctx->file != NULL) {
 		isc_mem_free(dctx->mctx, dctx->file);
 	}
@@ -1496,7 +1492,7 @@ master_dump_cb(void *data) {
 }
 
 /*
- * This will run in a network/task manager thread when the dump is complete.
+ * This will run in a loop manager thread when the dump is complete.
  */
 static void
 master_dump_done_cb(void *data) {
@@ -1506,36 +1502,6 @@ master_dump_done_cb(void *data) {
 	dns_dumpctx_detach(&dctx);
 }
 
-/*
- * This must be run from a network/task manager thread.
- */
-static void
-setup_dump(isc_task_t *task, isc_event_t *event) {
-	dns_dumpctx_t *dctx = NULL;
-	isc_loopmgr_t *loopmgr = isc_task_getloopmgr(task);
-	isc_loop_t *loop = isc_loop_current(loopmgr);
-
-	REQUIRE(event != NULL);
-
-	dctx = event->ev_arg;
-
-	REQUIRE(DNS_DCTX_VALID(dctx));
-
-	isc_work_enqueue(loop, master_dump_cb, master_dump_done_cb, dctx);
-
-	isc_event_free(&event);
-}
-
-static isc_result_t
-task_send(dns_dumpctx_t *dctx) {
-	isc_event_t *event;
-
-	event = isc_event_allocate(dctx->mctx, NULL, DNS_EVENT_DUMPQUANTUM,
-				   setup_dump, dctx, sizeof(*event));
-	isc_task_send(dctx->task, &event);
-	return (ISC_R_SUCCESS);
-}
-
 static isc_result_t
 dumpctx_create(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version,
 	       const dns_master_style_t *style, FILE *f, dns_dumpctx_t **dctxp,
@@ -1545,19 +1511,11 @@ dumpctx_create(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version,
 	unsigned int options;
 
 	dctx = isc_mem_get(mctx, sizeof(*dctx));
+	*dctx = (dns_dumpctx_t){
+		.f = f,
+		.format = format,
+	};
 
-	dctx->mctx = NULL;
-	dctx->f = f;
-	dctx->dbiter = NULL;
-	dctx->db = NULL;
-	dctx->version = NULL;
-	dctx->done = NULL;
-	dctx->done_arg = NULL;
-	dctx->task = NULL;
-	atomic_init(&dctx->canceled, false);
-	dctx->file = NULL;
-	dctx->tmpfile = NULL;
-	dctx->format = format;
 	if (header == NULL) {
 		dns_master_initrawheader(&dctx->header);
 	} else {
@@ -1772,12 +1730,12 @@ isc_result_t
 dns_master_dumptostreamasync(isc_mem_t *mctx, dns_db_t *db,
 			     dns_dbversion_t *version,
 			     const dns_master_style_t *style, FILE *f,
-			     isc_task_t *task, dns_dumpdonefunc_t done,
+			     isc_loop_t *loop, dns_dumpdonefunc_t done,
 			     void *done_arg, dns_dumpctx_t **dctxp) {
 	dns_dumpctx_t *dctx = NULL;
 	isc_result_t result;
 
-	REQUIRE(task != NULL);
+	REQUIRE(loop != NULL);
 	REQUIRE(f != NULL);
 	REQUIRE(done != NULL);
 
@@ -1786,18 +1744,13 @@ dns_master_dumptostreamasync(isc_mem_t *mctx, dns_db_t *db,
 	if (result != ISC_R_SUCCESS) {
 		return (result);
 	}
-	isc_task_attach(task, &dctx->task);
 	dctx->done = done;
 	dctx->done_arg = done_arg;
 
-	result = task_send(dctx);
-	if (result == ISC_R_SUCCESS) {
-		dns_dumpctx_attach(dctx, dctxp);
-		return (DNS_R_CONTINUE);
-	}
+	dns_dumpctx_attach(dctx, dctxp);
+	isc_work_enqueue(loop, master_dump_cb, master_dump_done_cb, dctx);
 
-	dns_dumpctx_detach(&dctx);
-	return (result);
+	return (ISC_R_SUCCESS);
 }
 
 isc_result_t
@@ -1867,7 +1820,7 @@ cleanup:
 isc_result_t
 dns_master_dumpasync(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version,
 		     const dns_master_style_t *style, const char *filename,
-		     isc_task_t *task, dns_dumpdonefunc_t done, void *done_arg,
+		     isc_loop_t *loop, dns_dumpdonefunc_t done, void *done_arg,
 		     dns_dumpctx_t **dctxp, dns_masterformat_t format,
 		     dns_masterrawheader_t *header) {
 	FILE *f = NULL;
@@ -1880,41 +1833,33 @@ dns_master_dumpasync(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version,
 
 	result = opentmp(mctx, format, filename, &tempname, &f);
 	if (result != ISC_R_SUCCESS) {
-		goto cleanup;
+		goto cleanup_file;
 	}
 
 	result = dumpctx_create(mctx, db, version, style, f, &dctx, format,
 				header);
 	if (result != ISC_R_SUCCESS) {
-		(void)isc_stdio_close(f);
-		(void)isc_file_remove(tempname);
-		goto cleanup;
+		goto cleanup_tempname;
 	}
 
-	isc_task_attach(task, &dctx->task);
 	dctx->done = done;
 	dctx->done_arg = done_arg;
 	dctx->file = file;
-	file = NULL;
 	dctx->tmpfile = tempname;
-	tempname = NULL;
 
-	result = task_send(dctx);
-	if (result == ISC_R_SUCCESS) {
-		dns_dumpctx_attach(dctx, dctxp);
-		return (DNS_R_CONTINUE);
-	}
+	dns_dumpctx_attach(dctx, dctxp);
+	isc_work_enqueue(loop, master_dump_cb, master_dump_done_cb, dctx);
+
+	return (ISC_R_SUCCESS);
+
+cleanup_tempname:
+	(void)isc_stdio_close(f);
+	(void)isc_file_remove(tempname);
+	isc_mem_free(mctx, tempname);
+
+cleanup_file:
+	isc_mem_free(mctx, file);
 
-cleanup:
-	if (dctx != NULL) {
-		dns_dumpctx_detach(&dctx);
-	}
-	if (file != NULL) {
-		isc_mem_free(mctx, file);
-	}
-	if (tempname != NULL) {
-		isc_mem_free(mctx, tempname);
-	}
 	return (result);
 }
 

lib/dns/zone.c

@@ -2691,7 +2691,7 @@ zone_gotwritehandle(isc_task_t *task, isc_event_t *event) {
 		}
 		result = dns_master_dumpasync(
 			zone->mctx, db, version, output_style, zone->masterfile,
-			zone->task, dump_done, zone, &zone->dctx,
+			zone->loop, dump_done, zone, &zone->dctx,
 			zone->masterformat, &rawdata);
 		dns_db_closeversion(db, &version, false);
 	} else {
@@ -2701,7 +2701,7 @@ zone_gotwritehandle(isc_task_t *task, isc_event_t *event) {
 		dns_db_detach(&db);
 	}
 	UNLOCK_ZONE(zone);
-	if (result != DNS_R_CONTINUE) {
+	if (result != ISC_R_SUCCESS) {
 		goto fail;
 	}
 	return;
@@ -12174,8 +12174,6 @@ redo:
 				       &zone->writeio);
 		if (result != ISC_R_SUCCESS) {
 			zone_idetach(&dummy);
-		} else {
-			result = DNS_R_CONTINUE;
 		}
 		UNLOCK_ZONE(zone);
 	} else {
@@ -12205,7 +12203,7 @@ fail:
 	}
 	masterfile = NULL;
 
-	if (result == DNS_R_CONTINUE) {
+	if (result == ISC_R_SUCCESS) {
 		return (ISC_R_SUCCESS); /* XXXMPA */
 	}
 
@@ -23674,3 +23672,8 @@ unsigned int
 dns_zone_gettid(dns_zone_t *zone) {
 	return (zone->tid);
 }
+
+isc_loop_t *
+dns_zone_getloop(dns_zone_t *zone) {
+	return (zone->loop);
+}