Remove cleaning-interval remnants.

Since 2008, the cleaning-interval timer has been documented as
"effectively obsolete" and disabled in the default configuration with
a comment saying "now meaningless".

This change deletes all the code that implements the cleaning-interval
timer, except for the config parser in whcih it is now explicitly
marked as obsolete.

I have verified (using the deletelru and deletettl cache stats) that
named still cleans the cache after this change.

CHANGES

@@ -1,3 +1,6 @@
+5247.	[cleanup]	The 'cleaning-interval' option has been removed.
+			[GL !1731]
+
 5246.	[func]		Log TSIG if appropriate in 'sending notify to' message.
 			[GL #1058]
 

bin/named/config.c

@@ -140,7 +140,6 @@ options {\n\
 	check-names response ignore;\n\
 	check-names slave warn;\n\
 	check-spf warn;\n\
-	cleaning-interval 0;  /* now meaningless */\n\
 	clients-per-query 10;\n\
 	dnssec-accept-expired no;\n\
 	dnssec-validation " VALIDATION_DEFAULT "; \n"

bin/named/server.c

@@ -1804,7 +1804,6 @@ cache_reusable(dns_view_t *originview, dns_view_t *view,
 static bool
 cache_sharable(dns_view_t *originview, dns_view_t *view,
 	       bool new_zero_no_soattl,
-	       unsigned int new_cleaning_interval,
 	       uint64_t new_max_cache_size,
 	       uint32_t new_stale_ttl)
 {
@@ -1819,9 +1818,7 @@ cache_sharable(dns_view_t *originview, dns_view_t *view,
 	 * Check other cache related parameters that must be consistent among
 	 * the sharing views.
 	 */
-	if (dns_cache_getcleaninginterval(originview->cache) !=
+	if (dns_cache_getservestalettl(originview->cache) != new_stale_ttl ||
-	    new_cleaning_interval ||
-	    dns_cache_getservestalettl(originview->cache) != new_stale_ttl ||
 	    dns_cache_getcachesize(originview->cache) != new_max_cache_size) {
 		return (false);
 	}
@@ -3800,7 +3797,6 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist,
 	in_port_t port;
 	dns_cache_t *cache = NULL;
 	isc_result_t result;
-	unsigned int cleaning_interval;
 	size_t max_cache_size;
 	uint32_t max_cache_size_percent = 0;
 	size_t max_adb_size;
@@ -4014,11 +4010,6 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist,
 	 * Obtain configuration parameters that affect the decision of whether
 	 * we can reuse/share an existing cache.
 	 */
-	obj = NULL;
-	result = named_config_get(maps, "cleaning-interval", &obj);
-	INSIST(result == ISC_R_SUCCESS);
-	cleaning_interval = cfg_obj_asuint32(obj) * 60;
-
 	obj = NULL;
 	result = named_config_get(maps, "max-cache-size", &obj);
 	INSIST(result == ISC_R_SUCCESS);
@@ -4306,8 +4297,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist,
 	nsc = cachelist_find(cachelist, cachename, view->rdclass);
 	if (nsc != NULL) {
 		if (!cache_sharable(nsc->primaryview, view, zero_no_soattl,
-				    cleaning_interval, max_cache_size,
+				    max_cache_size, max_stale_ttl))
-				    max_stale_ttl))
 		{
 			isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
 				      NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR,
@@ -4405,7 +4395,6 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist,
 			CHECK(dns_cache_load(cache));
 	}
 
-	dns_cache_setcleaninginterval(cache, cleaning_interval);
 	dns_cache_setcachesize(cache, max_cache_size);
 	dns_cache_setservestalettl(cache, max_stale_ttl);
 

bin/tests/named.conf

@@ -174,7 +174,6 @@ options {
 	files 230;
 	max-cache-size 1m;
 	stacksize 231;
-	cleaning-interval 1000;
 	heartbeat-interval 1001;
 	interface-interval 1002;
 	statistics-interval 1003;
@@ -285,7 +284,6 @@ view "test-view" in {
 	query-source-v6 address 6:6:6:6:6:6:6:6 port *;
 	max-transfer-time-out 45;
 	max-transfer-idle-out 55;
-	cleaning-interval 100;
 	min-roots 3;
 	lame-ttl 477;
 	max-ncache-ttl 333;

doc/arm/Bv9ARM-book.xml

@@ -4275,7 +4275,6 @@ badresp:1,adberr:0,findfail:0,valfail:0]
 		  configurable options be consistent among these
 		  views:
 		  <command>check-names</command>,
-		  <command>cleaning-interval</command>,
 		  <command>dnssec-accept-expired</command>,
 		  <command>dnssec-validation</command>,
 		  <command>max-cache-ttl</command>,
@@ -8403,14 +8402,7 @@ avoid-v6-udp-ports { 40000; range 50000 60000; };
 	      <term><command>cleaning-interval</command></term>
 	      <listitem>
 		<para>
-		  This interval is effectively obsolete.  Previously,
+		  This option is obsolete.
-		  the server would remove expired resource records
-		  from the cache every <command>cleaning-interval</command> minutes.
-		  <acronym>BIND</acronym> 9 now manages cache
-		  memory in a more sophisticated manner and does not
-		  rely on the periodic cleaning any more.
-		  Specifying this option therefore has no effect on
-		  the server's behavior.
 		</para>
 	      </listitem>
 	    </varlistentry>

doc/arm/notes.xml

@@ -125,6 +125,12 @@
 	  if signatures and other DNSSEC data are present. [GL #866]
 	</para>
       </listitem>
+      <listitem>
+	<para>
+	  The <command>cleaning-interval</command> option has been
+	  removed.  [GL !1731]
+	</para>
+      </listitem>
     </itemizedlist>
   </section>
 

lib/bind9/check.c

@@ -864,7 +864,6 @@ check_options(const cfg_obj_t *options, isc_log_t *logctx, isc_mem_t *mctx,
 	 * (scale * value) <= UINT32_MAX
 	 */
 	static intervaltable intervals[] = {
-		{ "cleaning-interval", 60, 28 * 24 * 60 },	/* 28 days */
 		{ "heartbeat-interval", 60, 28 * 24 * 60 },	/* 28 days */
 		{ "interface-interval", 60, 28 * 24 * 60 },	/* 28 days */
 		{ "max-transfer-idle-in", 60, 28 * 24 * 60 },	/* 28 days */

lib/dns/cache.c

@@ -99,9 +99,6 @@ struct cache_cleaner {
 
 	dns_cache_t	*cache;
 	isc_task_t	*task;
-	unsigned int	cleaning_interval; /*% The cleaning-interval from
-					      named.conf, in seconds. */
-	isc_timer_t	*cleaning_timer;
 	isc_event_t	*resched_event;	/*% Sent by cleaner task to
 					   itself to reschedule */
 	isc_event_t	*overmem_event;
@@ -153,9 +150,6 @@ static isc_result_t
 cache_cleaner_init(dns_cache_t *cache, isc_taskmgr_t *taskmgr,
 		   isc_timermgr_t *timermgr, cache_cleaner_t *cleaner);
 
-static void
-cleaning_timer_action(isc_task_t *task, isc_event_t *event);
-
 static void
 incremental_cleaning_action(isc_task_t *task, isc_event_t *event);
 
@@ -521,56 +515,6 @@ dns_cache_dump(dns_cache_t *cache) {
 
 }
 
-void
-dns_cache_setcleaninginterval(dns_cache_t *cache, unsigned int t) {
-	isc_interval_t interval;
-	isc_result_t result;
-
-	LOCK(&cache->lock);
-
-	/*
-	 * It may be the case that the cache has already shut down.
-	 * If so, it has no timer.
-	 */
-	if (cache->cleaner.cleaning_timer == NULL)
-		goto unlock;
-
-	cache->cleaner.cleaning_interval = t;
-
-	if (t == 0) {
-		result = isc_timer_reset(cache->cleaner.cleaning_timer,
-					 isc_timertype_inactive,
-					 NULL, NULL, true);
-	} else {
-		isc_interval_set(&interval, cache->cleaner.cleaning_interval,
-				 0);
-		result = isc_timer_reset(cache->cleaner.cleaning_timer,
-					 isc_timertype_ticker,
-					 NULL, &interval, false);
-	}
-	if (result != ISC_R_SUCCESS)
-		isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE,
-			      DNS_LOGMODULE_CACHE, ISC_LOG_WARNING,
-			      "could not set cache cleaning interval: %s",
-			      isc_result_totext(result));
-
- unlock:
-	UNLOCK(&cache->lock);
-}
-
-unsigned int
-dns_cache_getcleaninginterval(dns_cache_t *cache) {
-	unsigned int t;
-
-	REQUIRE(VALID_CACHE(cache));
-
-	LOCK(&cache->lock);
-	t = cache->cleaner.cleaning_interval;
-	UNLOCK(&cache->lock);
-
-	return (t);
-}
-
 const char *
 dns_cache_getname(dns_cache_t *cache) {
 	REQUIRE(VALID_CACHE(cache));
@@ -599,10 +543,8 @@ cache_cleaner_init(dns_cache_t *cache, isc_taskmgr_t *taskmgr,
 	cleaner->replaceiterator = false;
 
 	cleaner->task = NULL;
-	cleaner->cleaning_timer = NULL;
 	cleaner->resched_event = NULL;
 	cleaner->overmem_event = NULL;
-	cleaner->cleaning_interval = 0; /* Initially turned off. */
 
 	result = dns_db_createiterator(cleaner->cache->db, false,
 				       &cleaner->iterator);
@@ -631,18 +573,6 @@ cache_cleaner_init(dns_cache_t *cache, isc_taskmgr_t *taskmgr,
 			goto cleanup;
 		}
 
-		result = isc_timer_create(timermgr, isc_timertype_inactive,
-					   NULL, NULL, cleaner->task,
-					   cleaning_timer_action, cleaner,
-					   &cleaner->cleaning_timer);
-		if (result != ISC_R_SUCCESS) {
-			UNEXPECTED_ERROR(__FILE__, __LINE__,
-					 "isc_timer_create() failed: %s",
-					 dns_result_totext(result));
-			result = ISC_R_UNEXPECTED;
-			goto cleanup;
-		}
-
 		cleaner->resched_event =
 			isc_event_allocate(cache->mctx, cleaner,
 					   DNS_EVENT_CACHECLEAN,
@@ -671,8 +601,6 @@ cache_cleaner_init(dns_cache_t *cache, isc_taskmgr_t *taskmgr,
 		isc_event_free(&cleaner->overmem_event);
 	if (cleaner->resched_event != NULL)
 		isc_event_free(&cleaner->resched_event);
-	if (cleaner->cleaning_timer != NULL)
-		isc_timer_detach(&cleaner->cleaning_timer);
 	if (cleaner->task != NULL)
 		isc_task_detach(&cleaner->task);
 	if (cleaner->iterator != NULL)
@@ -748,9 +676,6 @@ end_cleaning(cache_cleaner_t *cleaner, isc_event_t *event) {
 	if (result != ISC_R_SUCCESS)
 		dns_dbiterator_destroy(&cleaner->iterator);
 
-	dns_cache_setcleaninginterval(cleaner->cache,
-				      cleaner->cleaning_interval);
-
 	isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_CACHE,
 		      ISC_LOG_DEBUG(1), "end cache cleaning, mem inuse %lu",
 		      (unsigned long)isc_mem_inuse(cleaner->cache->mctx));
@@ -759,28 +684,6 @@ end_cleaning(cache_cleaner_t *cleaner, isc_event_t *event) {
 	cleaner->resched_event = event;
 }
 
-/*
- * This is run once for every cache-cleaning-interval as defined in named.conf.
- */
-static void
-cleaning_timer_action(isc_task_t *task, isc_event_t *event) {
-	cache_cleaner_t *cleaner = event->ev_arg;
-
-	UNUSED(task);
-
-	INSIST(task == cleaner->task);
-	INSIST(event->ev_type == ISC_TIMEREVENT_TICK);
-
-	isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_CACHE,
-		      ISC_LOG_DEBUG(1), "cache cleaning timer fired, "
-		      "cleaner state = %d", cleaner->state);
-
-	if (cleaner->state == cleaner_s_idle)
-		begin_cleaning(cleaner);
-
-	isc_event_free(&event);
-}
-
 /*
  * This is called when the cache either surpasses its upper limit
  * or shrinks beyond its lower limit.
@@ -1123,14 +1026,6 @@ cleaner_shutdown_action(isc_task_t *task, isc_event_t *event) {
 	if (cache->references == 0)
 		should_free = true;
 
-	/*
-	 * By detaching the timer in the context of its task,
-	 * we are guaranteed that there will be no further timer
-	 * events.
-	 */
-	if (cache->cleaner.cleaning_timer != NULL)
-		isc_timer_detach(&cache->cleaner.cleaning_timer);
-
 	/* Make sure we don't reschedule anymore. */
 	(void)isc_task_purge(task, NULL, DNS_EVENT_CACHECLEAN, NULL);
 

lib/dns/include/dns/cache.h

@@ -219,18 +219,6 @@ dns_cache_clean(dns_cache_t *cache, isc_stdtime_t now);
  * references.
  */
 
-void
-dns_cache_setcleaninginterval(dns_cache_t *cache, unsigned int interval);
-/*%<
- * Set the periodic cache cleaning interval to 'interval' seconds.
- */
-
-unsigned int
-dns_cache_getcleaninginterval(dns_cache_t *cache);
-/*%<
- * Get the periodic cache cleaning interval to 'interval' seconds.
- */
-
 const char *
 dns_cache_getname(dns_cache_t *cache);
 /*%<

lib/dns/win32/libdns.def.in

@@ -82,7 +82,6 @@ dns_cache_flush
 dns_cache_flushname
 dns_cache_flushnode
 dns_cache_getcachesize
-dns_cache_getcleaninginterval
 dns_cache_getname
 dns_cache_getservestalettl
 dns_cache_getstats
@@ -94,7 +93,6 @@ dns_cache_renderjson
 dns_cache_renderxml
 @END LIBXML2
 dns_cache_setcachesize
-dns_cache_setcleaninginterval
 dns_cache_setfilename
 dns_cache_setservestalettl
 dns_cache_updatestats

lib/isccfg/namedconf.c

@@ -1846,7 +1846,7 @@ view_clauses[] = {
 	{ "cache-file", &cfg_type_qstring, 0 },
 	{ "catalog-zones", &cfg_type_catz, 0 },
 	{ "check-names", &cfg_type_checknames, CFG_CLAUSEFLAG_MULTI },
-	{ "cleaning-interval", &cfg_type_uint32, 0 },
+	{ "cleaning-interval", &cfg_type_uint32, CFG_CLAUSEFLAG_OBSOLETE },
 	{ "clients-per-query", &cfg_type_uint32, 0 },
 	{ "deny-answer-addresses", &cfg_type_denyaddresses, 0 },
 	{ "deny-answer-aliases", &cfg_type_denyaliases, 0 },