From 0cdf85d20491eca328ed43f8c3f70513735757ba Mon Sep 17 00:00:00 2001 From: Kevin Chen Date: Thu, 11 Mar 2021 15:45:44 +0100 Subject: [PATCH 1/2] Several serve-stale improvements Commit a83c8cb0afd88d54b9cf67239f2495c9b0391e97 updated masterdump so that stale records in "rndc dumpdb" output no longer shows 0 TTLs. In this commit we change the name of the `rdataset->stale_ttl` field to `rdataset->expired` to make its purpose clearer, and set it to zero in cases where it's unused. Add 'rbtdb->serve_stale_ttl' to various checks so that stale records are not purged from the cache when they've been stale for RBTDB_VIRTUAL (300) seconds. Increment 'ns_statscounter_usedstale' when a stale answer is used. Note: There was a question of whether 'overmem_purge' should be purging ancient records, instead of stale ones. It is left as purging stale records, since stale records could take up the majority of the cache. This submission is copyrighted Akamai Technologies, Inc. and provided under an MPL 2.0 license. This commit was originally authored by Kevin Chen, and was updated by Matthijs Mekking to match recent serve-stale developments. --- lib/dns/include/dns/rdataset.h | 15 +++++++++------ lib/dns/masterdump.c | 4 ++-- lib/dns/rbtdb.c | 21 +++++++++++++-------- lib/ns/query.c | 1 + 4 files changed, 25 insertions(+), 16 deletions(-) diff --git a/lib/dns/include/dns/rdataset.h b/lib/dns/include/dns/rdataset.h index f12419bcac..906edd0aa3 100644 --- a/lib/dns/include/dns/rdataset.h +++ b/lib/dns/include/dns/rdataset.h @@ -95,7 +95,7 @@ typedef struct dns_rdatasetmethods { * rdataset implementations may change any of the fields. */ struct dns_rdataset { - unsigned int magic; /* XXX ? */ + unsigned int magic; dns_rdatasetmethods_t *methods; ISC_LINK(dns_rdataset_t) link; @@ -107,11 +107,7 @@ struct dns_rdataset { dns_rdataclass_t rdclass; dns_rdatatype_t type; dns_ttl_t ttl; - /* - * Stale ttl is used to see how long this RRset can still be used - * to serve to clients, after the TTL has expired. - */ - dns_ttl_t stale_ttl; + dns_trust_t trust; dns_rdatatype_t covers; @@ -134,6 +130,13 @@ struct dns_rdataset { */ isc_stdtime_t resign; + /* + * When a cache rdataset's TTL has expired but it hasn't been + * cleaned up yet, it will have this value set so that the time + * it expired can be printed by dns_master_dump*(). + */ + isc_stdtime_t expired; + /*@{*/ /*% * These are for use by the rdataset implementation, and MUST NOT diff --git a/lib/dns/masterdump.c b/lib/dns/masterdump.c index 75cd0d2509..74ec7972ba 100644 --- a/lib/dns/masterdump.c +++ b/lib/dns/masterdump.c @@ -1109,12 +1109,12 @@ again: isc_result_t result; if (STALE(rds)) { fprintf(f, "; stale\n"); - } else if (ANCIENT(rds)) { + } else if (ANCIENT(rds) && rds->expired != 0) { isc_buffer_t b; char buf[sizeof("YYYYMMDDHHMMSS")]; memset(buf, 0, sizeof(buf)); isc_buffer_init(&b, buf, sizeof(buf) - 1); - dns_time64_totext((uint64_t)rds->stale_ttl, &b); + dns_time64_totext((uint64_t)rds->expired, &b); fprintf(f, "; expired since %s " "(awaiting cleanup)\n", diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index 98f88e29de..f1074d3fd4 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -3135,7 +3135,9 @@ bind_rdataset(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node, rdatasetheader_t *header, rdataset->type = RBTDB_RDATATYPE_BASE(header->type); rdataset->covers = RBTDB_RDATATYPE_EXT(header->type); rdataset->ttl = header->rdh_ttl - now; + rdataset->expired = 0; rdataset->trust = header->trust; + if (NEGATIVE(header)) { rdataset->attributes |= DNS_RDATASETATTR_NEGATIVE; } @@ -3148,22 +3150,21 @@ bind_rdataset(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node, rdatasetheader_t *header, if (PREFETCH(header)) { rdataset->attributes |= DNS_RDATASETATTR_PREFETCH; } + if (stale && !ancient) { dns_ttl_t stale_ttl = header->rdh_ttl + rbtdb->serve_stale_ttl; if (stale_ttl > now) { - stale_ttl = stale_ttl - now; + rdataset->ttl = stale_ttl - now; } else { - stale_ttl = 0; + rdataset->ttl = 0; } if (STALE_WINDOW(header)) { rdataset->attributes |= DNS_RDATASETATTR_STALE_WINDOW; } rdataset->attributes |= DNS_RDATASETATTR_STALE; - rdataset->stale_ttl = stale_ttl; - rdataset->ttl = stale_ttl; } else if (IS_CACHE(rbtdb) && !ACTIVE(header, now)) { rdataset->attributes |= DNS_RDATASETATTR_ANCIENT; - rdataset->stale_ttl = header->rdh_ttl; + rdataset->expired = header->rdh_ttl; rdataset->ttl = 0; } @@ -5589,7 +5590,8 @@ expirenode(dns_db_t *db, dns_dbnode_t *node, isc_stdtime_t now) { isc_rwlocktype_write); for (header = rbtnode->data; header != NULL; header = header->next) { - if (header->rdh_ttl <= now - RBTDB_VIRTUAL) { + if (header->rdh_ttl + rbtdb->serve_stale_ttl <= + now - RBTDB_VIRTUAL) { /* * We don't check if refcurrent(rbtnode) == 0 and try * to free like we do in cache_find(), because @@ -5860,7 +5862,8 @@ cache_findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, for (header = rbtnode->data; header != NULL; header = header_next) { header_next = header->next; if (!ACTIVE(header, now)) { - if ((header->rdh_ttl < now - RBTDB_VIRTUAL) && + if ((header->rdh_ttl + rbtdb->serve_stale_ttl < + now - RBTDB_VIRTUAL) && (locktype == isc_rwlocktype_write || NODE_TRYUPGRADE(lock) == ISC_R_SUCCESS)) { @@ -6957,7 +6960,9 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, } header = isc_heap_element(rbtdb->heaps[rbtnode->locknum], 1); - if (header && header->rdh_ttl < now - RBTDB_VIRTUAL) { + if (header != NULL && header->rdh_ttl + rbtdb->serve_stale_ttl < + now - RBTDB_VIRTUAL) + { expire_header(rbtdb, header, tree_locked, expire_ttl); } diff --git a/lib/ns/query.c b/lib/ns/query.c index 97c3be1dd0..749c748704 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -5875,6 +5875,7 @@ query_lookup(query_ctx_t *qctx) { { qctx->rdataset->ttl = qctx->view->staleanswerttl; stale_found = true; + inc_stats(qctx->client, ns_statscounter_usedstale); } else { stale_found = false; } From f7f543d99bcb7008808e0c55e35ccd6788cb32a7 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 27 May 2021 09:43:21 +0200 Subject: [PATCH 2/2] Reuse rdatset->ttl when dumping ancient RRsets Rather than having an expensive 'expired' (fka 'stale_ttl') in the rdataset structure, that is only used to be printed in a comment on ancient RRsets, reuse the TTL field of the RRset. --- lib/dns/include/dns/rdataset.h | 7 ------- lib/dns/masterdump.c | 4 ++-- lib/dns/rbtdb.c | 4 +--- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/lib/dns/include/dns/rdataset.h b/lib/dns/include/dns/rdataset.h index 906edd0aa3..2569e40d23 100644 --- a/lib/dns/include/dns/rdataset.h +++ b/lib/dns/include/dns/rdataset.h @@ -130,13 +130,6 @@ struct dns_rdataset { */ isc_stdtime_t resign; - /* - * When a cache rdataset's TTL has expired but it hasn't been - * cleaned up yet, it will have this value set so that the time - * it expired can be printed by dns_master_dump*(). - */ - isc_stdtime_t expired; - /*@{*/ /*% * These are for use by the rdataset implementation, and MUST NOT diff --git a/lib/dns/masterdump.c b/lib/dns/masterdump.c index 74ec7972ba..c66eb7cceb 100644 --- a/lib/dns/masterdump.c +++ b/lib/dns/masterdump.c @@ -1109,12 +1109,12 @@ again: isc_result_t result; if (STALE(rds)) { fprintf(f, "; stale\n"); - } else if (ANCIENT(rds) && rds->expired != 0) { + } else if (ANCIENT(rds)) { isc_buffer_t b; char buf[sizeof("YYYYMMDDHHMMSS")]; memset(buf, 0, sizeof(buf)); isc_buffer_init(&b, buf, sizeof(buf) - 1); - dns_time64_totext((uint64_t)rds->expired, &b); + dns_time64_totext((uint64_t)rds->ttl, &b); fprintf(f, "; expired since %s " "(awaiting cleanup)\n", diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index f1074d3fd4..4d37a36ba3 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -3135,7 +3135,6 @@ bind_rdataset(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node, rdatasetheader_t *header, rdataset->type = RBTDB_RDATATYPE_BASE(header->type); rdataset->covers = RBTDB_RDATATYPE_EXT(header->type); rdataset->ttl = header->rdh_ttl - now; - rdataset->expired = 0; rdataset->trust = header->trust; if (NEGATIVE(header)) { @@ -3164,8 +3163,7 @@ bind_rdataset(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node, rdatasetheader_t *header, rdataset->attributes |= DNS_RDATASETATTR_STALE; } else if (IS_CACHE(rbtdb) && !ACTIVE(header, now)) { rdataset->attributes |= DNS_RDATASETATTR_ANCIENT; - rdataset->expired = header->rdh_ttl; - rdataset->ttl = 0; + rdataset->ttl = header->rdh_ttl; } rdataset->private1 = rbtdb;