Fix puppetlabs#294: handle larger service graphs

src/puppetlabs/trapperkeeper/internal.clj

@@ -133,16 +133,43 @@
                     (i18n/trs "Services ''{0}'' not found" missing-services)))))
         (throw e)))))
 
+(schema/defschema GraphCompiler
+  (schema/=> (schema/=> (schema/pred service-graph?)
+                        (schema/pred map?))
+             (schema/pred service-graph?)))
+
+(schema/defn compile-graph*
+  "Helper function for `compile-graph` for testing purposes."
+  [graph-map
+   eager-compile :- GraphCompiler
+   interpreted-eager-compile :- GraphCompiler]
+  {:pre  [(service-graph? graph-map)]
+   :post [(ifn? %)]}
+  (try
+    (eager-compile graph-map)
+    (catch ExceptionInfo e
+      (handle-prismatic-exception! e))
+    ;; work around https://github.com/plumatic/plumbing/issues/138
+    ;; approach: switch to interpreted compilation, which scales to large
+    ;;           service graphs.
+    (catch clojure.lang.Compiler$CompilerException e
+      (log/debug e (str "Error while compiling specialized service graph, trying interpreted mode"
+                        " to handle larger graphs."))
+      (try
+        (interpreted-eager-compile graph-map)
+        (catch ExceptionInfo e
+          (handle-prismatic-exception! e))))))
+
 (defn compile-graph
   "Given the merged map of services, compile it into a function suitable for instantiation.
   Throws an exception if there is a dependency on a service that is not found in the map."
   [graph-map]
   {:pre  [(service-graph? graph-map)]
    :post [(ifn? %)]}
-  (try
-    (graph/eager-compile graph-map)
-    (catch ExceptionInfo e
-      (handle-prismatic-exception! e))))
+  (compile-graph*
+    graph-map
+    graph/eager-compile
+    graph/interpreted-eager-compile))
 
 (defn instantiate
   "Given the compiled graph function, instantiate the application. Throws an exception

test/puppetlabs/trapperkeeper/core_test.clj

@@ -2,13 +2,14 @@
   (:require [clojure.test :refer :all]
             [slingshot.slingshot :refer [try+]]
             [puppetlabs.kitchensink.core :refer [without-ns]]
-            [puppetlabs.trapperkeeper.services :refer [service]]
-            [puppetlabs.trapperkeeper.app :refer [get-service]]
+            [puppetlabs.trapperkeeper.services :refer [defservice service]]
+            [puppetlabs.trapperkeeper.app :as app :refer [get-service]]
             [puppetlabs.trapperkeeper.internal :refer [parse-cli-args!]]
             [puppetlabs.trapperkeeper.core :as trapperkeeper]
             [puppetlabs.trapperkeeper.testutils.bootstrap :as testutils]
             [puppetlabs.trapperkeeper.config :as config]
             [puppetlabs.trapperkeeper.testutils.logging :as logging]
+            [puppetlabs.trapperkeeper.logging :refer [root-logger-name]]
             [schema.test :as schema-test]
             [puppetlabs.kitchensink.core :as ks]))
 
@@ -130,3 +131,60 @@
                     {:config (str tk-config-file-with-restart)
                      :restart-file cli-restart-file})]
         (is (= cli-restart-file (get-in config [:global :restart-file])))))))
+
+;; related: https://github.com/plumatic/plumbing/issues/138
+(def number-of-test-services
+  "Should be higher than the number of defrecord fields
+  allowed in practice in order to hit plumatic graph specialization limits."
+  200)
+
+;; create a chain of services of length `number-of-test-services`
+(doseq [i (range number-of-test-services)
+        :let [->service-symbol (fn [i] (symbol (str "LargeServiceGraphTestService" i)))
+              ->method-symbol (fn [i] (symbol (str "large-service-graph-test-service-method" i)))
+              ->defservice-symbol (fn [i] (symbol (str "large-service-graph-test-service" i)))
+              service-symbol (->service-symbol i)
+              method-symbol (->method-symbol i)
+              defservice-symbol (->defservice-symbol i)
+              previous-service (when (pos? i)
+                                 (->service-symbol (dec i)))
+              previous-method (when (pos? i)
+                                (->method-symbol (dec i)))]]
+  (eval
+    `(defprotocol ~service-symbol
+       (~method-symbol [this# example-input#])))
+  (eval
+    `(defservice
+       ~defservice-symbol
+       ~service-symbol
+       ~(if previous-service
+          [[(keyword previous-service) previous-method]]
+          [])
+       (~method-symbol [this# example-input#]
+                       [~i example-input#
+                        ~(if previous-method
+                           ;; check service graph dependencies still work
+                           (list `first (list previous-method :dummy-arg))
+                           ;; makes writing tests easier
+                           -1)]))))
+
+(def this-ns (ns-name *ns*))
+
+(deftest large-service-graph-test
+  (testing "large service graphs compile correctly via interpretation"
+    (let [services (mapv (fn [i]
+                           @(ns-resolve this-ns (symbol (str "large-service-graph-test-service" i))))
+                         (range number-of-test-services))]
+      (logging/with-test-logging
+        (testutils/with-app-with-config app services {}
+          (is (logged? #"Error while compiling specialized service graph.*" :debug))
+          (let [sg (app/service-graph app)]
+            (testing "right number of services"
+              (is (= (+ 2 number-of-test-services) (count (app/service-graph app)))))
+            (doseq [i (range number-of-test-services)]
+              (let [g (gensym)]
+                (testing (str "service graph function for service " i)
+                  (is (= [i g (dec i)]
+                         ((get-in sg [(keyword (str "LargeServiceGraphTestService" i))
+                                      (keyword (str "large-service-graph-test-service-method" i))])
+                          g))))))))))))

test/puppetlabs/trapperkeeper/internal_test.clj

@@ -1,10 +1,13 @@
 (ns puppetlabs.trapperkeeper.internal-test
   (:require [clojure.test :refer :all]
+            [plumbing.core :refer [fnk]]
+            [plumbing.graph :as graph]
             [puppetlabs.trapperkeeper.core :as tk]
             [puppetlabs.trapperkeeper.app :as tk-app]
             [puppetlabs.trapperkeeper.internal :as internal]
             [puppetlabs.trapperkeeper.testutils.bootstrap :as testutils]
-            [puppetlabs.trapperkeeper.testutils.logging :as logging]))
+            [puppetlabs.trapperkeeper.testutils.logging :as logging]
+            [schema.core :as schema]))
 
 (deftest test-queued-restarts
   (testing "main lifecycle and calls to `restart-tk-apps` are not executed concurrently"
@@ -108,4 +111,62 @@
       (tk-app/stop app)
       ;; and make sure that we got one last :stop
       (is (= (conj expected-lifecycle-events :stop)
-             @lifecycle-events)))))
+             @lifecycle-events)))))
+
+(def dummy-service-map-val 
+  (fnk dummy-service-map-val
+       :- schema/Any
+       []
+       (assert nil)))
+
+;; related: https://github.com/puppetlabs/trapperkeeper/issues/294
+(deftest compile-graph-test
+  (testing "specialized compilation"
+    (let [dummy-small-service-graph {:Service1 dummy-service-map-val}]
+      (is (ifn? (internal/compile-graph dummy-small-service-graph)))))
+  (testing "interpreted compilation"
+    (let [dummy-huge-service-graph (into {}
+                                         (map (fn [i]
+                                                {(keyword (str "Service" i))
+                                                 dummy-service-map-val}))
+                                         ;; should be larger than the number of fields
+                                         ;; allowed in a defrecord in practice.
+                                         ;; related: https://github.com/plumatic/plumbing/issues/138
+                                         (range 1000))]
+      (is (ifn? (internal/compile-graph dummy-huge-service-graph)))))
+  (testing "internal logic"
+    (let [eager-compile-succeed (fn [g]
+                                  ::eager-compile-succeed)
+          eager-compile-too-large (fn [g]
+                                    ;; simulate a "Method too large!"
+                                    ;; or "Too many arguments in method signature in class file" exception
+                                    ;; (ie., the symptoms of hitting the limits of graph/eager-compile).
+                                    (throw (clojure.lang.Compiler$CompilerException.
+                                             ""
+                                             0
+                                             0
+                                             (Exception.))))
+          interpreted-eager-compile-succeed (fn [g]
+                                              ::interpreted-eager-compile-succeed)
+          interpreted-eager-compile-fail (fn [g]
+                                           (throw (Exception. "Interpreted compile failed")))]
+      (testing "specialization succeeds"
+        (is (= ::eager-compile-succeed
+               (internal/compile-graph*
+                 {}
+                 eager-compile-succeed
+                 interpreted-eager-compile-fail))))
+      (testing "specialization fails, interpretation succeeds"
+        (is (= ::interpreted-eager-compile-succeed
+               (internal/compile-graph*
+                 {}
+                 eager-compile-too-large
+                 interpreted-eager-compile-succeed))))
+      (testing "specialization and interpretation fails, throws non-prismatic error"
+        (is (thrown-with-msg?
+              Exception
+              #"Interpreted compile failed"
+              (internal/compile-graph*
+                {}
+                eager-compile-too-large
+                interpreted-eager-compile-fail)))))))