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).

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 9909b1e8488ead59c3a89fe0d64b9952b8787baa..ce7b6fffd6ccda22b789e29c3f14301bad978bfd 100644 (file)
--- a/lib/dns/stats.c
+++ b/lib/dns/stats.c
@@ -48,19 +48,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
 };
 
@@ -245,17 +258,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);
        }
 }
@@ -356,34 +369,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 53e518229b78f48632067c9d8f402d5aa0da00d1..ca81a6d55c60cfd996173b32eca1224f0d3fea65 100644 (file)
--- a/lib/dns/tests/rdatasetstats_test.c
+++ b/lib/dns/tests/rdatasetstats_test.c
@@ -54,41 +54,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;
@@ -101,14 +130,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);
@@ -116,7 +147,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;
@@ -129,27 +160,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;
 
@@ -159,39 +224,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));