- Fix cachedb for serve-expired with serve-expired-client-timeout.

cachedb/cachedb.c

@@ -267,11 +267,6 @@ cachedb_init(struct module_env* env, int id)
 		return 0;
 	}
 	cachedb_env->enabled = 1;
-	if(env->cfg->serve_expired && env->cfg->serve_expired_client_timeout)
-		log_warn(
-			"cachedb: serve-expired-client-timeout is set but not working for "
-			"data originating from the external cache; expired data are used "
-			"in the reply without first trying to refresh the data.");
 	return 1;
 }
 
@@ -753,6 +748,13 @@ cachedb_intcache_store(struct module_qstate* qstate, int msg_expired)
 		qstate->return_msg->rep, 0, qstate->prefetch_leeway, 0,
 		qstate->region, store_flags, qstate->qstarttime);
 	if(serve_expired && msg_expired) {
+		if(qstate->env->cfg->serve_expired_client_timeout) {
+			/* No expired response from the query state, the
+			 * query resolution needs to continue and it can
+			 * pick up the expired result after the timer out
+			 * of cache. */
+			return;
+		}
 		/* set TTLs to zero again */
 		adjust_msg_ttl(qstate->return_msg, -1);
 		/* Send serve expired responses based on the cachedb
@@ -822,8 +824,13 @@ cachedb_handle_query(struct module_qstate* qstate,
 		 * data first.
 		 * TODO: this needs revisit. The expired data stored from cachedb has
 		 * 0 TTL which is picked up by iterator later when looking in the cache.
-		 * Document that ext cachedb does not work properly with
+		 */
-		 * serve_stale_reply_ttl yet. */
+		if(qstate->env->cfg->serve_expired && msg_expired &&
+			qstate->env->cfg->serve_expired_client_timeout) {
+			qstate->return_msg = NULL;
+			qstate->ext_state[id] = module_wait_module;
+			return;
+		}
 		if(qstate->need_refetch && qstate->serve_expired_data &&
 			qstate->serve_expired_data->timer) {
 				qstate->return_msg = NULL;
@@ -837,7 +844,8 @@ cachedb_handle_query(struct module_qstate* qstate,
 	}
 
 	if(qstate->serve_expired_data &&
-		qstate->env->cfg->cachedb_check_when_serve_expired) {
+		qstate->env->cfg->cachedb_check_when_serve_expired &&
+		!qstate->env->cfg->serve_expired_client_timeout) {
 		/* Reply with expired data if any to client, because cachedb
 		 * also has no useful, current data */
 		mesh_respond_serve_expired(qstate->mesh_info);

doc/Changelog

@@ -1,3 +1,6 @@
+12 April 2024: Wouter
+	- Fix cachedb for serve-expired with serve-expired-client-timeout.
+
 10 April 2024: Wouter
 	- Implement cachedb-check-when-serve-expired: yes option, default
 	  is enabled. When serve expired is enabled with cachedb, it first

doc/unbound.conf.5.in

@@ -2640,11 +2640,7 @@ If Unbound cannot even find an answer in the backend, it resolves the
 query as usual, and stores the answer in the backend.
 .P
 This module interacts with the \fBserve\-expired\-*\fR options and will reply
-with expired data if Unbound is configured for that.  Currently the use
+with expired data if Unbound is configured for that.
-of \fBserve\-expired\-client\-timeout:\fR
-is not consistent for data originating from
-the external cache as it will result in a reply without trying to
-update the data first, ignoring the configured value.
 .P
 If Unbound was built with
 \fB\-\-with\-libhiredis\fR
@@ -2707,7 +2703,9 @@ When \fBserve\-expired\fR is enabled, without \fBserve\-expired\-client\-timeout
 does not immediately respond with an expired response from cache, but instead
 first checks the cachedb for valid contents, and if so returns it. If the
 cachedb also has no valid contents, the serve expired response is sent.
-The default is yes.
+If also \fBserve\-expired\-client\-timeout\fR is enabled, the expired response
+is delayed until the timeout expires. Unless the lookup succeeds within the
+timeout. The default is yes.
 .P
 The following
 .B cachedb

testdata/cachedb_expired_client_timeout.crpl

@@ -0,0 +1,338 @@
+; config options
+server:
+	target-fetch-policy: "0 0 0 0 0"
+	qname-minimisation: no
+	minimal-responses: no
+	serve-expired: yes
+	serve-expired-reply-ttl: 30
+	; at least one second, so we can time skip past the timer in the
+	; testbound script steps, but also reply within the time.
+	serve-expired-client-timeout: 1200
+	module-config: "cachedb iterator"
+
+cachedb:
+	backend: "testframe"
+	secret-seed: "testvalue"
+	cachedb-check-when-serve-expired: yes
+
+stub-zone:
+	name: "."
+	stub-addr: 193.0.14.129
+CONFIG_END
+
+SCENARIO_BEGIN Test cachedb and serve-expired-client-timeout.
+
+; K.ROOT-SERVERS.NET.
+RANGE_BEGIN 0 400
+	ADDRESS 193.0.14.129
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR NOERROR
+SECTION QUESTION
+. IN NS
+SECTION ANSWER
+. IN NS K.ROOT-SERVERS.NET.
+SECTION ADDITIONAL
+K.ROOT-SERVERS.NET.     IN      A       193.0.14.129
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode subdomain
+ADJUST copy_id copy_query
+REPLY QR NOERROR
+SECTION QUESTION
+com. IN NS
+SECTION AUTHORITY
+com. IN NS a.gtld-servers.net.
+SECTION ADDITIONAL
+a.gtld-servers.net.	IN	A	192.5.6.30
+ENTRY_END
+RANGE_END
+
+; a.gtld-servers.net.
+RANGE_BEGIN 0 400
+	ADDRESS 192.5.6.30
+ENTRY_BEGIN
+MATCH opcode subdomain
+ADJUST copy_id copy_query
+REPLY QR NOERROR
+SECTION QUESTION
+example.com. IN NS
+SECTION AUTHORITY
+example.com. IN NS ns2.example.com.
+SECTION ADDITIONAL
+ns2.example.com.	IN	A	1.2.3.5
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode subdomain
+ADJUST copy_id copy_query
+REPLY QR NOERROR
+SECTION QUESTION
+foo.com. IN NS
+SECTION AUTHORITY
+foo.com. IN NS ns.example.com.
+ENTRY_END
+RANGE_END
+
+; ns2.example.com.
+RANGE_BEGIN 0 60
+	ADDRESS 1.2.3.5
+ENTRY_BEGIN
+MATCH opcode qname qtype
+REPLY QR AA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 10 IN A 1.2.3.4
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode qname qtype
+REPLY QR AA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 10 IN A 1.2.3.5
+ENTRY_END
+RANGE_END
+
+; ns2.example.com. - after a change
+RANGE_BEGIN 80 90
+	ADDRESS 1.2.3.5
+ENTRY_BEGIN
+MATCH opcode qname qtype
+REPLY QR AA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 10 IN A 1.2.3.6
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode qname qtype
+REPLY QR AA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 10 IN A 1.2.3.7
+ENTRY_END
+RANGE_END
+
+; ns2.example.com. - steps 90-120 not responding.
+
+; ns2.example.com. - after a change
+RANGE_BEGIN 130 140
+	ADDRESS 1.2.3.5
+ENTRY_BEGIN
+MATCH opcode qname qtype
+REPLY QR AA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 10 IN A 1.2.3.8
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode qname qtype
+REPLY QR AA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 10 IN A 1.2.3.9
+ENTRY_END
+RANGE_END
+
+; ns2.example.com. - steps 150-160 not responding.
+
+; ns2.example.com. - after a change
+RANGE_BEGIN 170 200
+	ADDRESS 1.2.3.5
+ENTRY_BEGIN
+MATCH opcode qname qtype
+REPLY QR AA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 10 IN A 1.2.3.10
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode qname qtype
+REPLY QR AA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 10 IN A 1.2.3.11
+ENTRY_END
+RANGE_END
+
+; make time not 0
+STEP 2 TIME_PASSES ELAPSE 212
+
+; Get an entry in cache.
+STEP 4 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+; get the answer for it
+STEP 10 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 10 IN A 1.2.3.4
+ENTRY_END
+
+; Get another query in cache.
+STEP 20 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www2.example.com. IN A
+ENTRY_END
+
+; get the answer for it
+STEP 30 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 10 IN A 1.2.3.5
+ENTRY_END
+
+; www.example.com and www2.example.com are in cache, www2 in cachedb.
+STEP 40 FLUSH_MESSAGE www2.example.com. IN A
+; now www in cache, www2 not in cache, www2 in cachedb.
+; because of the client timeout, it should be able to use the
+; response from cachedb for www2.
+
+STEP 50 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www2.example.com. IN A
+ENTRY_END
+
+STEP 60 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 10 IN A 1.2.3.5
+ENTRY_END
+
+; make both cache and cachedb expired
+STEP 70 TIME_PASSES ELAPSE 20
+
+; www and www2 expired in cache, www2 expired in cachedb.
+; the query should now try to resolve and complete within the
+; client timeout, and return the upstream version.
+; the upstream is changed to give a different one now.
+STEP 80 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www2.example.com. IN A
+ENTRY_END
+
+STEP 90 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 10 IN A 1.2.3.7
+ENTRY_END
+
+; expire the data again
+STEP 100 TIME_PASSES ELAPSE 20
+
+; the query should now try to resolve, but the upstream is not
+; responsive for several testbound steps. When the timer expires,
+; the expired answer should be returned.
+
+; www2 expired in cache and www2 expired in cachedb.
+STEP 110 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www2.example.com. IN A
+ENTRY_END
+
+; make 2 seconds pass to go past the client timeout
+STEP 112 TIME_PASSES ELAPSE 2
+
+STEP 120 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 30 IN A 1.2.3.7
+ENTRY_END
+
+; make traffic flow to resolve the query, server responds.
+STEP 130 TRAFFIC
+
+; expire the data again
+STEP 140 TIME_PASSES ELAPSE 20
+
+; The client query tries to resolve, but gets no immediate answer,
+; so the expired data is used. But the expired data is in cache and
+; the query is not in cachedb.
+STEP 150 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+; make 2 seconds pass to go past the client timeout
+STEP 152 TIME_PASSES ELAPSE 2
+
+STEP 160 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 30 IN A 1.2.3.4
+ENTRY_END
+
+; make traffic flow to resolve the query, server responds.
+STEP 170 TRAFFIC
+
+; now the client query tries to resolve, and completes within the client
+; timeout, but there is expired data in cache but not in cachedb.
+STEP 180 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www2.example.com. IN A
+ENTRY_END
+
+STEP 190 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 10 IN A 1.2.3.11
+ENTRY_END
+
+SCENARIO_END