Migrate Cache Collector into it's own Module

We need the cache collector in the CQL expression cache. Needed in: #1051
samply · Jun 24, 2024 · a2d8afe · a2d8afe
1 parent 8a4b1a9
commit a2d8afe
Show file tree

Hide file tree

Showing 16 changed files with 257 additions and 137 deletions.
diff --git a/dev/blaze/dev.clj b/dev/blaze/dev.clj
@@ -3,7 +3,7 @@
     [blaze.byte-string :as bs]
     [blaze.db.api :as d]
     [blaze.db.api-spec]
-    [blaze.db.cache-collector.protocols :as ccp]
+    [blaze.cache-collector.protocols :as ccp]
     [blaze.db.resource-cache :as resource-cache]
     [blaze.db.resource-store :as rs]
     [blaze.db.tx-log :as tx-log]

diff --git a/modules/cache-collector/.clj-kondo/config.edn b/modules/cache-collector/.clj-kondo/config.edn
@@ -0,0 +1,3 @@
+{:config-paths
+ ["../../../.clj-kondo/root"
+  "../../module-test-util/resources/clj-kondo.exports/blaze/module-test-util"]}
diff --git a/modules/cache-collector/Makefile b/modules/cache-collector/Makefile
@@ -0,0 +1,25 @@
+fmt:
+	cljfmt check
+
+lint:
+	clj-kondo --lint src test deps.edn
+
+prep:
+	clojure -X:deps prep
+
+test: prep
+	clojure -M:test:kaocha --profile :ci
+
+test-coverage: prep
+	clojure -M:test:coverage
+
+deps-tree:
+	clojure -X:deps tree
+
+deps-list:
+	clojure -X:deps list
+
+clean:
+	rm -rf .clj-kondo/.cache .cpcache target
+
+.PHONY: fmt lint prep test test-coverage deps-tree deps-list clean
diff --git a/modules/cache-collector/deps.edn b/modules/cache-collector/deps.edn
@@ -0,0 +1,45 @@
+{:deps
+ {blaze/metrics
+  {:local/root "../metrics"}
+
+  blaze/module-base
+  {:local/root "../module-base"}
+
+  com.github.ben-manes.caffeine/caffeine
+  {:mvn/version "3.1.8"}}
+
+ :aliases
+ {:test
+  {:extra-paths ["test"]
+
+   :extra-deps
+   {blaze/module-test-util
+    {:local/root "../module-test-util"}}}
+
+  :kaocha
+  {:extra-deps
+   {lambdaisland/kaocha
+    {:mvn/version "1.85.1342"}}
+
+   :main-opts ["-m" "kaocha.runner"]}
+
+  :test-perf
+  {:extra-paths ["test-perf"]
+
+   :extra-deps
+   {blaze/fhir-test-util
+    {:local/root "../fhir-test-util"}
+
+    criterium/criterium
+    {:mvn/version "0.4.6"}
+
+    org.openjdk.jol/jol-core
+    {:mvn/version "0.17"}}}
+
+  :coverage
+  {:extra-deps
+   {cloverage/cloverage
+    {:mvn/version "1.2.4"}}
+
+   :main-opts ["-m" "cloverage.coverage" "--codecov" "-p" "src" "-s" "test"
+               "-e" ".+spec"]}}}
diff --git a/modules/db/src/blaze/db/cache_collector.clj → ...e-collector/src/blaze/cache_collector.clj b/modules/db/src/blaze/db/cache_collector.clj → ...e-collector/src/blaze/cache_collector.clj
@@ -1,13 +1,13 @@
-(ns blaze.db.cache-collector
+(ns blaze.cache-collector
   (:require
-   [blaze.db.cache-collector.protocols :as p]
-   [blaze.db.cache-collector.spec]
+   [blaze.cache-collector.protocols :as p]
+   [blaze.cache-collector.spec]
    [blaze.metrics.core :as metrics]
    [blaze.module :as m]
    [clojure.spec.alpha :as s]
    [integrant.core :as ig])
   (:import
-   [com.github.benmanes.caffeine.cache Cache]
+   [com.github.benmanes.caffeine.cache AsyncCache Cache]
    [com.github.benmanes.caffeine.cache.stats CacheStats]))
 
 (set! *warn-on-reflection* true)
@@ -17,7 +17,12 @@
   (-stats [cache]
     (.stats cache))
   (-estimated-size [cache]
-    (.estimatedSize cache)))
+    (.estimatedSize cache))
+  AsyncCache
+  (-stats [cache]
+    (.stats (.synchronous cache)))
+  (-estimated-size [cache]
+    (.estimatedSize (.synchronous cache))))
 
 (defn- sample-xf [f]
   (map (fn [[name stats estimated-size]] {:label-values [name] :value (f stats estimated-size)})))
@@ -36,47 +41,47 @@
      (when cache
        [name (p/-stats cache) (p/-estimated-size cache)]))))
 
-(defmethod m/pre-init-spec :blaze.db/cache-collector [_]
+(defmethod m/pre-init-spec :blaze/cache-collector [_]
   (s/keys :req-un [::caches]))
 
-(defmethod ig/init-key :blaze.db/cache-collector
+(defmethod ig/init-key :blaze/cache-collector
   [_ {:keys [caches]}]
   (metrics/collector
     (let [stats (into [] mapper caches)]
       [(counter-metric
-        "blaze_db_cache_hits_total"
+        "blaze_cache_hits_total"
         "Returns the number of times Cache lookup methods have returned a cached value."
         (fn [stats _] (.hitCount ^CacheStats stats))
         stats)
        (counter-metric
-        "blaze_db_cache_misses_total"
+        "blaze_cache_misses_total"
         "Returns the number of times Cache lookup methods have returned an uncached (newly loaded) value, or null."
         (fn [stats _] (.missCount ^CacheStats stats))
         stats)
        (counter-metric
-        "blaze_db_cache_load_successes_total"
+        "blaze_cache_load_successes_total"
         "Returns the number of times Cache lookup methods have successfully loaded a new value."
         (fn [stats _] (.loadSuccessCount ^CacheStats stats))
         stats)
        (counter-metric
-        "blaze_db_cache_load_failures_total"
+        "blaze_cache_load_failures_total"
         "Returns the number of times Cache lookup methods failed to load a new value, either because no value was found or an exception was thrown while loading."
         (fn [stats _] (.loadFailureCount ^CacheStats stats))
         stats)
        (counter-metric
-        "blaze_db_cache_load_seconds_total"
+        "blaze_cache_load_seconds_total"
         "Returns the total number of seconds the cache has spent loading new values."
         (fn [stats _] (/ (double (.totalLoadTime ^CacheStats stats)) 1e9))
         stats)
        (counter-metric
-        "blaze_db_cache_evictions_total"
+        "blaze_cache_evictions_total"
         "Returns the number of times an entry has been evicted."
         (fn [stats _] (.evictionCount ^CacheStats stats))
         stats)
        (gauge-metric
-        "blaze_db_cache_estimated_size"
+        "blaze_cache_estimated_size"
         "Returns the approximate number of entries in this cache."
         (fn [_ estimated-size] estimated-size)
         stats)])))
 
-(derive :blaze.db/cache-collector :blaze.metrics/collector)
+(derive :blaze/cache-collector :blaze.metrics/collector)
diff --git a/...rc/blaze/db/cache_collector/protocols.clj → ...r/src/blaze/cache_collector/protocols.clj b/...rc/blaze/db/cache_collector/protocols.clj → ...r/src/blaze/cache_collector/protocols.clj
@@ -1,4 +1,4 @@
-(ns blaze.db.cache-collector.protocols)
+(ns blaze.cache-collector.protocols)
 
 (defprotocol StatsCache
   (-stats [_])

diff --git a/modules/cache-collector/src/blaze/cache_collector/spec.clj b/modules/cache-collector/src/blaze/cache_collector/spec.clj
@@ -0,0 +1,7 @@
+(ns blaze.cache-collector.spec
+  (:require
+   [blaze.cache-collector.protocols :as p]
+   [clojure.spec.alpha :as s]))
+
+(s/def :blaze.cache-collector/caches
+  (s/map-of string? (s/nilable #(satisfies? p/StatsCache %))))
diff --git a/modules/cache-collector/test/blaze/cache_collector_test.clj b/modules/cache-collector/test/blaze/cache_collector_test.clj
@@ -0,0 +1,142 @@
+(ns blaze.cache-collector-test
+  (:require
+   [blaze.cache-collector]
+   [blaze.metrics.core :as metrics]
+   [blaze.module.test-util :refer [with-system]]
+   [blaze.test-util :as tu :refer [given-thrown]]
+   [clojure.spec.alpha :as s]
+   [clojure.spec.test.alpha :as st]
+   [clojure.test :as test :refer [deftest testing]]
+   [integrant.core :as ig]
+   [juxt.iota :refer [given]])
+  (:import
+   [com.github.benmanes.caffeine.cache AsyncCache Cache Caffeine]
+   [java.util.function Function]))
+
+(set! *warn-on-reflection* true)
+(st/instrument)
+
+(test/use-fixtures :each tu/fixture)
+
+(def ^Cache cache (-> (Caffeine/newBuilder) (.recordStats) (.build)))
+(def ^AsyncCache async-cache (-> (Caffeine/newBuilder) (.recordStats) (.buildAsync)))
+
+(def config
+  {:blaze/cache-collector
+   {:caches
+    {"name-135224" cache
+     "name-145135" async-cache
+     "name-093214" nil}}})
+
+(deftest init-test
+  (testing "nil config"
+    (given-thrown (ig/init {:blaze/cache-collector nil})
+      :key := :blaze/cache-collector
+      :reason := ::ig/build-failed-spec
+      [:cause-data ::s/problems 0 :pred] := `map?))
+
+  (testing "missing config"
+    (given-thrown (ig/init {:blaze/cache-collector {}})
+      :key := :blaze/cache-collector
+      :reason := ::ig/build-failed-spec
+      [:cause-data ::s/problems 0 :pred] := `(fn ~'[%] (contains? ~'% :caches))))
+
+  (testing "invalid caches"
+    (given-thrown (ig/init {:blaze/cache-collector {:caches ::invalid}})
+      :key := :blaze/cache-collector
+      :reason := ::ig/build-failed-spec
+      [:cause-data ::s/problems 0 :pred] := `map?
+      [:cause-data ::s/problems 0 :val] := ::invalid)))
+
+(deftest cache-collector-test
+  (with-system [{collector :blaze/cache-collector} config]
+
+    (testing "all zero on fresh cache"
+      (given (metrics/collect collector)
+        [0 :name] := "blaze_cache_hits"
+        [0 :type] := :counter
+        [0 :samples count] := 2
+        [0 :samples 0 :value] := 0.0
+        [0 :samples 0 :label-values] := ["name-135224"]
+        [0 :samples 1 :value] := 0.0
+        [0 :samples 1 :label-values] := ["name-145135"]
+        [1 :name] := "blaze_cache_misses"
+        [1 :type] := :counter
+        [1 :samples count] := 2
+        [1 :samples 0 :value] := 0.0
+        [1 :samples 1 :value] := 0.0
+        [2 :name] := "blaze_cache_load_successes"
+        [2 :type] := :counter
+        [2 :samples count] := 2
+        [2 :samples 0 :value] := 0.0
+        [2 :samples 1 :value] := 0.0
+        [3 :name] := "blaze_cache_load_failures"
+        [3 :type] := :counter
+        [3 :samples count] := 2
+        [3 :samples 0 :value] := 0.0
+        [3 :samples 1 :value] := 0.0
+        [4 :name] := "blaze_cache_load_seconds"
+        [4 :type] := :counter
+        [4 :samples count] := 2
+        [4 :samples 0 :value] := 0.0
+        [4 :samples 1 :value] := 0.0
+        [5 :name] := "blaze_cache_evictions"
+        [5 :type] := :counter
+        [5 :samples count] := 2
+        [5 :samples 0 :value] := 0.0
+        [5 :samples 1 :value] := 0.0
+        [6 :name] := "blaze_cache_estimated_size"
+        [6 :type] := :gauge
+        [6 :samples count] := 2
+        [6 :samples 0 :value] := 0.0
+        [6 :samples 1 :value] := 0.0))
+
+    (testing "one load"
+      (.get cache "1" (reify Function (apply [_ key] key)))
+      (.get async-cache "1" (reify Function (apply [_ key] key)))
+      (Thread/sleep 100)
+
+      (given (metrics/collect collector)
+        [0 :name] := "blaze_cache_hits"
+        [0 :samples 0 :value] := 0.0
+        [0 :samples 1 :value] := 0.0
+        [1 :name] := "blaze_cache_misses"
+        [1 :samples 0 :value] := 1.0
+        [1 :samples 1 :value] := 1.0
+        [2 :name] := "blaze_cache_load_successes"
+        [2 :samples 0 :value] := 1.0
+        [2 :samples 1 :value] := 1.0
+        [3 :name] := "blaze_cache_load_failures"
+        [3 :samples 0 :value] := 0.0
+        [3 :samples 1 :value] := 0.0
+        [5 :name] := "blaze_cache_evictions"
+        [5 :samples 0 :value] := 0.0
+        [5 :samples 1 :value] := 0.0
+        [6 :name] := "blaze_cache_estimated_size"
+        [6 :samples 0 :value] := 1.0
+        [6 :samples 1 :value] := 1.0))
+
+    (testing "one loads and one hit"
+      (.get cache "1" (reify Function (apply [_ key] key)))
+      (.get async-cache "1" (reify Function (apply [_ key] key)))
+      (Thread/sleep 100)
+
+      (given (metrics/collect collector)
+        [0 :name] := "blaze_cache_hits"
+        [0 :samples 0 :value] := 1.0
+        [0 :samples 1 :value] := 1.0
+        [1 :name] := "blaze_cache_misses"
+        [1 :samples 0 :value] := 1.0
+        [1 :samples 1 :value] := 1.0
+        [2 :name] := "blaze_cache_load_successes"
+        [2 :samples 0 :value] := 1.0
+        [2 :samples 1 :value] := 1.0
+        [3 :name] := "blaze_cache_load_failures"
+        [3 :samples 0 :value] := 0.0
+        [3 :samples 1 :value] := 0.0
+        [5 :name] := "blaze_cache_evictions"
+        [5 :samples 0 :value] := 0.0
+        [5 :samples 1 :value] := 0.0
+        [6 :name] := "blaze_cache_estimated_size"
+        [6 :samples 0 :value] := 1.0
+        [6 :samples 1 :value] := 1.0))))
diff --git a/modules/cache-collector/tests.edn b/modules/cache-collector/tests.edn
@@ -0,0 +1,5 @@
+#kaocha/v1
+    #merge
+        [{}
+         #profile {:ci {:reporter kaocha.report/documentation
+                        :color? false}}]
diff --git a/modules/db/deps.edn b/modules/db/deps.edn
@@ -10,6 +10,9 @@
   blaze/byte-string
   {:local/root "../byte-string"}
 
+  blaze/cache-collector
+  {:local/root "../cache-collector"}
+
   blaze/coll
   {:local/root "../coll"}
 

diff --git a/modules/db/src/blaze/db/cache_collector/spec.clj b/modules/db/src/blaze/db/cache_collector/spec.clj
diff --git a/modules/db/src/blaze/db/resource_cache.clj b/modules/db/src/blaze/db/resource_cache.clj
@@ -4,7 +4,7 @@
   Caffeine is used because it have better performance characteristics as a
   ConcurrentHashMap."
   (:require
-   [blaze.db.cache-collector.protocols :as ccp]
+   [blaze.cache-collector.protocols :as ccp]
    [blaze.db.resource-cache.spec]
    [blaze.db.resource-store :as rs]
    [blaze.db.resource-store.spec]