No longer have stale tracking in stats module

Having the decrement/increment logic in stats makes the code hard
to follow. Remove it here and adjust the unit test. The caller
will be responsible for maintaining the correct increments and
decrements for statistics counters (in the following commit).

(cherry picked from commit 48332d4478c6283a108772841a26c527c6c0e001)

diff --git a/lib/dns/include/dns/stats.h b/lib/dns/include/dns/stats.h
index a4ee695db899601e41ea2fdc52201aadabd316be..fff230a500f7fc4ddd93e9e5c6c078ebce15de03 100644 (file)
--- a/lib/dns/include/dns/stats.h
+++ b/lib/dns/include/dns/stats.h
@@ -477,15 +477,9 @@ LIBDNS_EXTERNAL_DATA extern const char *dns_statscounter_names[];
  *     RRset type counters only.  This indicates a record that is stale
  *     but may still be served.
  *
- *     Note: incrementing _STALE will decrement the corresponding non-stale
- *     counter.
- *
  * _ANCIENT
  *     RRset type counters only.  This indicates a record that is marked for
  *     removal.
- *
- *     Note: incrementing _ANCIENT will decrement the corresponding
- *     non-ancient counter.
  */
 #define DNS_RDATASTATSTYPE_ATTR_OTHERTYPE      0x0001
 #define DNS_RDATASTATSTYPE_ATTR_NXRRSET                0x0002

diff --git a/lib/dns/stats.c b/lib/dns/stats.c
index 615c72f2f23bb39e264e01e8bbb7c267f6abde4c..1c1b140ab44297c6d89ad08b669264e502611ffd 100644 (file)
--- a/lib/dns/stats.c
+++ b/lib/dns/stats.c
@@ -49,19 +49,32 @@ typedef enum {
  * Ideally, we should have rdata handle this type of details.
  */
 /*
- * types, !types, nxdomain, stale types, stale !types, stale nxdomain
+ * types, !types, nxdomain, stale types, stale !types, stale nxdomain,
+ * ancient types, ancient !types, ancient nxdomain
  */
 enum {
        /* For 0-255, we use the rdtype value as counter indices */
        rdtypecounter_dlv = 256,        /* for dns_rdatatype_dlv */
        rdtypecounter_others = 257,     /* anything else */
        rdtypecounter_max = 258,
-       /* The following are used for rdataset */
+       /* The following are used for nxrrset rdataset */
        rdtypenxcounter_max = rdtypecounter_max * 2,
+       /* nxdomain counter */
        rdtypecounter_nxdomain = rdtypenxcounter_max,
        /* stale counters offset */
        rdtypecounter_stale = rdtypecounter_nxdomain + 1,
-       rdatasettypecounter_max = rdtypecounter_stale * 2,
+       rdtypecounter_stale_max = rdtypecounter_stale + rdtypecounter_max,
+       rdtypenxcounter_stale_max = rdtypecounter_stale_max + rdtypecounter_max,
+       rdtypecounter_stale_nxdomain = rdtypenxcounter_stale_max,
+       /* ancient counters offset */
+       rdtypecounter_ancient = rdtypecounter_stale_nxdomain + 1,
+       rdtypecounter_ancient_max = rdtypecounter_ancient + rdtypecounter_max,
+       rdtypenxcounter_ancient_max = rdtypecounter_ancient_max +
+                                     rdtypecounter_max,
+       rdtypecounter_ancient_nxdomain = rdtypenxcounter_ancient_max,
+       /* limit of number counter types */
+       rdatasettypecounter_max = rdtypecounter_ancient_nxdomain + 1,
+       /* dnssec maximum key id */
        dnssec_keyid_max = 65535
 };
 
@@ -262,17 +275,17 @@ update_rdatasetstats(dns_stats_t *stats, dns_rdatastatstype_t rrsettype,
                        counter += rdtypecounter_max;
        }
 
+       if ((DNS_RDATASTATSTYPE_ATTR(rrsettype) &
+            DNS_RDATASTATSTYPE_ATTR_ANCIENT) != 0) {
+               counter += rdtypecounter_ancient;
+       } else if ((DNS_RDATASTATSTYPE_ATTR(rrsettype) &
+            DNS_RDATASTATSTYPE_ATTR_STALE) != 0) {
+               counter += rdtypecounter_stale;
+       }
+
        if (increment) {
-               if ((DNS_RDATASTATSTYPE_ATTR(rrsettype) &
-                    DNS_RDATASTATSTYPE_ATTR_STALE) != 0) {
-                       isc_stats_decrement(stats->counters, counter);
-                       counter += rdtypecounter_stale;
-               }
                isc_stats_increment(stats->counters, counter);
        } else {
-               if ((DNS_RDATASTATSTYPE_ATTR(rrsettype) &
-                    DNS_RDATASTATSTYPE_ATTR_STALE) != 0)
-                       counter += rdtypecounter_stale;
                isc_stats_decrement(stats->counters, counter);
        }
 }
@@ -373,34 +386,46 @@ static void
 rdataset_dumpcb(isc_statscounter_t counter, uint64_t value, void *arg) {
        rdatadumparg_t *rdatadumparg = arg;
        unsigned int attributes;
+       bool dump = true;
 
        if (counter < rdtypecounter_max) {
-               dump_rdentry(counter, value, 0, rdatadumparg->fn,
-                            rdatadumparg->arg);
-       } else if (counter < rdtypecounter_nxdomain) {
+               attributes = 0;
+       } else if (counter < rdtypenxcounter_max) {
                counter -= rdtypecounter_max;
                attributes = DNS_RDATASTATSTYPE_ATTR_NXRRSET;
-               dump_rdentry(counter, value, attributes, rdatadumparg->fn,
-                            rdatadumparg->arg);
        } else if (counter == rdtypecounter_nxdomain) {
-               dump_rdentry(0, value, DNS_RDATASTATSTYPE_ATTR_NXDOMAIN,
-                            rdatadumparg->fn, rdatadumparg->arg);
-       } else if (counter < rdtypecounter_stale + rdtypecounter_max) {
+               counter = 0;
+               attributes = DNS_RDATASTATSTYPE_ATTR_NXDOMAIN;
+       } else if (counter < rdtypecounter_stale_max) {
                counter -= rdtypecounter_stale;
                attributes = DNS_RDATASTATSTYPE_ATTR_STALE;
-               dump_rdentry(counter, value, attributes, rdatadumparg->fn,
-                            rdatadumparg->arg);
-       } else if (counter < rdtypecounter_stale + rdtypecounter_nxdomain) {
-               counter -= rdtypecounter_stale + rdtypecounter_max;
+       } else if (counter < rdtypenxcounter_stale_max) {
+               counter -= rdtypecounter_stale_max;
                attributes = DNS_RDATASTATSTYPE_ATTR_NXRRSET |
                             DNS_RDATASTATSTYPE_ATTR_STALE;
-               dump_rdentry(counter, value, attributes, rdatadumparg->fn,
-                            rdatadumparg->arg);
-       } else {
+       } else if (counter == rdtypecounter_stale_nxdomain) {
+               counter = 0;
                attributes = DNS_RDATASTATSTYPE_ATTR_NXDOMAIN |
                             DNS_RDATASTATSTYPE_ATTR_STALE;
-               dump_rdentry(0, value, attributes, rdatadumparg->fn,
-                            rdatadumparg->arg);
+       } else if (counter < rdtypecounter_ancient_max) {
+               counter -= rdtypecounter_ancient;
+               attributes = DNS_RDATASTATSTYPE_ATTR_ANCIENT;
+       } else if (counter < rdtypenxcounter_ancient_max) {
+               counter -= rdtypecounter_ancient_max;
+               attributes = DNS_RDATASTATSTYPE_ATTR_NXRRSET |
+                            DNS_RDATASTATSTYPE_ATTR_ANCIENT;
+       } else if (counter == rdtypecounter_ancient_nxdomain) {
+               counter = 0;
+               attributes = DNS_RDATASTATSTYPE_ATTR_NXDOMAIN |
+                            DNS_RDATASTATSTYPE_ATTR_ANCIENT;
+       } else {
+               /* Out of bounds, do not dump entry. */
+               dump = false;
+       }
+
+       if (dump) {
+               dump_rdentry(counter, value, attributes, rdatadumparg->fn,
+                    rdatadumparg->arg);
        }
 }
 

diff --git a/lib/dns/tests/rdatasetstats_test.c b/lib/dns/tests/rdatasetstats_test.c
index 88433ca26c572556e9f4ae4dfb6fc42d65916382..088cf79101973383e288619abf34e6849c4f8dfe 100644 (file)
--- a/lib/dns/tests/rdatasetstats_test.c
+++ b/lib/dns/tests/rdatasetstats_test.c
@@ -56,41 +56,70 @@ _teardown(void **state) {
 }
 
 static void
-set_typestats(dns_stats_t *stats, dns_rdatatype_t type, bool stale) {
+set_typestats(dns_stats_t *stats, dns_rdatatype_t type) {
        dns_rdatastatstype_t which;
        unsigned int attributes;
 
        attributes = 0;
-       if (stale) {
-               attributes |= DNS_RDATASTATSTYPE_ATTR_STALE;
-       }
        which = DNS_RDATASTATSTYPE_VALUE(type, attributes);
        dns_rdatasetstats_increment(stats, which);
 
        attributes = DNS_RDATASTATSTYPE_ATTR_NXRRSET;
-       if (stale) {
-               attributes |= DNS_RDATASTATSTYPE_ATTR_STALE;
-       }
        which = DNS_RDATASTATSTYPE_VALUE(type, attributes);
        dns_rdatasetstats_increment(stats, which);
 }
 
 static void
-set_nxdomainstats(dns_stats_t *stats, bool stale) {
+set_nxdomainstats(dns_stats_t *stats) {
        dns_rdatastatstype_t which;
        unsigned int attributes;
 
        attributes = DNS_RDATASTATSTYPE_ATTR_NXDOMAIN;
-       if (stale) {
-               attributes |= DNS_RDATASTATSTYPE_ATTR_STALE;
-       }
+       which = DNS_RDATASTATSTYPE_VALUE(0, attributes);
+       dns_rdatasetstats_increment(stats, which);
+}
+
+static void
+mark_stale(dns_stats_t *stats, dns_rdatatype_t type, int from, int to)
+{
+       dns_rdatastatstype_t which;
+       unsigned int attributes;
+
+       attributes = from;
+       which = DNS_RDATASTATSTYPE_VALUE(type, attributes);
+       dns_rdatasetstats_decrement(stats, which);
+
+       attributes |= to;
+       which = DNS_RDATASTATSTYPE_VALUE(type, attributes);
+       dns_rdatasetstats_increment(stats, which);
+
+       attributes = DNS_RDATASTATSTYPE_ATTR_NXRRSET | from;
+       which = DNS_RDATASTATSTYPE_VALUE(type, attributes);
+       dns_rdatasetstats_decrement(stats, which);
+
+       attributes |= to;
+       which = DNS_RDATASTATSTYPE_VALUE(type, attributes);
+       dns_rdatasetstats_increment(stats, which);
+}
+
+static void
+mark_nxdomain_stale(dns_stats_t *stats, int from, int to)
+{
+       dns_rdatastatstype_t which;
+       unsigned int attributes;
+
+       attributes = DNS_RDATASTATSTYPE_ATTR_NXDOMAIN | from;
+       which = DNS_RDATASTATSTYPE_VALUE(0, attributes);
+       dns_rdatasetstats_decrement(stats, which);
+
+       attributes |= to;
        which = DNS_RDATASTATSTYPE_VALUE(0, attributes);
        dns_rdatasetstats_increment(stats, which);
 }
 
 #define ATTRIBUTE_SET(y) ((attributes & (y)) != 0)
 static void
-checkit1(dns_rdatastatstype_t which, uint64_t value, void *arg) {
+verify_active_counters(dns_rdatastatstype_t which, uint64_t value, void *arg) {
        unsigned int attributes;
 #if debug
        unsigned int type;
@@ -103,14 +132,16 @@ checkit1(dns_rdatastatstype_t which, uint64_t value, void *arg) {
 #if debug
        type = DNS_RDATASTATSTYPE_BASE(which);
 
-       fprintf(stderr, "%s%s%s%s/%u, %u\n",
+       fprintf(stderr, "%s%s%s%s%s/%u, %u\n",
                ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_OTHERTYPE) ? "O" : " ",
                ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_NXRRSET) ? "!" : " ",
+               ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_ANCIENT) ? "~" : " ",
                ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_STALE) ? "#" : " ",
                ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_NXDOMAIN) ? "X" : " ",
                type, (unsigned)value);
 #endif
-       if ((attributes & DNS_RDATASTATSTYPE_ATTR_STALE) == 0) {
+       if ((attributes & DNS_RDATASTATSTYPE_ATTR_ANCIENT) == 0 &&
+           (attributes & DNS_RDATASTATSTYPE_ATTR_STALE) == 0) {
                assert_int_equal(value, 1);
        } else {
                assert_int_equal(value, 0);
@@ -118,7 +149,7 @@ checkit1(dns_rdatastatstype_t which, uint64_t value, void *arg) {
 }
 
 static void
-checkit2(dns_rdatastatstype_t which, uint64_t value, void *arg) {
+verify_stale_counters(dns_rdatastatstype_t which, uint64_t value, void *arg) {
        unsigned int attributes;
 #if debug
        unsigned int type;
@@ -131,27 +162,61 @@ checkit2(dns_rdatastatstype_t which, uint64_t value, void *arg) {
 #if debug
        type = DNS_RDATASTATSTYPE_BASE(which);
 
-       fprintf(stderr, "%s%s%s%s/%u, %u\n",
+       fprintf(stderr, "%s%s%s%s%s/%u, %u\n",
                ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_OTHERTYPE) ? "O" : " ",
                ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_NXRRSET) ? "!" : " ",
+               ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_ANCIENT) ? "~" : " ",
                ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_STALE) ? "#" : " ",
                ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_NXDOMAIN) ? "X" : " ",
                type, (unsigned)value);
 #endif
-       if ((attributes & DNS_RDATASTATSTYPE_ATTR_STALE) == 0) {
-               assert_int_equal(value, 0);
+       if ((attributes & DNS_RDATASTATSTYPE_ATTR_STALE) != 0) {
+               assert_int_equal(value, 1);
        } else {
+               assert_int_equal(value, 0);
+       }
+}
+
+static void
+verify_ancient_counters(dns_rdatastatstype_t which, uint64_t value, void *arg) {
+       unsigned int attributes;
+#if debug
+       unsigned int type;
+#endif
+
+       UNUSED(which);
+       UNUSED(arg);
+
+       attributes = DNS_RDATASTATSTYPE_ATTR(which);
+#if debug
+       type = DNS_RDATASTATSTYPE_BASE(which);
+
+       fprintf(stderr, "%s%s%s%s%s/%u, %u\n",
+               ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_OTHERTYPE) ? "O" : " ",
+               ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_NXRRSET) ? "!" : " ",
+               ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_ANCIENT) ? "~" : " ",
+               ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_STALE) ? "#" : " ",
+               ATTRIBUTE_SET(DNS_RDATASTATSTYPE_ATTR_NXDOMAIN) ? "X" : " ",
+               type, (unsigned)value);
+#endif
+       if ((attributes & DNS_RDATASTATSTYPE_ATTR_ANCIENT) != 0) {
                assert_int_equal(value, 1);
+       } else {
+               assert_int_equal(value, 0);
        }
 }
 /*
  * Individual unit tests
  */
 
-/* test that rdatasetstats counters are properly set */
+/*
+ * Test that rdatasetstats counters are properly set when moving from
+ * active -> stale -> ancient.
+ */
 static void
-rdatasetstats(void **state) {
+rdatasetstats(void **state, bool servestale) {
        unsigned int i;
+       unsigned int from = 0;
        dns_stats_t *stats = NULL;
        isc_result_t result;
 
@@ -161,39 +226,82 @@ rdatasetstats(void **state) {
        assert_int_equal(result, ISC_R_SUCCESS);
 
        /* First 256 types. */
-       for (i = 0; i <= 255; i++)
-               set_typestats(stats, (dns_rdatatype_t)i, false);
+       for (i = 0; i <= 255; i++) {
+               set_typestats(stats, (dns_rdatatype_t)i);
+       }
        /* Specials */
-       set_typestats(stats, dns_rdatatype_dlv, false);
-       set_typestats(stats, (dns_rdatatype_t)1000, false);
-       set_nxdomainstats(stats, false);
-
-       /*
-        * Check that all counters are set to appropriately.
-        */
-       dns_rdatasetstats_dump(stats, checkit1, NULL, 1);
+       set_typestats(stats, dns_rdatatype_dlv);
+       set_typestats(stats, (dns_rdatatype_t)1000);
+       set_nxdomainstats(stats);
+
+       /* Check that all active counters are set to appropriately. */
+       dns_rdatasetstats_dump(stats, verify_active_counters, NULL, 1);
+
+       if (servestale) {
+               /* Mark stale */
+               for (i = 0; i <= 255; i++) {
+                       mark_stale(stats, (dns_rdatatype_t)i, 0,
+                                  DNS_RDATASTATSTYPE_ATTR_STALE);
+               }
+               mark_stale(stats, dns_rdatatype_dlv, 0,
+                          DNS_RDATASTATSTYPE_ATTR_STALE);
+               mark_stale(stats, (dns_rdatatype_t)1000, 0,
+                          DNS_RDATASTATSTYPE_ATTR_STALE);
+               mark_nxdomain_stale(stats, 0, DNS_RDATASTATSTYPE_ATTR_STALE);
+
+               /* Check that all counters are set to appropriately. */
+               dns_rdatasetstats_dump(stats, verify_stale_counters, NULL, 1);
+
+               /* Set correct staleness state */
+               from = DNS_RDATASTATSTYPE_ATTR_STALE;
+       }
 
-       /* First 256 types. */
-       for (i = 0; i <= 255; i++)
-               set_typestats(stats, (dns_rdatatype_t)i, true);
-       /* Specials */
-       set_typestats(stats, dns_rdatatype_dlv, true);
-       set_typestats(stats, (dns_rdatatype_t)1000, true);
-       set_nxdomainstats(stats, true);
+       /* Mark ancient */
+       for (i = 0; i <= 255; i++) {
+               mark_stale(stats, (dns_rdatatype_t)i, from,
+                          DNS_RDATASTATSTYPE_ATTR_ANCIENT);
+       }
+       mark_stale(stats, dns_rdatatype_dlv, from,
+                  DNS_RDATASTATSTYPE_ATTR_ANCIENT);
+       mark_stale(stats, (dns_rdatatype_t)1000, from,
+                  DNS_RDATASTATSTYPE_ATTR_ANCIENT);
+       mark_nxdomain_stale(stats, from, DNS_RDATASTATSTYPE_ATTR_ANCIENT);
 
        /*
         * Check that all counters are set to appropriately.
         */
-       dns_rdatasetstats_dump(stats, checkit2, NULL, 1);
+       dns_rdatasetstats_dump(stats, verify_ancient_counters, NULL, 1);
 
        dns_stats_detach(&stats);
 }
 
+/*
+ * Test that rdatasetstats counters are properly set when moving from
+ * active -> stale -> ancient.
+ */
+static void
+test_rdatasetstats_active_stale_ancient(void **state) {
+       rdatasetstats(state, true);
+}
+
+/*
+ * Test that rdatasetstats counters are properly set when moving from
+ * active -> ancient.
+ */
+static void
+test_rdatasetstats_active_ancient(void **state) {
+       rdatasetstats(state, false);
+}
+
 int
 main(void) {
        const struct CMUnitTest tests[] = {
-               cmocka_unit_test_setup_teardown(rdatasetstats,
-                                               _setup, _teardown),
+               cmocka_unit_test_setup_teardown(
+                                       test_rdatasetstats_active_stale_ancient,
+                                       _setup, _teardown),
+               cmocka_unit_test_setup_teardown(
+                                       test_rdatasetstats_active_ancient,
+                                       _setup, _teardown),
        };
 
        return (cmocka_run_group_tests(tests, NULL, NULL));