diff --git a/CHANGES b/CHANGES index a7acefc77d..51eb7c80e8 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,10 @@ +5618. [bug] When introducing change 5149, "rndc dumpdb" started + to print a line above a stale RRset, indicating how + long the data will be retained. Also, TTLs were + increased with 'max-stale-ttl'. This could lead to + nonsensical values and both issues have been fixed. + [GL #389] [GL #2289] + 5617. [placeholder] 5616. [placeholder] diff --git a/bin/tests/system/serve-stale/tests.sh b/bin/tests/system/serve-stale/tests.sh index 2205962c7d..4a83e35ac6 100755 --- a/bin/tests/system/serve-stale/tests.sh +++ b/bin/tests/system/serve-stale/tests.sh @@ -105,11 +105,25 @@ status=$((status+ret)) sleep 2 +# Run rndc dumpdb, test whether the stale data has correct comment printed. +# The max-stale-ttl is 3600 seconds, so the comment should say the data is +# stale for somewhere between 3500-3599 seconds. +echo_i "check rndc dump stale data.example ($n)" +rndc_dumpdb ns1 || ret=1 +awk '/; stale/ { x=$0; getline; print x, $0}' ns1/named_dump.db.test$n | + grep "; stale data\.example.*3[56]...*TXT.*A text record with a 2 second ttl" > /dev/null 2>&1 || ret=1 +# Also make sure the not expired data does not have a stale comment. +awk '/; answer/ { x=$0; getline; print x, $0}' ns1/named_dump.db.test$n | + grep "; answer longttl\.example.*[56]...*TXT.*A text record with a 600 second ttl" > /dev/null 2>&1 || ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + echo_i "sending queries for tests $((n+1))-$((n+4))..." $DIG -p ${PORT} @10.53.0.1 data.example TXT > dig.out.test$((n+1)) & -$DIG -p ${PORT} @10.53.0.1 othertype.example CAA > dig.out.test$((n+2)) & -$DIG -p ${PORT} @10.53.0.1 nodata.example TXT > dig.out.test$((n+3)) & -$DIG -p ${PORT} @10.53.0.1 nxdomain.example TXT > dig.out.test$((n+4)) +$DIG -p ${PORT} @10.53.0.1 longttl.example TXT > dig.out.test$((n+2)) & +$DIG -p ${PORT} @10.53.0.1 othertype.example CAA > dig.out.test$((n+3)) & +$DIG -p ${PORT} @10.53.0.1 nodata.example TXT > dig.out.test$((n+4)) & +$DIG -p ${PORT} @10.53.0.1 nxdomain.example TXT > dig.out.test$((n+5)) wait @@ -122,16 +136,12 @@ grep "data\.example\..*4.*IN.*TXT.*A text record with a 2 second ttl" dig.out.te if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status+ret)) -# Run rndc dumpdb, test whether the stale data has correct comment printed. -# The max-stale-ttl is 3600 seconds, so the comment should say the data is -# stale for somewhere between 3500-3599 seconds. -echo_i "check rndc dump stale data.example ($n)" -rndc_dumpdb ns1 || ret=1 -awk '/; stale/ { x=$0; getline; print x, $0}' ns1/named_dump.db.test$n | - grep "; stale (will be retained for 35.. more seconds) data\.example.*A text record with a 2 second ttl" > /dev/null 2>&1 || ret=1 -# Also make sure the not expired data does not have a stale comment. -awk '/; answer/ { x=$0; getline; print x, $0}' ns1/named_dump.db.test$n | - grep "; answer longttl\.example.*A text record with a 600 second ttl" > /dev/null 2>&1 || ret=1 +n=$((n+1)) +echo_i "check non-stale longttl.example ($n)" +ret=0 +grep "status: NOERROR" dig.out.test$n > /dev/null || ret=1 +grep "ANSWER: 1," dig.out.test$n > /dev/null || ret=1 +grep "longttl\.example\..*59[0-9].*IN.*TXT.*A text record with a 600 second ttl" dig.out.test$n > /dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status+ret)) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index cd3af2cb7b..4098e8ea0a 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -80,3 +80,9 @@ Bug Fixes state. (Other zone journal files were not affected.) This has been fixed. If a corrupt journal file is detected, ``named`` can now recover from it. [GL #2600] + +- When dumping the cache to file, TTLs were being increased with + ``max-stale-ttl``. Also the comment above stale RRsets could have nonsensical + values if the RRset was still marked a stale but the ``max-stale-ttl`` has + passed (and is actually an RRset awaiting cleanup). Both issues have now + been fixed. [GL #389] [GL #2289] diff --git a/lib/dns/masterdump.c b/lib/dns/masterdump.c index edd4fcaaa0..c8dcce5aa2 100644 --- a/lib/dns/masterdump.c +++ b/lib/dns/masterdump.c @@ -1108,11 +1108,7 @@ again: } else { isc_result_t result; if (STALE(rds)) { - fprintf(f, - "; stale (will be retained for %u more " - "seconds)\n", - (rds->stale_ttl - - ctx->serve_stale_ttl)); + fprintf(f, "; stale\n"); } else if (ANCIENT(rds)) { isc_buffer_t b; char buf[sizeof("YYYYMMDDHHMMSS")]; @@ -1591,14 +1587,8 @@ dumpctx_create(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, dctx->do_date = dns_db_iscache(dctx->db); if (dctx->do_date) { - /* - * Adjust the date backwards by the serve-stale TTL, if any. - * This is so the TTL will be loaded correctly when next - * started. - */ (void)dns_db_getservestalettl(dctx->db, &dctx->tctx.serve_stale_ttl); - dctx->now -= dctx->tctx.serve_stale_ttl; } if (dctx->format == dns_masterformat_text && diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index a18fcceaa5..f9c93416ed 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -3088,6 +3088,8 @@ bind_rdataset(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node, rdatasetheader_t *header, isc_stdtime_t now, isc_rwlocktype_t locktype, dns_rdataset_t *rdataset) { unsigned char *raw; /* RDATASLAB */ + bool stale = STALE(header); + bool ancient = ANCIENT(header); /* * Caller must be holding the node reader lock. @@ -3105,6 +3107,29 @@ bind_rdataset(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node, rdatasetheader_t *header, INSIST(rdataset->methods == NULL); /* We must be disassociated. */ + /* + * Mark header stale or ancient if the RRset is no longer active. + */ + if (!ACTIVE(header, now)) { + dns_ttl_t stale_ttl = header->rdh_ttl + rbtdb->serve_stale_ttl; + /* + * If this data is in the stale window keep it and if + * DNS_DBFIND_STALEOK is not set we tell the caller to + * skip this record. We skip the records with ZEROTTL + * (these records should not be cached anyway). + */ + + if (KEEPSTALE(rbtdb) && stale_ttl > now) { + stale = true; + } else { + /* + * We are not keeping stale, or it is outside the + * stale window. Mark ancient, i.e. ready for cleanup. + */ + ancient = true; + } + } + rdataset->methods = &rdataset_methods; rdataset->rdclass = rbtdb->common.rdclass; rdataset->type = RBTDB_RDATATYPE_BASE(header->type); @@ -3123,14 +3148,19 @@ bind_rdataset(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node, rdatasetheader_t *header, if (PREFETCH(header)) { rdataset->attributes |= DNS_RDATASETATTR_PREFETCH; } - if (STALE(header)) { + if (stale && !ancient) { + dns_ttl_t stale_ttl = header->rdh_ttl + rbtdb->serve_stale_ttl; + if (stale_ttl > now) { + stale_ttl = stale_ttl - now; + } else { + stale_ttl = 0; + } if (STALE_WINDOW(header)) { rdataset->attributes |= DNS_RDATASETATTR_STALE_WINDOW; } rdataset->attributes |= DNS_RDATASETATTR_STALE; - rdataset->stale_ttl = - (rbtdb->serve_stale_ttl + header->rdh_ttl) - now; - rdataset->ttl = 0; + 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; diff --git a/lib/dns/tests/db_test.c b/lib/dns/tests/db_test.c index c12e90ee1c..e41a63c0cf 100644 --- a/lib/dns/tests/db_test.c +++ b/lib/dns/tests/db_test.c @@ -248,7 +248,6 @@ dns_dbfind_staleok_test(void **state) { count++; assert_in_range(count, 0, 49); /* loop sanity */ assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(rdataset.ttl, 0); assert_int_equal(rdataset.attributes & DNS_RDATASETATTR_STALE, DNS_RDATASETATTR_STALE);