Skip to content

Commit

Permalink
Critical bugfix: (where) evaluated its children every time.
Browse files Browse the repository at this point in the history
When I added (else) blocks to (where) in 0.1.3, I inadvertently
introduced a bug which caused children of (where) to be evaluated
*every* time. This breaks all stateful streams like ddt, rate, etc. It's
a straightforward fix. I've also added a test explicitly confirming
that (by) evaluates its children at the correct times.

Future changes to macros will be accompanied by careful
time-of-evaluation tests. This stuff is surprisingly tricky to do right.
  • Loading branch information
aphyr committed Dec 4, 2012
1 parent 73226ad commit 8f47631
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 5 deletions.
12 changes: 7 additions & 5 deletions src/riemann/streams.clj
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@
(let [dt (- (:time event) (:time prev-event))]
(when-not (zero? dt)
(let [diff (/ (- m (:metric prev-event)) dt)]
(call-rescue (assoc event :metric diff) args))))))))))))
(call-rescue (assoc event :metric diff) args))))))))))))

(defn rate
"Take the sum of every event over interval seconds and divide by the interval
Expand Down Expand Up @@ -1035,10 +1035,12 @@
[expr & children]
(let [p (where-rewrite expr)
[true-kids else-kids] (where-partition-clauses children)]
`(fn [event#]
(if (let [~'event event#] ~p)
(call-rescue event# ~true-kids)
(call-rescue event# ~else-kids)))))
`(let [true-kids# ~true-kids
else-kids# ~else-kids]
(fn [event#]
(if (let [~'event event#] ~p)
(call-rescue event# true-kids#)
(call-rescue event# else-kids#))))))

(defn update-index
"Updates the given index with all events received."
Expand Down
10 changes: 10 additions & 0 deletions test/riemann/test/streams.clj
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,16 @@
(is (= @a ["cat" "badger"]))
(is (= @b ["dog" nil]))))

(deftest where-child-evaluated-once
; Where should evaluate its children exactly once.
(let [x (atom 0)
s (where true (do (swap! x inc) identity))]
(is (= @x 1))
(s {:service "test"})
(is (= @x 1))
(s {:service "test"})
(is (= @x 1))))

(deftest default-kv
(let [r (ref nil)
s (default :service "foo" (register r))]
Expand Down

0 comments on commit 8f47631

Please sign in to comment.