Graphite and pool improvements: logging, better defaults.

sihil · Apr 12, 2013 · a17db77 · a17db77
1 parent ee34854
commit a17db77
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 12 deletions.
diff --git a/src/riemann/graphite.clj b/src/riemann/graphite.clj
@@ -95,7 +95,7 @@
               the path of that event in graphite. graphite-path-percentiles by
               default.
 
-  :pool-size  The number of connections to keep open.
+  :pool-size  The number of connections to keep open. Default 4.
 
   :reconnect-interval   How many seconds to wait between attempts to connect.
                         Default 5.
@@ -111,6 +111,8 @@
   (let [opts (merge {:host "127.0.0.1"
                      :port 2003
                      :protocol :tcp
+                     :claim-timeout 0.1
+                     :pool-size 4
                      :path graphite-path-percentiles} opts)
         pool (fixed-pool
                (fn []

diff --git a/src/riemann/pool.clj b/src/riemann/pool.clj
@@ -1,6 +1,7 @@
 (ns riemann.pool
   "A generic thread-safe resource pool."
-  (:use clojure.tools.logging)
+  (:use clojure.tools.logging
+        [slingshot.slingshot :only [throw+]])
   (:import [java.util.concurrent LinkedBlockingQueue TimeUnit]))
 
 ; THIS IS A MUTABLE STATE OF AFFAIRS. WHICH IS TO SAY, IT IS FUCKING TERRIBLE.
@@ -30,10 +31,16 @@
          (claim this nil))
 
   (claim [this timeout]
-         (try
-           (.poll queue (* 1000 (or timeout 0)) TimeUnit/MILLISECONDS)
-           (catch java.lang.InterruptedException e
-             nil)))
+         (let [timeout (* 1000 (or timeout 0))]
+           (or
+             (try
+               (.poll queue timeout TimeUnit/MILLISECONDS)
+               (catch java.lang.InterruptedException e
+                 nil))
+             (throw+
+               {:type ::timeout
+                :message (str "Couldn't claim a resource from the pool within "
+                              timeout " ms")}))))
 
   (release [this thingy]
            (when thingy

diff --git a/test/riemann/test/pool.clj b/test/riemann/test/pool.clj
@@ -1,5 +1,6 @@
 (ns riemann.test.pool
   (:use riemann.pool
+        [slingshot.slingshot :only [try+]]
         clojure.test))
 
 (deftest claim-release-test
@@ -8,20 +9,24 @@
                ; Claim both elements
                a  (claim pool)
                b  (claim pool)
-               ; Pool is empty; should be nil.
-               c  (claim pool)
+               ; Pool is empty; should throw
+               c  (try+ (claim pool 2/1000)
+                        (catch [:type :riemann.pool/timeout]
+                          {:keys [message]}
+                          message))
                a' (release pool a)
                ; Should re-acquire a
                d  (claim pool)
                ; Empty
-               e  (claim pool)
+               e  (try+ (claim pool)
+                        (catch [:type :riemann.pool/timeout] _ :timeout))
                b' (release pool b)
                ; Re-acquire b
                f  (claim pool)]
            (is (= #{1 2} #{a b}))
-           (is (nil? c))
+           (is (= c "Couldn't claim a resource from the pool within 2 ms"))
            (is (= a d))
-           (is (nil? e))
+           (is (= :timeout e))
            (is (= b f))
            ; Shouldn't have (open)'d more than twice.
            (is (= 2 @x))))
@@ -36,7 +41,7 @@
                b' (invalidate pool b)
                c  (claim pool 1)
                d  (claim pool 1)
-               e  (claim pool 1)
+               e  (try (claim pool 1) (catch Exception e nil))
                c' (invalidate pool c)
                d' (invalidate pool d)
                ; Invalidate nil should be a noop