Incrementally apply AXFR transfer

Reintroduce logic to apply diffs when the number of pending tuples is above 128. The previous strategy of accumulating all the tuples and pushing them at the end leads to excessive memory consumption during transfer. This effectively reverts half of e3892805d6
2025-08-31 14:35:26 +00:00 · 2024-11-03 21:25:15 +01:00
parent 9f628a34cd
commit 99b4f01b33
13 changed files with 283 additions and 53 deletions
--- a/bin/tests/system/masterformat/ns1/compile.sh
+++ b/bin/tests/system/masterformat/ns1/compile.sh
@@ -28,9 +28,9 @@ $CHECKZONE -D -F raw -L 3333 -o example.db.serial.raw example \
  example.db >/dev/null 2>&1
 $CHECKZONE -D -F raw -o under-limit.db.raw under-limit under-limit.db >/dev/null 2>&1
 $CHECKZONE -D -F raw -o under-limit-kasp.db.raw under-limit-kasp under-limit-kasp.db >/dev/null 2>&1
-$CHECKZONE -D -F raw -o on-limit.db.raw on-limit on-limit.db >/dev/null 2>&1
-$CHECKZONE -D -F raw -o on-limit-kasp.db.raw on-limit-kasp on-limit-kasp.db >/dev/null 2>&1
-$CHECKZONE -D -F raw -o over-limit.db.raw over-limit over-limit.db >/dev/null 2>&1
+$CHECKZONE -D -F raw -o below-limit.db.raw below-limit below-limit.db >/dev/null 2>&1
+$CHECKZONE -D -F raw -o below-limit-kasp.db.raw below-limit-kasp below-limit-kasp.db >/dev/null 2>&1
+$CHECKZONE -D -F raw -o above-limit.db.raw above-limit above-limit.db >/dev/null 2>&1
 $CHECKZONE -D -F raw -o 255types.db.raw 255types 255types.db >/dev/null 2>&1

 $KEYGEN -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -f KSK signed >/dev/null 2>&1
--- a/bin/tests/system/masterformat/ns1/named.conf.in
+++ b/bin/tests/system/masterformat/ns1/named.conf.in
@@ -95,16 +95,16 @@ zone "under-limit-kasp" {
 	allow-transfer { any; };
 };

-zone "on-limit" {
+zone "below-limit" {
 	type primary;
-	file "on-limit.db.raw";
+	file "below-limit.db.raw";
 	masterfile-format raw;
 	allow-transfer { any; };
 };

-zone "on-limit-kasp" {
+zone "below-limit-kasp" {
 	type primary;
-	file "on-limit-kasp.db.raw";
+	file "below-limit-kasp.db.raw";
 	masterfile-format raw;
 	dnssec-policy masterformat;
 	inline-signing no;
@@ -112,9 +112,9 @@ zone "on-limit-kasp" {
 	allow-transfer { any; };
 };

-zone "over-limit" {
+zone "above-limit" {
 	type primary;
-	file "over-limit.db.raw";
+	file "above-limit.db.raw";
 	masterfile-format raw;
 	allow-transfer { any; };
 };
--- a/bin/tests/system/masterformat/ns2/named.conf.in
+++ b/bin/tests/system/masterformat/ns2/named.conf.in
@@ -72,18 +72,18 @@ zone "under-limit-kasp" {
 	file "under-limit-kasp.bk";
 };

-zone "on-limit" {
+zone "below-limit" {
 	type secondary;
 	primaries { 10.53.0.1; };
 	masterfile-format raw;
-	file "on-limit.bk";
+	file "below-limit.bk";
 };

-zone "on-limit-kasp" {
+zone "below-limit-kasp" {
 	type secondary;
 	primaries { 10.53.0.1; };
 	masterfile-format raw;
-	file "on-limit-kasp.bk";
+	file "below-limit-kasp.bk";
 };

 zone "255types" {
--- a/bin/tests/system/masterformat/setup.sh
+++ b/bin/tests/system/masterformat/setup.sh
@@ -33,23 +33,23 @@ awk 'END {
 }' </dev/null >>ns1/under-limit.db
 cp ns1/under-limit.db ns1/under-limit-kasp.db

-cp ns1/empty.db.in ns1/on-limit.db
+cp ns1/empty.db.in ns1/below-limit.db
 awk 'END {
 	 for (i = 0; i < 500; i++ ) { print "500-txt TXT", i; }
 	 for (i = 0; i < 1000; i++ ) { print "1000-txt TXT", i; }
 	 for (i = 0; i < 2000; i++ ) { print "2000-txt TXT", i; }
 	 for (i = 0; i < 2050; i++ ) { print "2050-txt TXT", i; }
-}' </dev/null >>ns1/on-limit.db
-cp ns1/on-limit.db ns1/on-limit-kasp.db
+}' </dev/null >>ns1/below-limit.db
+cp ns1/below-limit.db ns1/below-limit-kasp.db

-cp ns1/empty.db.in ns1/over-limit.db
+cp ns1/empty.db.in ns1/above-limit.db
 awk 'END {
 	 for (i = 0; i < 500; i++ ) { print "500-txt TXT", i; }
 	 for (i = 0; i < 1000; i++ ) { print "1000-txt TXT", i; }
 	 for (i = 0; i < 2000; i++ ) { print "2000-txt TXT", i; }
 	 for (i = 0; i < 2050; i++ ) { print "2050-txt TXT", i; }
 	 for (i = 0; i < 2100; i++ ) { print "2100-txt TXT", i; }
-}' </dev/null >>ns1/over-limit.db
+}' </dev/null >>ns1/above-limit.db

 cp ns1/empty.db.in ns1/255types.db
 for ntype in $(seq 65280 65534); do
--- a/bin/tests/system/masterformat/tests.sh
+++ b/bin/tests/system/masterformat/tests.sh
@@ -244,11 +244,11 @@ n=$((n + 1))
 [ $ret -eq 0 ] || echo_i "failed"
 status=$((status + ret))

-echo_i "checking that on-limit rdatasets loaded ($n)"
+echo_i "checking that below-limit rdatasets loaded ($n)"
 for _attempt in 0 1 2 3 4 5 6 7 8 9; do
  ret=0
  for rrcount in 500-txt 1000-txt 2000-txt 2050-txt; do
-    $DIG +tcp txt "${rrcount}.on-limit" @10.53.0.1 -p "${PORT}" >"dig.out.ns1.$rrcount.test$n"
+    $DIG +tcp txt "${rrcount}.below-limit" @10.53.0.1 -p "${PORT}" >"dig.out.ns1.$rrcount.test$n"
    grep "status: NOERROR" "dig.out.ns1.$rrcount.test$n" >/dev/null || ret=1
  done
  [ $ret -eq 0 ] && break
@@ -258,11 +258,11 @@ n=$((n + 1))
 [ $ret -eq 0 ] || echo_i "failed"
 status=$((status + ret))

-echo_i "checking that on-limit rdatasets not transfered ($n)"
+echo_i "checking that below-limit rdatasets not transfered ($n)"
 for _attempt in 0 1 2 3 4 5 6 7 8 9; do
  ret=0
  for rrcount in 500-txt 1000-txt 2000-txt 2050-txt; do
-    $DIG +tcp txt "${rrcount}.on-limit" @10.53.0.2 -p "${PORT}" >"dig.out.ns2.$rrcount.test$n"
+    $DIG +tcp txt "${rrcount}.below-limit" @10.53.0.2 -p "${PORT}" >"dig.out.ns2.$rrcount.test$n"
    grep "status: SERVFAIL" "dig.out.ns2.$rrcount.test$n" >/dev/null || ret=1
  done
  [ $ret -eq 0 ] && break
@@ -272,11 +272,11 @@ n=$((n + 1))
 [ $ret -eq 0 ] || echo_i "failed"
 status=$((status + ret))

-echo_i "checking that on-limit-kasp rdatasets loaded ($n)"
+echo_i "checking that below-limit-kasp rdatasets loaded ($n)"
 for _attempt in 0 1 2 3 4 5 6 7 8 9; do
  ret=0
  for rrcount in 500-txt 1000-txt 2000-txt 2050-txt; do
-    $DIG +tcp +dnssec txt "${rrcount}.on-limit-kasp" @10.53.0.1 -p "${PORT}" >"dig.out.ns1.$rrcount.test$n"
+    $DIG +tcp +dnssec txt "${rrcount}.below-limit-kasp" @10.53.0.1 -p "${PORT}" >"dig.out.ns1.$rrcount.test$n"
    grep "status: NOERROR" "dig.out.ns1.$rrcount.test$n" >/dev/null || ret=1
    grep "RRSIG" "dig.out.ns1.$rrcount.test$n" >/dev/null || ret=1
  done
@@ -287,11 +287,11 @@ n=$((n + 1))
 [ $ret -eq 0 ] || echo_i "failed"
 status=$((status + ret))

-echo_i "checking that on-limit-kasp rdatasets not transfered ($n)"
+echo_i "checking that below-limit-kasp rdatasets not transfered ($n)"
 for _attempt in 0 1 2 3 4 5 6 7 8 9; do
  ret=0
  for rrcount in 500-txt 1000-txt 2000-txt 2050-txt; do
-    $DIG +tcp +dnssec txt "${rrcount}.on-limit-kasp" @10.53.0.2 -p "${PORT}" >"dig.out.ns2.$rrcount.test$n"
+    $DIG +tcp +dnssec txt "${rrcount}.below-limit-kasp" @10.53.0.2 -p "${PORT}" >"dig.out.ns2.$rrcount.test$n"
    grep "status: SERVFAIL" "dig.out.ns2.$rrcount.test$n" >/dev/null || ret=1
  done
  [ $ret -eq 0 ] && break
@@ -301,11 +301,11 @@ n=$((n + 1))
 [ $ret -eq 0 ] || echo_i "failed"
 status=$((status + ret))

-echo_i "checking that over-limit rdatasets not loaded ($n)"
+echo_i "checking that above-limit rdatasets not loaded ($n)"
 for _attempt in 0 1 2 3 4 5 6 7 8 9; do
  ret=0
  for rrcount in 500-txt 1000-txt 2000-txt 2050-txt 2100-txt; do
-    $DIG +tcp txt "${rrcount}.over-limit" @10.53.0.1 -p "${PORT}" >"dig.out.ns1.$rrcount.test$n"
+    $DIG +tcp txt "${rrcount}.above-limit" @10.53.0.1 -p "${PORT}" >"dig.out.ns1.$rrcount.test$n"
    grep "status: SERVFAIL" "dig.out.ns1.$rrcount.test$n" >/dev/null || ret=1
  done
  [ $ret -eq 0 ] && break
@@ -519,7 +519,7 @@ n=$((n + 1))
 [ $ret -eq 0 ] || echo_i "failed"
 status=$((status + ret))

-echo_i "checking that on-limit-kasp rdatasets loaded after re-sign and re-start ($n)"
+echo_i "checking that below-limit-kasp rdatasets loaded after re-sign and re-start ($n)"
 ret=0
 stop_server ns1
 start_server --noclean --restart --port "${PORT}" ns1
@@ -527,7 +527,7 @@ start_server --noclean --restart --port "${PORT}" ns1
 for _attempt in 0 1 2 3 4 5 6 7 8 9; do
  ret=0
  for rrcount in 500-txt 1000-txt 2000-txt 2050-txt; do
-    $DIG +tcp +dnssec txt "${rrcount}.on-limit-kasp" @10.53.0.1 -p "${PORT}" >"dig.out.ns1.$rrcount.test$n"
+    $DIG +tcp +dnssec txt "${rrcount}.below-limit-kasp" @10.53.0.1 -p "${PORT}" >"dig.out.ns1.$rrcount.test$n"
    grep "status: NOERROR" "dig.out.ns1.$rrcount.test$n" >/dev/null || ret=1
    grep "RRSIG" "dig.out.ns1.$rrcount.test$n" >/dev/null || ret=1
  done
--- a/bin/tests/system/masterformat/tests_sh_masterformat.py
+++ b/bin/tests/system/masterformat/tests_sh_masterformat.py
@@ -26,9 +26,9 @@ pytestmark = pytest.mark.extra_artifacts(
        "ns*/K*",
        "ns1/255types.db",
        "ns1/example.db.compat",
-        "ns1/on-limit-kasp.db",
-        "ns1/on-limit.db",
-        "ns1/over-limit.db",
+        "ns1/below-limit-kasp.db",
+        "ns1/below-limit.db",
+        "ns1/above-limit.db",
        "ns1/under-limit-kasp.db",
        "ns1/under-limit.db",
        "ns2/db-*",
--- a/lib/dns/diff.c
+++ b/lib/dns/diff.c
@@ -15,6 +15,7 @@

 #include <inttypes.h>
 #include <stdbool.h>
+#include <stddef.h>
 #include <stdlib.h>

 #include <isc/buffer.h>
@@ -123,6 +124,7 @@ dns_diff_init(isc_mem_t *mctx, dns_diff_t *diff) {
 	diff->mctx = mctx;
 	ISC_LIST_INIT(diff->tuples);
 	diff->magic = DNS_DIFF_MAGIC;
+	diff->size = 0;
 }

 void
@@ -133,15 +135,37 @@ dns_diff_clear(dns_diff_t *diff) {
 		ISC_LIST_UNLINK(diff->tuples, t, link);
 		dns_difftuple_free(&t);
 	}
+	diff->size = 0;
 	ENSURE(ISC_LIST_EMPTY(diff->tuples));
 }

 void
 dns_diff_append(dns_diff_t *diff, dns_difftuple_t **tuplep) {
+	REQUIRE(DNS_DIFF_VALID(diff));
 	ISC_LIST_APPEND(diff->tuples, *tuplep, link);
+	diff->size += 1;
 	*tuplep = NULL;
 }

+bool
+dns_diff_is_boundary(const dns_diff_t *diff, dns_name_t *new_name) {
+	REQUIRE(DNS_DIFF_VALID(diff));
+	REQUIRE(DNS_NAME_VALID(new_name));
+
+	if (ISC_LIST_EMPTY(diff->tuples)) {
+		return false;
+	}
+
+	dns_difftuple_t *tail = ISC_LIST_TAIL(diff->tuples);
+	return !dns_name_caseequal(&tail->name, new_name);
+}
+
+size_t
+dns_diff_size(const dns_diff_t *diff) {
+	REQUIRE(DNS_DIFF_VALID(diff));
+	return diff->size;
+}
+
 /* XXX this is O(N) */

 void
@@ -170,6 +194,9 @@ dns_diff_appendminimal(dns_diff_t *diff, dns_difftuple_t **tuplep) {
 		    ot->ttl == (*tuplep)->ttl)
 		{
 			ISC_LIST_UNLINK(diff->tuples, ot, link);
+			INSIST(diff->size > 0);
+			diff->size -= 1;
+
 			if ((*tuplep)->op == ot->op) {
 				UNEXPECTED_ERROR("unexpected non-minimal diff");
 			} else {
@@ -182,6 +209,7 @@ dns_diff_appendminimal(dns_diff_t *diff, dns_difftuple_t **tuplep) {

 	if (*tuplep != NULL) {
 		ISC_LIST_APPEND(diff->tuples, *tuplep, link);
+		diff->size += 1;
 		*tuplep = NULL;
 	}
 }
@@ -253,7 +281,8 @@ optotext(dns_diffop_t op) {
 }

 static isc_result_t
-diff_apply(dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver, bool warn) {
+diff_apply(const dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver,
+	   bool warn) {
 	dns_difftuple_t *t;
 	dns_dbnode_t *node = NULL;
 	isc_result_t result;
@@ -493,19 +522,20 @@ failure:
 }

 isc_result_t
-dns_diff_apply(dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver) {
+dns_diff_apply(const dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver) {
 	return diff_apply(diff, db, ver, true);
 }

 isc_result_t
-dns_diff_applysilently(dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver) {
+dns_diff_applysilently(const dns_diff_t *diff, dns_db_t *db,
+		       dns_dbversion_t *ver) {
 	return diff_apply(diff, db, ver, false);
 }

 /* XXX this duplicates lots of code in diff_apply(). */

 isc_result_t
-dns_diff_load(dns_diff_t *diff, dns_rdatacallbacks_t *callbacks) {
+dns_diff_load(const dns_diff_t *diff, dns_rdatacallbacks_t *callbacks) {
 	dns_difftuple_t *t;
 	isc_result_t result;

@@ -641,7 +671,7 @@ diff_tuple_tordataset(dns_difftuple_t *t, dns_rdata_t *rdata,
 }

 isc_result_t
-dns_diff_print(dns_diff_t *diff, FILE *file) {
+dns_diff_print(const dns_diff_t *diff, FILE *file) {
 	isc_result_t result;
 	dns_difftuple_t *t;
 	char *mem = NULL;
--- a/lib/dns/include/dns/diff.h
+++ b/lib/dns/include/dns/diff.h
@@ -27,6 +27,8 @@
 *** Imports
 ***/

+#include <stddef.h>
+
 #include <isc/lang.h>
 #include <isc/magic.h>

@@ -97,6 +99,7 @@ struct dns_diff {
 	unsigned int	    magic;
 	isc_mem_t	   *mctx;
 	dns_difftuplelist_t tuples;
+	size_t		    size;
 };

 /* Type of comparison function for sorting diffs. */
@@ -186,6 +189,7 @@ dns_diff_append(dns_diff_t *diff, dns_difftuple_t **tuple);
 /*%<
 * Append a single tuple to a diff.
 *
+ * Requires:
 * \li	'diff' is a valid diff.
 * \li	'*tuple' is a valid tuple.
 *
@@ -194,6 +198,25 @@ dns_diff_append(dns_diff_t *diff, dns_difftuple_t **tuple);
 * \li	The tuple has been freed, or will be freed when the diff is cleared.
 */

+bool
+dns_diff_is_boundary(const dns_diff_t *diff, dns_name_t *name);
+/*%<
+ * Checks if 'name' is equal, up to case, to the last name of the diff.
+ *
+ * Requires:
+ * \li	'diff' is a valid diff.
+ * \li	'name' is a valid dns name.
+ */
+
+size_t
+dns_diff_size(const dns_diff_t *diff);
+/*%<
+ * Returns the number of elements in the diff.
+ *
+ * Requires:
+ * \li	'diff' is a valid diff.
+ */
+
 void
 dns_diff_appendminimal(dns_diff_t *diff, dns_difftuple_t **tuple);
 /*%<
@@ -218,9 +241,10 @@ dns_diff_sort(dns_diff_t *diff, dns_diff_compare_func *compare);
 */

 isc_result_t
-dns_diff_apply(dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver);
+dns_diff_apply(const dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver);
 isc_result_t
-dns_diff_applysilently(dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver);
+dns_diff_applysilently(const dns_diff_t *diff, dns_db_t *db,
+		       dns_dbversion_t *ver);
 /*%<
 * Apply 'diff' to the database 'db'.
 *
@@ -239,7 +263,7 @@ 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_rdatacallbacks_t *callbacks);
+dns_diff_load(const 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
@@ -251,7 +275,7 @@ dns_diff_load(dns_diff_t *diff, dns_rdatacallbacks_t *callbacks);
 */

 isc_result_t
-dns_diff_print(dns_diff_t *diff, FILE *file);
+dns_diff_print(const dns_diff_t *diff, FILE *file);

 /*%<
 * Print the differences to 'file' or if 'file' is NULL via the
--- a/lib/dns/xfrin.c
+++ b/lib/dns/xfrin.c
@@ -201,9 +201,13 @@ struct dns_xfrin {
 #define XFRIN_MAGIC    ISC_MAGIC('X', 'f', 'r', 'I')
 #define VALID_XFRIN(x) ISC_MAGIC_VALID(x, XFRIN_MAGIC)

+#define XFRIN_WORK_MAGIC    ISC_MAGIC('X', 'f', 'r', 'W')
+#define VALID_XFRIN_WORK(x) ISC_MAGIC_VALID(x, XFRIN_WORK_MAGIC)
+
 typedef struct xfrin_work {
-	dns_xfrin_t *xfr;
+	unsigned int magic;
 	isc_result_t result;
+	dns_xfrin_t *xfr;
 } xfrin_work_t;

 /**************************************************************************/
@@ -300,6 +304,9 @@ failure:
 	return result;
 }

+static void
+axfr_apply(void *arg);
+
 static isc_result_t
 axfr_putdata(dns_xfrin_t *xfr, dns_diffop_t op, dns_name_t *name, dns_ttl_t ttl,
 	     dns_rdata_t *rdata) {
@@ -312,8 +319,21 @@ axfr_putdata(dns_xfrin_t *xfr, dns_diffop_t op, dns_name_t *name, dns_ttl_t ttl,
 	}

 	CHECK(dns_zone_checknames(xfr->zone, name, rdata));
+	if (dns_diff_size(&xfr->diff) > 128 &&
+	    dns_diff_is_boundary(&xfr->diff, name))
+	{
+		xfrin_work_t work = (xfrin_work_t){
+			.magic = XFRIN_WORK_MAGIC,
+			.result = ISC_R_UNSET,
+			.xfr = xfr,
+		};
+		axfr_apply((void *)&work);
+		CHECK(work.result);
+	}
+
 	dns_difftuple_create(xfr->diff.mctx, op, name, ttl, rdata, &tuple);
 	dns_diff_append(&xfr->diff, &tuple);
+
 	result = ISC_R_SUCCESS;
 failure:
 	return result;
@@ -325,12 +345,14 @@ failure:
 static void
 axfr_apply(void *arg) {
 	xfrin_work_t *work = arg;
+	REQUIRE(VALID_XFRIN_WORK(work));
+
 	dns_xfrin_t *xfr = work->xfr;
+	REQUIRE(VALID_XFRIN(xfr));
+
 	isc_result_t result = ISC_R_SUCCESS;
 	uint64_t records;

-	REQUIRE(VALID_XFRIN(xfr));
-
 	if (atomic_load(&xfr->shuttingdown)) {
 		result = ISC_R_SHUTTINGDOWN;
 		goto failure;
@@ -357,6 +379,7 @@ axfr_apply_done(void *arg) {
 	isc_result_t result = work->result;

 	REQUIRE(VALID_XFRIN(xfr));
+	REQUIRE(VALID_XFRIN_WORK(work));

 	if (atomic_load(&xfr->shuttingdown)) {
 		result = ISC_R_SHUTTINGDOWN;
@@ -392,8 +415,9 @@ axfr_commit(dns_xfrin_t *xfr) {

 	xfrin_work_t *work = isc_mem_get(xfr->mctx, sizeof(*work));
 	*work = (xfrin_work_t){
-		.xfr = dns_xfrin_ref(xfr),
+		.magic = XFRIN_WORK_MAGIC,
 		.result = ISC_R_UNSET,
+		.xfr = dns_xfrin_ref(xfr),
 	};
 	xfr->diff_running = true;
 	isc_work_enqueue(xfr->loop, axfr_apply, axfr_apply_done, work);
@@ -527,6 +551,7 @@ ixfr_apply(void *arg) {
 	isc_result_t result = ISC_R_SUCCESS;

 	REQUIRE(VALID_XFRIN(xfr));
+	REQUIRE(VALID_XFRIN_WORK(work));

 	struct __cds_wfcq_head diff_head;
 	struct cds_wfcq_tail diff_tail;
@@ -564,11 +589,13 @@ ixfr_apply(void *arg) {
 static void
 ixfr_apply_done(void *arg) {
 	xfrin_work_t *work = arg;
-	dns_xfrin_t *xfr = work->xfr;
-	isc_result_t result = work->result;
+	REQUIRE(VALID_XFRIN_WORK(work));

+	dns_xfrin_t *xfr = work->xfr;
 	REQUIRE(VALID_XFRIN(xfr));

+	isc_result_t result = work->result;
+
 	if (atomic_load(&xfr->shuttingdown)) {
 		result = ISC_R_SHUTTINGDOWN;
 	}
@@ -629,8 +656,9 @@ ixfr_commit(dns_xfrin_t *xfr) {
 	if (!xfr->diff_running) {
 		xfrin_work_t *work = isc_mem_get(xfr->mctx, sizeof(*work));
 		*work = (xfrin_work_t){
-			.xfr = dns_xfrin_ref(xfr),
+			.magic = XFRIN_WORK_MAGIC,
 			.result = ISC_R_UNSET,
+			.xfr = dns_xfrin_ref(xfr),
 		};
 		xfr->diff_running = true;
 		isc_work_enqueue(xfr->loop, ixfr_apply, ixfr_apply_done, work);
@@ -1097,7 +1125,6 @@ xfrin_reset(dns_xfrin_t *xfr) {
 	}

 	dns_diff_clear(&xfr->diff);
-
 	xfr->ixfr.diffs = 0;

 	if (xfr->ixfr.journal != NULL) {
--- a/lib/isc/include/isc/region.h
+++ b/lib/isc/include/isc/region.h
@@ -85,6 +85,8 @@ isc_region_compare(isc_region_t *r1, isc_region_t *r2);
 * Requires:
 *\li	'r1' is a valid region
 *\li	'r2' is a valid region
+ *\li	'r1->base' is not null
+ *\li	'r2->base' is not null
 *
 * Returns:
 *\li	 < 0 if r1 is lexicographically less than r2
--- a/lib/isc/region.c
+++ b/lib/isc/region.c
@@ -26,6 +26,8 @@ isc_region_compare(isc_region_t *r1, isc_region_t *r2) {

 	REQUIRE(r1 != NULL);
 	REQUIRE(r2 != NULL);
+	REQUIRE(r1->base != NULL);
+	REQUIRE(r2->base != NULL);

 	l = (r1->length < r2->length) ? r1->length : r2->length;

--- a/tests/dns/Makefile.am
+++ b/tests/dns/Makefile.am
@@ -24,6 +24,7 @@ check_PROGRAMS =		\
 	dbdiff_test		\
 	dbiterator_test		\
 	dbversion_test		\
+	diff_test		\
 	dispatch_test		\
 	dns64_test		\
 	dst_test		\
--- a/tests/dns/diff_test.c
+++ b/tests/dns/diff_test.c
@@ -0,0 +1,144 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * SPDX-License-Identifier: MPL-2.0
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+/* sched.h must be imported before cmocka to avoid redefinition errors */
+#include <sched.h> /* IWYU pragma: keep */
+#include <setjmp.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "isc/list.h"
+
+#define UNIT_TESTING
+#include <cmocka.h>
+
+#include <dns/diff.h>
+
+#include <tests/dns.h>
+
+unsigned char data_1[] = "\006name_1";
+unsigned char offsets_1[] = { 0, 7 };
+dns_name_t name_1 = DNS_NAME_INITABSOLUTE(data_1, offsets_1);
+
+unsigned char data_2[] = "\006name_2";
+unsigned char offsets_2[] = { 0, 7 };
+dns_name_t name_2 = DNS_NAME_INITABSOLUTE(data_2, offsets_2);
+
+unsigned char data_3[] = "\006name_3";
+unsigned char offsets_3[] = { 0, 7 };
+dns_name_t name_3 = DNS_NAME_INITABSOLUTE(data_3, offsets_3);
+
+unsigned char data_dup[] = "\006name_1";
+unsigned char offsets_dup[] = { 0, 7 };
+dns_name_t name_dup = DNS_NAME_INITABSOLUTE(data_dup, offsets_dup);
+
+unsigned char data_nodup[] = "\006name_1";
+unsigned char offsets_nodup[] = { 0, 7 };
+dns_name_t name_nodup = DNS_NAME_INITABSOLUTE(data_nodup, offsets_nodup);
+
+static size_t
+count_elements(const dns_diff_t *diff) {
+	dns_difftuple_t *ot = NULL;
+	size_t count = 0;
+
+	for (ot = ISC_LIST_HEAD(diff->tuples); ot != NULL;
+	     ot = ISC_LIST_NEXT(ot, link))
+	{
+		++count;
+	}
+
+	return count;
+}
+
+static void
+prepare_rdata(dns_rdata_t *rdata, unsigned char *dest, size_t dest_size) {
+	dns_rdataclass_t rdclass = dns_rdataclass_in;
+	dns_rdatatype_t type = dns_rdatatype_wallet;
+	const char text[] = "cid-example wid-example";
+
+	*rdata = (dns_rdata_t)DNS_RDATA_INIT;
+	isc_result_t result = dns_test_rdatafromstring(
+		rdata, rdclass, type, dest, dest_size, text, false);
+	INSIST(result == ISC_R_SUCCESS);
+}
+
+ISC_RUN_TEST_IMPL(dns_diff_size) {
+	dns_diff_t diff;
+	dns_diff_init(mctx, &diff);
+
+	assert_true(dns_diff_size(&diff) == 0);
+
+	dns_rdata_t rdatas[5] = { 0 };
+	unsigned char bufs[sizeof(rdatas) / sizeof(*rdatas)][128] = { 0 };
+	size_t buf_len = sizeof(bufs[0]);
+
+	for (size_t idx = 0; idx < sizeof(rdatas) / sizeof(*rdatas); ++idx) {
+		prepare_rdata(&rdatas[idx], bufs[idx], buf_len);
+	}
+
+	dns_difftuple_t *tup_1 = NULL, *tup_2 = NULL, *tup_3 = NULL;
+	dns_difftuple_create(mctx, DNS_DIFFOP_ADD, &name_1, 1, &rdatas[0],
+			     &tup_1);
+	dns_difftuple_create(mctx, DNS_DIFFOP_DEL, &name_2, 1, &rdatas[1],
+			     &tup_2);
+	dns_difftuple_create(mctx, DNS_DIFFOP_DEL, &name_3, 1, &rdatas[2],
+			     &tup_3);
+
+	dns_difftuple_t *tup_dup = NULL, *tup_nodup = NULL;
+	dns_difftuple_create(mctx, DNS_DIFFOP_DEL, &name_dup, 1, &rdatas[3],
+			     &tup_dup);
+	dns_difftuple_create(mctx, DNS_DIFFOP_ADD, &name_nodup, 1, &rdatas[4],
+			     &tup_nodup);
+
+	dns_diff_append(&diff, &tup_1);
+	assert_true(dns_diff_size(&diff) == 1);
+	assert_true(dns_diff_size(&diff) == count_elements(&diff));
+
+	dns_diff_append(&diff, &tup_2);
+	assert_true(dns_diff_size(&diff) == 2);
+	assert_true(dns_diff_size(&diff) == count_elements(&diff));
+
+	dns_diff_appendminimal(&diff, &tup_dup);
+	assert_true(dns_diff_size(&diff) == 1);
+	assert_true(dns_diff_size(&diff) == count_elements(&diff));
+
+	dns_diff_append(&diff, &tup_3);
+	assert_true(dns_diff_size(&diff) == 2);
+	assert_true(dns_diff_size(&diff) == count_elements(&diff));
+
+	dns_diff_appendminimal(&diff, &tup_nodup);
+	assert_true(dns_diff_size(&diff) == 3);
+	assert_true(dns_diff_size(&diff) == count_elements(&diff));
+
+	dns_diff_clear(&diff);
+	assert_true(dns_diff_size(&diff) == 0);
+	assert_true(dns_diff_size(&diff) == count_elements(&diff));
+
+	dns_difftuple_t *to_clear[] = { tup_1, tup_2, tup_3, tup_dup,
+					tup_nodup };
+	size_t to_clear_size = sizeof(to_clear) / sizeof(*to_clear);
+
+	for (size_t idx = 0; idx < to_clear_size; ++idx) {
+		if (to_clear[idx] != NULL) {
+			dns_difftuple_free(&to_clear[idx]);
+		}
+	}
+}
+
+ISC_TEST_LIST_START
+ISC_TEST_ENTRY(dns_diff_size)
+ISC_TEST_LIST_END
+
+ISC_TEST_MAIN