diff --git a/CHANGELOG.md b/CHANGELOG.md index 80ba676..450e1b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,12 @@ All notable changes to Jarl are documented here. The format follows ### Fixed +- Statistics: after a reconfig that added, removed or recreated views + (creating the first view, for instance), a minute could count everything + since named started: millions of queries in an hour. A per-view counter + (cache, upstream times, validation) going down was taken for a restart + of named. Now only a new boot time means a restart, and a counter that + goes down counts 0 for that minute. - Blocking page, checking a name: when your own block rule is for that very name, it offers to remove the rule instead of an allow rule, which was refused as a second rule for the same name. It also names the rule that diff --git a/internal/stats/poll.go b/internal/stats/poll.go index 1a7c460..bd05ef0 100644 --- a/internal/stats/poll.go +++ b/internal/stats/poll.go @@ -96,23 +96,20 @@ func Flatten(body []byte) (*Snapshot, error) { } // Delta returns the change from prev to cur. When named restarted (its -// boot time changed or a counter went down), the counters started again -// from zero, so the current values are the change. +// boot time changed), the counters started again from zero, so the +// current values are the change. Without a restart, a counter can still +// go down: the per-view ones (cache, upstream times, validation) are +// summed over named's views, and a reconfig that adds, removes or +// recreates views changes that sum. Such a counter counts 0 for the +// minute; it must never make every counter count from zero, which would +// add everything since named started (millions of "queries" in a minute). func Delta(prev, cur *Snapshot) map[string]int64 { out := map[string]int64{} restarted := prev.BootTime != cur.BootTime - if !restarted { - for k, n := range cur.Values { - if n < prev.Values[k] { - restarted = true - break - } - } - } for k, n := range cur.Values { d := n if !restarted { - d = n - prev.Values[k] + d = max(n-prev.Values[k], 0) } if d != 0 || k == "queries" { out[k] = d diff --git a/internal/stats/poll_test.go b/internal/stats/poll_test.go index 6e41a26..16336a9 100644 --- a/internal/stats/poll_test.go +++ b/internal/stats/poll_test.go @@ -47,9 +47,15 @@ func TestFlattenAndDelta(t *testing.T) { if d := Delta(s2, s3); d["queries"] != 3 || d["qtype:A"] != 3 { t.Errorf("restart delta = %v", d) } - // A counter going down also means a restart. - s4 := &Snapshot{BootTime: s2.BootTime, Values: map[string]int64{"queries": 2}} - if d := Delta(s2, s4); d["queries"] != 2 { - t.Errorf("reset delta = %v", d) + // A counter going down without a restart (views changed in a + // reconfig) counts 0, and doesn't make the others count from zero. + s4 := &Snapshot{BootTime: s2.BootTime, Values: map[string]int64{}} + for k, v := range s2.Values { + s4.Values[k] = v + } + s4.Values["queries"] += 4 + s4.Values["cache:hits"] = 1 // was 4: a view's cache counters are gone + if d := Delta(s2, s4); d["queries"] != 4 || d["cache:hits"] != 0 || len(d) != 1 { + t.Errorf("delta with a counter going down = %v", d) } }