From 9f83fbaa494476c84b369c64f42a6e27a45b9921 Mon Sep 17 00:00:00 2001 From: "prateek.khatri" Date: Sun, 3 Apr 2022 19:46:49 +0530 Subject: [PATCH 01/24] added a new state: consumer-connection in addition to the existing connection state --- src/ziggurat/messaging/connection.clj | 45 ++-- test/ziggurat/messaging/connection_test.clj | 241 +++++++------------- 2 files changed, 117 insertions(+), 169 deletions(-) diff --git a/src/ziggurat/messaging/connection.clj b/src/ziggurat/messaging/connection.clj index 889601ea..de6ae125 100644 --- a/src/ziggurat/messaging/connection.clj +++ b/src/ziggurat/messaging/connection.clj @@ -34,21 +34,28 @@ (reduce (fn [sum [_ route-config]] (+ sum (channel-threads (:channels route-config)) worker-count)) 0 stream-routes))) -(defn- create-traced-connection [config] - (let [connection-factory (TracingConnectionFactory. tracer)] - (.setCredentialsProvider connection-factory (DefaultCredentialsProvider. (:username config) (:password config))) - (.newConnection connection-factory ^ExecutorService (:executor config) ^ListAddressResolver (ListAddressResolver. (map #(Address. %) (util/list-of-hosts config)))))) +(defn create-connection + [config] + (rmq/connect (assoc config :hosts (util/list-of-hosts config)))) -(defn create-connection [config tracer-enabled] - (if tracer-enabled - (create-traced-connection config) - (rmq/connect (assoc config :hosts (util/list-of-hosts config))))) +(defn- get-tracer-config [] + (get-in (ziggurat-config) [:tracer :enabled])) -(defn- start-connection [] +(defn- get-connection-config + [is-producer?] + (if is-producer? + (:rabbit-mq-connection (ziggurat-config)) + (assoc (:rabbit-mq-connection (ziggurat-config)) + :executor (Executors/newFixedThreadPool (total-thread-count))))) + +(defn- start-connection + [is-producer?] (log/info "Connecting to RabbitMQ") (when (is-connection-required?) (try - (let [connection (create-connection (assoc (:rabbit-mq-connection (ziggurat-config)) :executor (Executors/newFixedThreadPool (total-thread-count))) (get-in (ziggurat-config) [:tracer :enabled]))] + (let + [connection (create-connection (get-connection-config is-producer?))] + (println "Connection created " connection) (doto connection (.addShutdownListener (reify ShutdownListener @@ -61,11 +68,17 @@ (defn- stop-connection [conn] (when (is-connection-required?) - (if (get-in (ziggurat-config) [:tracer :enabled]) - (.close conn) - (rmq/close conn)) - (log/info "Disconnected from RabbitMQ"))) + (log/info "Closing the RabbitMQ connection") + (rmq/close conn))) + +(defstate consumer-connection + :start (do (log/info "Creating consumer connection") + (start-connection false)) + :stop (do (log/info "Stopping consume connection") + (stop-connection consumer-connection))) (defstate connection - :start (start-connection) - :stop (stop-connection connection)) + :start (do (log/info "Creating producer connection") + (start-connection true)) + :stop (do (log/info "Stopping producer connection") + (stop-connection connection))) diff --git a/test/ziggurat/messaging/connection_test.clj b/test/ziggurat/messaging/connection_test.clj index f6ac2279..c50944bd 100644 --- a/test/ziggurat/messaging/connection_test.clj +++ b/test/ziggurat/messaging/connection_test.clj @@ -4,13 +4,55 @@ [langohr.core :as rmq] [mount.core :as mount] [ziggurat.config :as config] - [ziggurat.messaging.connection :as mc :refer [connection, create-connection]] + [ziggurat.messaging.connection :as mc :refer [connection, consumer-connection, create-connection]] [ziggurat.util.error :refer [report-error]])) (use-fixtures :once fix/mount-config-with-tracer) +(deftest connection-test + (testing "creates thread-pool for consumer connection" + (let [executor-present? (atom false) + orig-rmq-connect rmq/connect + ziggurat-config (config/ziggurat-config) + stream-routes {:default {:handler-fn (constantly :channel-1) + :channel-1 (constantly :success)}}] + (with-redefs [rmq/connect (fn [provided-config] + (when (some? (:executor provided-config)) + (reset! executor-present? true)) + (orig-rmq-connect provided-config)) + config/ziggurat-config (constantly (assoc ziggurat-config + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} + :tracer {:enabled false}))] + (-> (mount/only #{#'consumer-connection}) + (mount/with-args {:stream-routes stream-routes}) + (mount/start)) + (mount/stop #'consumer-connection) + (is @executor-present?)))) + (testing "does not create thread-pool for producer connection" + (let [executor-present? (atom false) + orig-rmq-connect rmq/connect + ziggurat-config (config/ziggurat-config) + stream-routes {:default {:handler-fn (constantly :channel-1) + :channel-1 (constantly :success)}}] + (with-redefs [rmq/connect (fn [provided-config] + (when (some? (:executor provided-config)) + (reset! executor-present? true)) + (orig-rmq-connect provided-config)) + config/ziggurat-config (constantly (assoc ziggurat-config + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} + :tracer {:enabled false}))] + (-> (mount/only #{#'connection}) + (mount/with-args {:stream-routes stream-routes}) + (mount/start)) + (mount/stop #'connection) + (is (false? @executor-present?)))))) + (deftest start-connection-test-with-tracer-disabled - (testing "should provide the correct number of threads for the thread pool if channels are present" + (testing "[consumer-connection] should provide the correct number of threads for the thread pool if channels are present" (let [thread-count (atom 0) orig-rmq-connect rmq/connect rmq-connect-called? (atom false) @@ -26,12 +68,12 @@ :retry {:enabled true} :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} :tracer {}))] - (-> (mount/only #{#'connection}) + (-> (mount/only #{#'consumer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) - (mount/stop #'connection) - (is (= @thread-count 14)) - (is @rmq-connect-called?)))) + (mount/stop #'consumer-connection) + (is @rmq-connect-called?) + (is (= @thread-count 14))))) (testing "if retry is enabled and channels are not present it should create connection" (let [rmq-connect-called? (atom false) @@ -50,6 +92,23 @@ (mount/stop #'connection) (is @rmq-connect-called?)))) + (testing "[consumer-connection] if retry is enabled and channels are not present it should create connection" + (let [rmq-connect-called? (atom false) + orig-rmq-connect rmq/connect + ziggurat-config (config/ziggurat-config) + stream-routes {:default {:handler-fn (constantly :success)}}] + (with-redefs [rmq/connect (fn [provided-config] + (reset! rmq-connect-called? true) + (orig-rmq-connect provided-config)) + config/ziggurat-config (constantly (assoc ziggurat-config + :retry {:enabled true} + :tracer {:enabled false}))] + (-> (mount/only #{#'consumer-connection}) + (mount/with-args {:stream-routes stream-routes}) + (mount/start)) + (mount/stop #'consumer-connection) + (is @rmq-connect-called?)))) + (testing "if retry is disabled and channels are not present it should not create connection" (let [rmq-connect-called? (atom false) orig-rmq-connect rmq/connect @@ -67,6 +126,24 @@ (mount/stop #'connection) (is (not @rmq-connect-called?))))) + (testing "[consumer-connection] if retry is disabled and channels are not present it should not create connection" + (let [rmq-connect-called? (atom false) + orig-rmq-connect rmq/connect + ziggurat-config (config/ziggurat-config) + stream-routes {:default {:handler-fn (constantly :success)}}] + (with-redefs [rmq/connect (fn [provided-config] + (reset! rmq-connect-called? true) + (orig-rmq-connect provided-config)) + config/ziggurat-config (constantly (-> ziggurat-config + (assoc :retry {:enabled false}) + (dissoc :tracer)))] + (-> (mount/only #{#'consumer-connection}) + (mount/with-args {:stream-routes stream-routes}) + (mount/start)) + (mount/stop #'consumer-connection) + (is (not @rmq-connect-called?))))) + + (testing "if retry is disabled and channels are present it should create connection" (let [rmq-connect-called? (atom false) orig-rmq-connect rmq/connect @@ -87,27 +164,7 @@ (mount/stop #'connection) (is @rmq-connect-called?)))) - (testing "should provide the correct number of threads for the thread pool for multiple channels" - (let [thread-count (atom 0) - orig-rmq-connect rmq/connect - ziggurat-config (config/ziggurat-config) - stream-routes {:default {:handler-fn (constantly :success)}}] - (with-redefs [rmq/connect (fn [provided-config] - (reset! thread-count (.getCorePoolSize (:executor provided-config))) - (orig-rmq-connect provided-config)) - config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 5} - :channel-2 {:worker-count 10}}}} - :tracer {:enabled false}))] - (-> (mount/only #{#'connection}) - (mount/with-args {:stream-routes stream-routes}) - (mount/start)) - (mount/stop #'connection) - (is (= @thread-count 19))))) - - (testing "should provide the correct number of threads for the thread pool when channels are not present" + (testing "[consumer-connection] should provide the correct number of threads for the thread pool when channels are not present" (let [thread-count (atom 0) orig-rmq-connect rmq/connect ziggurat-config (config/ziggurat-config)] @@ -119,8 +176,8 @@ :retry {:enabled true} :stream-router {:default {}} :tracer {:enabled false}))] - (mount/start (mount/only [#'connection])) - (mount/stop #'connection) + (mount/start (mount/only [#'consumer-connection])) + (mount/stop #'consumer-connection) (is (= @thread-count 4))))) (testing "should provide the correct number of threads for the thread pool for multiple stream routes" @@ -136,133 +193,11 @@ :stream-router {:default {:channels {:channel-1 {:worker-count 10}}} :default-1 {:channels {:channel-1 {:worker-count 8}}}} :tracer {:enabled false}))] - (mount/start (mount/only [#'connection])) - (mount/stop #'connection) + (mount/start (mount/only [#'consumer-connection])) + (mount/stop #'consumer-connection) (is (= @thread-count 26)))))) -(deftest start-connection-test-with-tracer-enabled - (testing "should provide the correct number of threads for the thread pool if channels are present" - (let [thread-count (atom 0) - orig-create-conn mc/create-connection - create-connect-called? (atom false) - ziggurat-config (config/ziggurat-config) - stream-routes {:default {:handler-fn (constantly :channel-1) - :channel-1 (constantly :success)}}] - (with-redefs [mc/create-connection (fn [provided-config tracer-enabled] - (reset! create-connect-called? true) - (reset! thread-count (.getCorePoolSize (:executor provided-config))) - (orig-create-conn provided-config tracer-enabled)) - config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}}))] - (-> (mount/only #{#'connection}) - (mount/with-args {:stream-routes stream-routes}) - (mount/start)) - (mount/stop #'connection) - (is (= @thread-count 14)) - (is @create-connect-called?)))) - - (testing "if retry is enabled and channels are not present it should create connection" - (let [create-connect-called? (atom false) - orig-create-conn mc/create-connection - ziggurat-config (config/ziggurat-config) - stream-routes {:default {:handler-fn (constantly :success)}}] - (with-redefs [mc/create-connection (fn [provided-config tracer-enabled] - (reset! create-connect-called? true) - (orig-create-conn provided-config tracer-enabled)) - config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled true}))] - (-> (mount/only #{#'connection}) - (mount/with-args {:stream-routes stream-routes}) - (mount/start)) - (mount/stop #'connection) - (is @create-connect-called?)))) - - (testing "if retry is disabled and channels are not present it should not create connection" - (let [create-connect-called? (atom false) - orig-create-conn mc/create-connection - ziggurat-config (config/ziggurat-config) - stream-routes {:default {:handler-fn (constantly :success)}}] - (with-redefs [mc/create-connection (fn [provided-config tracer-enabled] - (reset! create-connect-called? true) - (orig-create-conn provided-config tracer-enabled)) - config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false}))] - (-> (mount/only #{#'connection}) - (mount/with-args {:stream-routes stream-routes}) - (mount/start)) - (mount/stop #'connection) - (is (not @create-connect-called?))))) - - (testing "if retry is disabled and channels are present it should create connection" - (let [create-connect-called? (atom false) - orig-create-conn mc/create-connection - ziggurat-config (config/ziggurat-config) - stream-routes {:default {:handler-fn (constantly :channel-1) - :channel-1 (constantly :success)} - :default-1 {:handler-fn (constantly :channel-3) - :channel-3 (constantly :success)}}] - (with-redefs [mc/create-connection (fn [provided-config tracer-enabled] - (reset! create-connect-called? true) - (orig-create-conn provided-config tracer-enabled)) - config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false}))] - (-> (mount/only #{#'connection}) - (mount/with-args {:stream-routes stream-routes}) - (mount/start)) - (mount/stop #'connection) - (is @create-connect-called?)))) - - (testing "should provide the correct number of threads for the thread pool for multiple channels" - (let [thread-count (atom 0) - orig-create-conn mc/create-connection - ziggurat-config (config/ziggurat-config) - stream-routes {:default {:handler-fn (constantly :success)}}] - (with-redefs [mc/create-connection (fn [provided-config tracer-enabled] - (reset! thread-count (.getCorePoolSize (:executor provided-config))) - (orig-create-conn provided-config tracer-enabled)) - config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 5} - :channel-2 {:worker-count 10}}}}))] - (-> (mount/only #{#'connection}) - (mount/with-args {:stream-routes stream-routes}) - (mount/start)) - (mount/stop #'connection) - (is (= @thread-count 19))))) - - (testing "should provide the correct number of threads for the thread pool when channels are not present" - (let [thread-count (atom 0) - orig-create-conn mc/create-connection - ziggurat-config (config/ziggurat-config)] - (with-redefs [mc/create-connection (fn [provided-config tracer-enabled] - (reset! thread-count (.getCorePoolSize (:executor provided-config))) - (orig-create-conn provided-config tracer-enabled)) - config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {}}))] - (mount/start (mount/only [#'connection])) - (mount/stop #'connection) - (is (= @thread-count 4))))) - (testing "should provide the correct number of threads for the thread pool for multiple stream routes" - (let [thread-count (atom 0) - orig-create-conn mc/create-connection - ziggurat-config (config/ziggurat-config)] - (with-redefs [mc/create-connection (fn [provided-config tracer-enabled] - (reset! thread-count (.getCorePoolSize (:executor provided-config))) - (orig-create-conn provided-config tracer-enabled)) - config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}} - :default-1 {:channels {:channel-1 {:worker-count 8}}}}))] - (mount/start (mount/only [#'connection])) - (mount/stop #'connection) - (is (= @thread-count 26)))))) (deftest start-connection-test-with-errors (testing "if rabbitmq connect throws an error, it gets reported" (let [stream-routes {:default {:handler-fn (constantly :success)}} From e08e5992e335e8ab72fbc9f8b2c923851e59e346 Mon Sep 17 00:00:00 2001 From: "prateek.khatri" Date: Mon, 4 Apr 2022 13:16:58 +0530 Subject: [PATCH 02/24] Using consumer-connection in consumer.clj (and edited the tests) --- src/ziggurat/messaging/consumer.clj | 10 +++++----- test/ziggurat/messaging/consumer_test.clj | 22 +++++++++++----------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/ziggurat/messaging/consumer.clj b/src/ziggurat/messaging/consumer.clj index ade474fd..b8906052 100644 --- a/src/ziggurat/messaging/consumer.clj +++ b/src/ziggurat/messaging/consumer.clj @@ -7,7 +7,7 @@ [ziggurat.config :refer [get-in-config rabbitmq-config]] [ziggurat.kafka-consumer.consumer-handler :as ch] [ziggurat.mapper :as mpr] - [ziggurat.messaging.connection :refer [connection]] + [ziggurat.messaging.connection :refer [consumer-connection]] [ziggurat.messaging.util :as util] [ziggurat.metrics :as metrics] [ziggurat.util.error :refer [report-error]] @@ -85,7 +85,7 @@ (get-dead-set-messages topic-entity nil count)) ([topic-entity channel count] (remove nil? - (with-open [ch (lch/open connection)] + (with-open [ch (lch/open consumer-connection)] (doall (for [_ (range count)] (read-message-from-queue ch (construct-queue-name topic-entity channel) topic-entity false))))))) @@ -95,7 +95,7 @@ ([topic-entity count processing-fn] (process-dead-set-messages topic-entity nil count processing-fn)) ([topic-entity channel count processing-fn] - (with-open [ch (lch/open connection)] + (with-open [ch (lch/open consumer-connection)] (doall (for [_ (range count)] (let [queue-name (construct-queue-name topic-entity channel) [meta payload] (lb/get ch queue-name false)] @@ -128,7 +128,7 @@ (defn start-retry-subscriber* [handler-fn topic-entity] (when (get-in-config [:retry :enabled]) (dotimes [_ (get-in-config [:jobs :instant :worker-count])] - (start-subscriber* (lch/open connection) + (start-subscriber* (lch/open consumer-connection) (get-in-config [:jobs :instant :prefetch-count]) (util/prefixed-queue-name topic-entity (get-in-config [:rabbit-mq :instant :queue-name])) handler-fn @@ -139,7 +139,7 @@ (let [channel-key (first channel) channel-handler-fn (second channel)] (dotimes [_ (get-in-config [:stream-router topic-entity :channels channel-key :worker-count])] - (start-subscriber* (lch/open connection) + (start-subscriber* (lch/open consumer-connection) 1 (util/prefixed-channel-name topic-entity channel-key (get-in-config [:rabbit-mq :instant :queue-name])) (mpr/channel-mapper-func channel-handler-fn channel-key) diff --git a/test/ziggurat/messaging/consumer_test.clj b/test/ziggurat/messaging/consumer_test.clj index d78e79da..83cdcc25 100644 --- a/test/ziggurat/messaging/consumer_test.clj +++ b/test/ziggurat/messaging/consumer_test.clj @@ -5,7 +5,7 @@ [taoensso.nippy :as nippy] [ziggurat.config :refer [ziggurat-config rabbitmq-config]] [ziggurat.fixtures :as fix] - [ziggurat.messaging.connection :refer [connection]] + [ziggurat.messaging.connection :refer [consumer-connection]] [ziggurat.messaging.consumer :as consumer] [ziggurat.messaging.producer :as producer] [ziggurat.messaging.util :refer [prefixed-queue-name]] @@ -104,7 +104,7 @@ (fix/with-queues {topic-entity {:handler-fn #(constantly nil)}} (let [no-of-workers 3 original-zig-config (ziggurat-config) - ch (lch/open connection) + ch (lch/open consumer-connection) counter (atom 0)] (with-redefs [ziggurat-config (fn [] (-> original-zig-config (update-in [:retry :enabled] (constantly true)) @@ -118,7 +118,7 @@ (testing "start subscribers should call start-subscriber* according to the product of worker and mapper-fns in stream-routes" (let [no-of-workers 3 original-zig-config (ziggurat-config) - ch (lch/open connection) + ch (lch/open consumer-connection) counter (atom 0) stream-routes {topic-entity {:handler-fn #(constantly nil)} :test {:handler-fn #(constantly nil)}}] @@ -134,7 +134,7 @@ (fix/with-queues {topic-entity {:handler-fn #(constantly nil)}} (let [no-of-workers 3 original-zig-config (ziggurat-config) - ch (lch/open connection) + ch (lch/open consumer-connection) counter (atom 0)] (with-redefs [ziggurat-config (fn [] (-> original-zig-config @@ -151,7 +151,7 @@ (fix/with-queues {topic-entity {:handler-fn #(constantly nil)}} (let [no-of-workers 3 original-zig-config (ziggurat-config) - ch (lch/open connection) + ch (lch/open consumer-connection) counter (atom 0)] (with-redefs [ziggurat-config (fn [] (-> original-zig-config @@ -177,7 +177,7 @@ :retry-limit 2 :success-promise success-promise}) original-zig-config (ziggurat-config) - rmq-ch (lch/open connection)] + rmq-ch (lch/open consumer-connection)] (fix/with-queues {topic-entity {:handler-fn #(constantly nil) channel channel-fn}} (with-redefs [ziggurat-config (fn [] (-> original-zig-config @@ -204,7 +204,7 @@ :retry-limit 2 :success-promise success-promise}) original-zig-config (ziggurat-config) - rmq-ch (lch/open connection)] + rmq-ch (lch/open consumer-connection)] (fix/with-queues {topic-entity {:handler-fn #(constantly nil) channel channel-fn}} (with-redefs [ziggurat-config (fn [] (-> original-zig-config @@ -225,7 +225,7 @@ retry-count 3 message-payload (assoc (gen-message-payload topic-entity) :retry-count 3) original-zig-config (ziggurat-config) - rmq-ch (lch/open connection)] + rmq-ch (lch/open consumer-connection)] (.reset tracer) (with-redefs [ziggurat-config (fn [] (-> original-zig-config (update-in [:retry :count] (constantly retry-count)) @@ -260,7 +260,7 @@ (is (= message-arg message))) topic-entity-name (name topic-entity)] (producer/publish-to-dead-queue message) - (with-open [ch (lch/open connection)] + (with-open [ch (lch/open consumer-connection)] (let [queue-name (get-in (rabbitmq-config) [:dead-letter :queue-name]) prefixed-queue-name (str topic-entity-name "_" queue-name) [meta payload] (lb/get ch prefixed-queue-name false) @@ -277,7 +277,7 @@ topic-entity-name (name topic-entity)] (producer/publish-to-dead-queue message) (with-redefs [consumer/convert-and-ack-message (fn [_ _ _ _ _] nil)] - (with-open [ch (lch/open connection)] + (with-open [ch (lch/open consumer-connection)] (let [queue-name (get-in (rabbitmq-config) [:dead-letter :queue-name]) prefixed-queue-name (str topic-entity-name "_" queue-name) [meta payload] (lb/get ch prefixed-queue-name false) @@ -295,7 +295,7 @@ report-fn-called? (atom false)] (with-redefs [report-error (fn [_ _] (reset! report-fn-called? true))] (producer/publish-to-dead-queue message) - (with-open [ch (lch/open connection)] + (with-open [ch (lch/open consumer-connection)] (let [queue-name (get-in (rabbitmq-config) [:dead-letter :queue-name]) prefixed-queue-name (str topic-entity-name "_" queue-name) [meta payload] (lb/get ch prefixed-queue-name false) From 4880354ea3b2a2e83a21b412f863b4656b95ff14 Mon Sep 17 00:00:00 2001 From: "prateek.khatri" Date: Mon, 4 Apr 2022 13:17:45 +0530 Subject: [PATCH 03/24] Restored the code for tracer-enabled connections --- src/ziggurat/messaging/connection.clj | 17 ++- test/ziggurat/fixtures.clj | 4 +- test/ziggurat/messaging/connection_test.clj | 156 ++++++++++++++++++-- 3 files changed, 158 insertions(+), 19 deletions(-) diff --git a/src/ziggurat/messaging/connection.clj b/src/ziggurat/messaging/connection.clj index de6ae125..e9a0dd11 100644 --- a/src/ziggurat/messaging/connection.clj +++ b/src/ziggurat/messaging/connection.clj @@ -34,13 +34,22 @@ (reduce (fn [sum [_ route-config]] (+ sum (channel-threads (:channels route-config)) worker-count)) 0 stream-routes))) -(defn create-connection - [config] - (rmq/connect (assoc config :hosts (util/list-of-hosts config)))) + +(defn- create-traced-connection [config] + (let [connection-factory (TracingConnectionFactory. tracer)] + (.setCredentialsProvider connection-factory (DefaultCredentialsProvider. (:username config) (:password config))) + (if (some? (:executor config)) + (.newConnection connection-factory ^ExecutorService (:executor config) ^ListAddressResolver (ListAddressResolver. (map #(Address. %) (util/list-of-hosts config)))) + (.newConnection connection-factory ^ListAddressResolver (ListAddressResolver. (map #(Address. %) (util/list-of-hosts config))))))) (defn- get-tracer-config [] (get-in (ziggurat-config) [:tracer :enabled])) +(defn create-connection [config tracer-enabled] + (if tracer-enabled + (create-traced-connection config) + (rmq/connect (assoc config :hosts (util/list-of-hosts config))))) + (defn- get-connection-config [is-producer?] (if is-producer? @@ -54,7 +63,7 @@ (when (is-connection-required?) (try (let - [connection (create-connection (get-connection-config is-producer?))] + [connection (create-connection (get-connection-config is-producer?) (get-tracer-config))] (println "Connection created " connection) (doto connection (.addShutdownListener diff --git a/test/ziggurat/fixtures.clj b/test/ziggurat/fixtures.clj index 841763b7..d1685d81 100644 --- a/test/ziggurat/fixtures.clj +++ b/test/ziggurat/fixtures.clj @@ -6,7 +6,7 @@ [mount.core :as mount] [ziggurat.config :as config] [ziggurat.kafka-consumer.executor-service :refer [thread-pool]] - [ziggurat.messaging.connection :refer [connection]] + [ziggurat.messaging.connection :refer [connection consumer-connection]] [ziggurat.messaging.producer :as pr] [ziggurat.messaging.util :as util] [ziggurat.metrics :as metrics] @@ -119,7 +119,7 @@ (mount-tracer) (-> - (mount/only [#'connection]) + (mount/only [#'connection #'consumer-connection]) (mount/with-args {:stream-routes stream-routes}) (mount/start)) (f) diff --git a/test/ziggurat/messaging/connection_test.clj b/test/ziggurat/messaging/connection_test.clj index c50944bd..8b0c22a3 100644 --- a/test/ziggurat/messaging/connection_test.clj +++ b/test/ziggurat/messaging/connection_test.clj @@ -73,26 +73,33 @@ (mount/start)) (mount/stop #'consumer-connection) (is @rmq-connect-called?) - (is (= @thread-count 14))))) + (is (= 14 @thread-count))))) - (testing "if retry is enabled and channels are not present it should create connection" - (let [rmq-connect-called? (atom false) + (testing "[consumer-connection] if retry is enabled and channels are not present it should create connection" + (let [thread-count (atom 0) + rmq-connect-called? (atom false) orig-rmq-connect rmq/connect ziggurat-config (config/ziggurat-config) stream-routes {:default {:handler-fn (constantly :success)}}] (with-redefs [rmq/connect (fn [provided-config] + (println "Tracer Disabled ********** " provided-config) + (println "Tracer Disabled ********** " (.getCorePoolSize (:executor provided-config))) (reset! rmq-connect-called? true) + (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled true} - :tracer {:enabled false}))] - (-> (mount/only #{#'connection}) + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} + :tracer {:enabled false}))] + (-> (mount/only #{#'consumer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) - (mount/stop #'connection) - (is @rmq-connect-called?)))) + (mount/stop #'consumer-connection) + (is @rmq-connect-called?) + (is (= 14 @thread-count))))) - (testing "[consumer-connection] if retry is enabled and channels are not present it should create connection" + (testing "if retry is enabled and channels are not present it should create connection" (let [rmq-connect-called? (atom false) orig-rmq-connect rmq/connect ziggurat-config (config/ziggurat-config) @@ -101,12 +108,12 @@ (reset! rmq-connect-called? true) (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled true} - :tracer {:enabled false}))] - (-> (mount/only #{#'consumer-connection}) + :retry {:enabled true} + :tracer {:enabled false}))] + (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) - (mount/stop #'consumer-connection) + (mount/stop #'connection) (is @rmq-connect-called?)))) (testing "if retry is disabled and channels are not present it should not create connection" @@ -197,6 +204,129 @@ (mount/stop #'consumer-connection) (is (= @thread-count 26)))))) +(deftest start-connection-test-with-tracer-enabled + (testing "[consumer-connection] should provide the correct number of threads for the thread pool if channels are present" + (let [thread-count (atom 0) + orig-create-conn mc/create-connection + create-connect-called? (atom false) + ziggurat-config (config/ziggurat-config) + stream-routes {:default {:handler-fn (constantly :channel-1) + :channel-1 (constantly :success)}}] + (with-redefs [mc/create-connection (fn [provided-config tracer-enabled] + (reset! create-connect-called? true) + (reset! thread-count (.getCorePoolSize (:executor provided-config))) + (orig-create-conn provided-config tracer-enabled)) + config/ziggurat-config (constantly (assoc ziggurat-config + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}}))] + (-> (mount/only #{#'consumer-connection}) + (mount/with-args {:stream-routes stream-routes}) + (mount/start)) + (mount/stop #'consumer-connection) + (is (= @thread-count 14)) + (is @create-connect-called?)))) + + (testing "if retry is enabled and channels are not present it should create connection" + (let [create-connect-called? (atom false) + orig-create-conn mc/create-connection + ziggurat-config (config/ziggurat-config) + stream-routes {:default {:handler-fn (constantly :success)}}] + (with-redefs [mc/create-connection (fn [provided-config tracer-enabled] + (reset! create-connect-called? true) + (orig-create-conn provided-config tracer-enabled)) + config/ziggurat-config (constantly (assoc ziggurat-config + :retry {:enabled true}))] + (-> (mount/only #{#'connection}) + (mount/with-args {:stream-routes stream-routes}) + (mount/start)) + (mount/stop #'connection) + (is @create-connect-called?)))) + + (testing "if retry is disabled and channels are not present it should not create connection" + (let [create-connect-called? (atom false) + orig-create-conn mc/create-connection + ziggurat-config (config/ziggurat-config) + stream-routes {:default {:handler-fn (constantly :success)}}] + (with-redefs [mc/create-connection (fn [provided-config tracer-enabled] + (reset! create-connect-called? true) + (orig-create-conn provided-config tracer-enabled)) + config/ziggurat-config (constantly (assoc ziggurat-config + :retry {:enabled false}))] + (-> (mount/only #{#'connection}) + (mount/with-args {:stream-routes stream-routes}) + (mount/start)) + (mount/stop #'connection) + (is (not @create-connect-called?))))) + + (testing "if retry is disabled and channels are present it should create connection" + (let [create-connect-called? (atom false) + orig-create-conn mc/create-connection + ziggurat-config (config/ziggurat-config) + stream-routes {:default {:handler-fn (constantly :channel-1) + :channel-1 (constantly :success)} + :default-1 {:handler-fn (constantly :channel-3) + :channel-3 (constantly :success)}}] + (with-redefs [mc/create-connection (fn [provided-config tracer-enabled] + (reset! create-connect-called? true) + (orig-create-conn provided-config tracer-enabled)) + config/ziggurat-config (constantly (assoc ziggurat-config + :retry {:enabled false}))] + (-> (mount/only #{#'connection}) + (mount/with-args {:stream-routes stream-routes}) + (mount/start)) + (mount/stop #'connection) + (is @create-connect-called?)))) + + (testing "should provide the correct number of threads for the thread pool for multiple channels" + (let [thread-count (atom 0) + orig-create-conn mc/create-connection + ziggurat-config (config/ziggurat-config) + stream-routes {:default {:handler-fn (constantly :success)}}] + (with-redefs [mc/create-connection (fn [provided-config tracer-enabled] + (reset! thread-count (.getCorePoolSize (:executor provided-config))) + (orig-create-conn provided-config tracer-enabled)) + config/ziggurat-config (constantly (assoc ziggurat-config + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 5} + :channel-2 {:worker-count 10}}}}))] + (-> (mount/only #{#'consumer-connection}) + (mount/with-args {:stream-routes stream-routes}) + (mount/start)) + (mount/stop #'consumer-connection) + (is (= @thread-count 19))))) + + (testing "should provide the correct number of threads for the thread pool when channels are not present" + (let [thread-count (atom 0) + orig-create-conn mc/create-connection + ziggurat-config (config/ziggurat-config)] + (with-redefs [mc/create-connection (fn [provided-config tracer-enabled] + (reset! thread-count (.getCorePoolSize (:executor provided-config))) + (orig-create-conn provided-config tracer-enabled)) + config/ziggurat-config (constantly (assoc ziggurat-config + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {}}))] + (mount/start (mount/only [#'consumer-connection])) + (mount/stop #'consumer-connection) + (is (= @thread-count 4))))) + + (testing "should provide the correct number of threads for the thread pool for multiple stream routes" + (let [thread-count (atom 0) + orig-create-conn mc/create-connection + ziggurat-config (config/ziggurat-config)] + (with-redefs [mc/create-connection (fn [provided-config tracer-enabled] + (reset! thread-count (.getCorePoolSize (:executor provided-config))) + (orig-create-conn provided-config tracer-enabled)) + config/ziggurat-config (constantly (assoc ziggurat-config + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}} + :default-1 {:channels {:channel-1 {:worker-count 8}}}}))] + (mount/start (mount/only [#'consumer-connection])) + (mount/stop #'consumer-connection) + (is (= @thread-count 26)))))) (deftest start-connection-test-with-errors (testing "if rabbitmq connect throws an error, it gets reported" From 3f4495e32babd95b7a793920d1bc80b354808a1f Mon Sep 17 00:00:00 2001 From: "prateek.khatri" Date: Mon, 4 Apr 2022 13:18:08 +0530 Subject: [PATCH 04/24] Created a RabbitMQ Channel Factory class --- project.clj | 3 +- .../channel_pool/RabbitMQChannelFactory.java | 38 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java diff --git a/project.clj b/project.clj index b7a1ed9b..dda2aa5e 100644 --- a/project.clj +++ b/project.clj @@ -57,7 +57,8 @@ [ch.qos.logback/logback-classic "1.2.9"] [ch.qos.logback.contrib/logback-json-classic "0.1.5"] [ch.qos.logback.contrib/logback-jackson "0.1.5"] - [net.logstash.logback/logstash-logback-encoder "6.6"]] + [net.logstash.logback/logstash-logback-encoder "6.6"] + [org.apache.commons/commons-pool2 "2.11.1"]] :deploy-repositories [["clojars" {:url "https://clojars.org/repo" :username :env/clojars_username :password :env/clojars_password diff --git a/src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java b/src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java new file mode 100644 index 00000000..f446078c --- /dev/null +++ b/src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java @@ -0,0 +1,38 @@ +package gojek.rabbitmq.channel_pool; + +import com.rabbitmq.client.Channel; +import com.rabbitmq.client.Connection; +import org.apache.commons.pool2.BasePooledObjectFactory; +import org.apache.commons.pool2.DestroyMode; +import org.apache.commons.pool2.PooledObject; +import org.apache.commons.pool2.impl.DefaultPooledObject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class RabbitMQChannelFactory extends BasePooledObjectFactory { + Logger log = LoggerFactory.getLogger(this.getClass()); + + private final Connection connection; + + public RabbitMQChannelFactory(Connection connection) { + this.connection = connection; + } + + @Override + public Channel create() throws Exception { + return connection.createChannel(); + } + + @Override + public PooledObject wrap(Channel channel) { + return new DefaultPooledObject(channel); + } + + @Override + public void destroyObject(PooledObject p, DestroyMode destroyMode) throws Exception { + super.destroyObject(p, destroyMode); + log.info("Closing the channel with id: " + p.getObject().getChannelNumber()); + System.out.println("Closing the channel with id: " + p.getObject().getChannelNumber()); + p.getObject().close(); + } +} From f2a1ae50c3a338e1a5e34ab7fcb019b59991ac5c Mon Sep 17 00:00:00 2001 From: "prateek.khatri" Date: Mon, 4 Apr 2022 13:19:37 +0530 Subject: [PATCH 05/24] Refactoring some log statements in RabbitMQ Channel Factory --- src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java b/src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java index f446078c..e9358a1f 100644 --- a/src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java +++ b/src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java @@ -20,6 +20,7 @@ public RabbitMQChannelFactory(Connection connection) { @Override public Channel create() throws Exception { + log.info("Creating a new channel"); return connection.createChannel(); } @@ -32,7 +33,6 @@ public PooledObject wrap(Channel channel) { public void destroyObject(PooledObject p, DestroyMode destroyMode) throws Exception { super.destroyObject(p, destroyMode); log.info("Closing the channel with id: " + p.getObject().getChannelNumber()); - System.out.println("Closing the channel with id: " + p.getObject().getChannelNumber()); p.getObject().close(); } } From 677eca559f3931f34191ff1356e82aff9a72394b Mon Sep 17 00:00:00 2001 From: "shubhang.balkundi" Date: Mon, 4 Apr 2022 18:16:44 +0530 Subject: [PATCH 06/24] adds a RabbitMQ channel pool for publishing --- src/ziggurat/config.clj | 6 +- src/ziggurat/messaging/channel_pool.clj | 43 +++++++ src/ziggurat/messaging/connection.clj | 15 ++- src/ziggurat/messaging/consumer.clj | 17 +-- src/ziggurat/messaging/producer.clj | 118 ++++++++++---------- test/ziggurat/messaging/connection_test.clj | 59 +++++----- 6 files changed, 154 insertions(+), 104 deletions(-) create mode 100644 src/ziggurat/messaging/channel_pool.clj diff --git a/src/ziggurat/config.clj b/src/ziggurat/config.clj index 26ac217e..a7a9053e 100644 --- a/src/ziggurat/config.clj +++ b/src/ziggurat/config.clj @@ -167,7 +167,7 @@ (str/trim (cond (keyword? v) (name v) - :else (str v)))) + :else (str v)))) (defn set-property [mapping-table p k v] @@ -191,8 +191,8 @@ (defn- add-jaas-properties [properties jaas-config] (if (some? jaas-config) - (let [username (get jaas-config :username) - password (get jaas-config :password) + (let [username (get jaas-config :username) + password (get jaas-config :password) mechanism (get jaas-config :mechanism)] (doto properties (.put SaslConfigs/SASL_JAAS_CONFIG (create-jaas-properties username password mechanism)))) diff --git a/src/ziggurat/messaging/channel_pool.clj b/src/ziggurat/messaging/channel_pool.clj new file mode 100644 index 00000000..f1a5985c --- /dev/null +++ b/src/ziggurat/messaging/channel_pool.clj @@ -0,0 +1,43 @@ +(ns ziggurat.messaging.channel_pool + (:require [mount.core :refer [defstate]] + [ziggurat.config :as zc] + [ziggurat.messaging.connection :as c] + [clojure.tools.logging :as log]) + (:import (com.rabbitmq.client Connection) + (org.apache.commons.pool2.impl GenericObjectPool GenericObjectPoolConfig) + (gojek.rabbitmq.channel_pool RabbitMQChannelFactory) + (java.time Duration))) + +(def default-config + {:max-wait-ms 5000 + :min-idle 8 + :max-idle 8}) + +(defn calc-total-thread-count [] + (let [buffer-threads 10 + channel-thread-count (c/total-thread-count) + + stream-router-config (get (zc/ziggurat-config) :stream-router) + stream-threads-count (reduce (fn [sum config] + (+ sum (:stream-threads-count config))) 0 (vals stream-router-config))] + (+ stream-threads-count channel-thread-count buffer-threads))) + +(defn create-object-pool-config [config] + (let [merged-config (merge config default-config)] + (println merged-config) + (doto (GenericObjectPoolConfig.) + (.setMaxWait (Duration/ofMillis (get merged-config :max-wait-ms))) + (.setMinIdle (get merged-config :min-idle)) + (.setMaxIdle (get merged-config :max-idle)) + (.setMaxTotal (calc-total-thread-count))))) + +(defn create-channel-pool [^Connection connection] + (let [pool-config (create-object-pool-config (get-in zc/ziggurat-config [:rabbit-mq-connection :channel-pool])) + rmq-chan-pool (GenericObjectPool. (RabbitMQChannelFactory. connection) pool-config)] + rmq-chan-pool)) + +(defstate channel-pool + :start (do (log/info "Creating channel pool") + (create-channel-pool c/connection)) + :stop (do (log/info "Stopping channel pool") + (.close channel-pool))) diff --git a/src/ziggurat/messaging/connection.clj b/src/ziggurat/messaging/connection.clj index e9a0dd11..24ac8cfc 100644 --- a/src/ziggurat/messaging/connection.clj +++ b/src/ziggurat/messaging/connection.clj @@ -28,13 +28,12 @@ (reduce (fn [sum [_ channel-config]] (+ sum (:worker-count channel-config))) 0 channels)) -(defn- total-thread-count [] +(defn total-thread-count [] (let [stream-routes (:stream-router (ziggurat-config)) worker-count (get-in (ziggurat-config) [:jobs :instant :worker-count])] (reduce (fn [sum [_ route-config]] (+ sum (channel-threads (:channels route-config)) worker-count)) 0 stream-routes))) - (defn- create-traced-connection [config] (let [connection-factory (TracingConnectionFactory. tracer)] (.setCredentialsProvider connection-factory (DefaultCredentialsProvider. (:username config) (:password config))) @@ -55,7 +54,7 @@ (if is-producer? (:rabbit-mq-connection (ziggurat-config)) (assoc (:rabbit-mq-connection (ziggurat-config)) - :executor (Executors/newFixedThreadPool (total-thread-count))))) + :executor (Executors/newFixedThreadPool (total-thread-count))))) (defn- start-connection [is-producer?] @@ -63,7 +62,7 @@ (when (is-connection-required?) (try (let - [connection (create-connection (get-connection-config is-producer?) (get-tracer-config))] + [connection (create-connection (get-connection-config is-producer?) (get-tracer-config))] (println "Connection created " connection) (doto connection (.addShutdownListener @@ -81,10 +80,10 @@ (rmq/close conn))) (defstate consumer-connection - :start (do (log/info "Creating consumer connection") - (start-connection false)) - :stop (do (log/info "Stopping consume connection") - (stop-connection consumer-connection))) + :start (do (log/info "Creating consumer connection") + (start-connection false)) + :stop (do (log/info "Stopping consume connection") + (stop-connection consumer-connection))) (defstate connection :start (do (log/info "Creating producer connection") diff --git a/src/ziggurat/messaging/consumer.clj b/src/ziggurat/messaging/consumer.clj index b8906052..33e59e50 100644 --- a/src/ziggurat/messaging/consumer.clj +++ b/src/ziggurat/messaging/consumer.clj @@ -5,6 +5,7 @@ [langohr.consumers :as lcons] [taoensso.nippy :as nippy] [ziggurat.config :refer [get-in-config rabbitmq-config]] + [ziggurat.messaging.channel_pool :as cpool] [ziggurat.kafka-consumer.consumer-handler :as ch] [ziggurat.mapper :as mpr] [ziggurat.messaging.connection :refer [consumer-connection]] @@ -18,14 +19,16 @@ (lb/reject ch delivery-tag)) (defn- publish-to-dead-set - [ch delivery-tag topic-entity payload] - (let [{:keys [exchange-name]} (:dead-letter (rabbitmq-config)) - exchange (util/prefixed-queue-name topic-entity exchange-name)] + [delivery-tag topic-entity payload] + (let [ch (.borrowObject cpool/channel-pool) + {:keys [exchange-name]} (:dead-letter (rabbitmq-config)) + exchange (util/prefixed-queue-name topic-entity exchange-name)] (try (lb/publish ch exchange "" payload) (catch Exception e (log/error e "Exception was encountered while publishing to RabbitMQ") - (reject-message ch delivery-tag))))) + (reject-message ch delivery-tag)) + (finally (.returnObject cpool/channel-pool))))) (defn convert-and-ack-message "De-serializes the message payload (`payload`) using `nippy/thaw` and converts it to `MessagePayload`. Acks the message @@ -38,7 +41,7 @@ message) (catch Exception e (report-error e "Error while decoding message, publishing to dead queue...") - (publish-to-dead-set ch delivery-tag topic-entity payload) + (publish-to-dead-set delivery-tag topic-entity payload) (metrics/increment-count ["rabbitmq-message" "conversion"] "failure" {:topic_name (name topic-entity)}) nil))) @@ -56,7 +59,7 @@ (processing-fn message-payload) (ack-message ch delivery-tag) (catch Exception e - (publish-to-dead-set ch delivery-tag topic-entity payload) + (publish-to-dead-set delivery-tag topic-entity payload) (report-error e "Error while processing message-payload from RabbitMQ") (metrics/increment-count ["rabbitmq-message" "process"] "failure" {:topic_name (name topic-entity)})))))) @@ -97,7 +100,7 @@ ([topic-entity channel count processing-fn] (with-open [ch (lch/open consumer-connection)] (doall (for [_ (range count)] - (let [queue-name (construct-queue-name topic-entity channel) + (let [queue-name (construct-queue-name topic-entity channel) [meta payload] (lb/get ch queue-name false)] (when (some? payload) (process-message-from-queue ch meta payload topic-entity processing-fn)))))))) diff --git a/src/ziggurat/messaging/producer.clj b/src/ziggurat/messaging/producer.clj index 8f561776..06181125 100644 --- a/src/ziggurat/messaging/producer.clj +++ b/src/ziggurat/messaging/producer.clj @@ -5,6 +5,7 @@ [langohr.exchange :as le] [langohr.http :as lh] [langohr.queue :as lq] + [ziggurat.messaging.channel_pool :as cpool] [taoensso.nippy :as nippy] [ziggurat.config :refer [config ziggurat-config rabbitmq-config channel-retry-config]] [ziggurat.messaging.connection :refer [connection is-connection-required?]] @@ -53,8 +54,8 @@ (let [host-endpoint (str "http://" (first hosts) ":" (get rmq-config :admin-port 15672)) resp (set-ha-policy-on-host host-endpoint username password ha-policy-body exchange-name queue-name) remaining-hosts (rest hosts)] - (when (and (nil? resp) - (pos? (count remaining-hosts))) + (when (and (nil? resp) + (pos? (count remaining-hosts))) (recur remaining-hosts)))))) (defn- declare-exchange [ch exchange] @@ -110,18 +111,23 @@ (defn- publish-internal [exchange message-payload expiration] (try - (with-open [ch (lch/open connection)] ;; it opens a connection everytime it publishes? - (lb/publish ch exchange "" (nippy/freeze (dissoc message-payload :headers)) - (properties-for-publish expiration (:headers message-payload)))) - false - (catch AlreadyClosedException e - (handle-network-exception e message-payload)) - (catch IOException e - (handle-network-exception e message-payload)) + (let [ch (.borrowObject cpool/channel-pool)] + (try + (lb/publish ch exchange "" (nippy/freeze (dissoc message-payload :headers)) + (properties-for-publish expiration (:headers message-payload))) + false + (catch AlreadyClosedException e + (handle-network-exception e message-payload)) + (catch IOException e + (handle-network-exception e message-payload)) + (catch Exception e + (log/error e "Exception was encountered while publishing to RabbitMQ") + (metrics/increment-count ["rabbitmq" "publish"] "exception" {:topic-entity (name (:topic-entity message-payload))}) + false) + (finally (.returnObject cpool/channel-pool ch)))) (catch Exception e - (log/error e "Exception was encountered while publishing to RabbitMQ") - (metrics/increment-count ["rabbitmq" "publish"] "exception" {:topic-entity (name (:topic-entity message-payload))}) - false))) + (log/error "Exception occurred while borrowing a channel from the pool") + (metrics/increment-count ["rabbitmq" "publish" "channel_borrow"] {:topic-entity (name (:topic-entity message-payload))})))) (defn publish ([exchange message-payload] @@ -204,8 +210,8 @@ "This function return delay exchange name for retry when using flow without channel. It will return exchange name with retry count as suffix if exponential backoff enabled." [topic-entity message-payload] (let [{:keys [exchange-name]} (:delay (rabbitmq-config)) - exchange-name (util/prefixed-queue-name topic-entity exchange-name) - retry-count (-> (ziggurat-config) :retry :count)] + exchange-name (util/prefixed-queue-name topic-entity exchange-name) + retry-count (-> (ziggurat-config) :retry :count)] (if (= :exponential (-> (ziggurat-config) :retry :type)) (let [message-retry-count (:retry-count message-payload) backoff-exponent (get-backoff-exponent retry-count message-retry-count)] @@ -216,8 +222,8 @@ "This function return delay exchange name for retry when using channel flow. It will return exchange name with retry count as suffix if exponential backoff enabled." [topic-entity channel message-payload] (let [{:keys [exchange-name]} (:delay (rabbitmq-config)) - exchange-name (util/prefixed-channel-name topic-entity channel exchange-name) - channel-retry-count (get-channel-retry-count topic-entity channel)] + exchange-name (util/prefixed-channel-name topic-entity channel exchange-name) + channel-retry-count (get-channel-retry-count topic-entity channel)] (if (= :exponential (channel-retry-type topic-entity channel)) (let [message-retry-count (:retry-count message-payload) exponential-backoff (get-backoff-exponent channel-retry-count message-retry-count)] @@ -232,14 +238,14 @@ (defn publish-to-dead-queue [message-payload] (let [{:keys [exchange-name]} (:dead-letter (rabbitmq-config)) - topic-entity (:topic-entity message-payload) - exchange-name (util/prefixed-queue-name topic-entity exchange-name)] + topic-entity (:topic-entity message-payload) + exchange-name (util/prefixed-queue-name topic-entity exchange-name)] (publish exchange-name message-payload))) (defn publish-to-instant-queue [message-payload] (let [{:keys [exchange-name]} (:instant (rabbitmq-config)) - topic-entity (:topic-entity message-payload) - exchange-name (util/prefixed-queue-name topic-entity exchange-name)] + topic-entity (:topic-entity message-payload) + exchange-name (util/prefixed-queue-name topic-entity exchange-name)] (publish exchange-name message-payload))) (defn publish-to-channel-delay-queue [channel message-payload] @@ -250,43 +256,43 @@ (defn publish-to-channel-dead-queue [channel message-payload] (let [{:keys [exchange-name]} (:dead-letter (rabbitmq-config)) - topic-entity (:topic-entity message-payload) - exchange-name (util/prefixed-channel-name topic-entity channel exchange-name)] + topic-entity (:topic-entity message-payload) + exchange-name (util/prefixed-channel-name topic-entity channel exchange-name)] (publish exchange-name message-payload))) (defn publish-to-channel-instant-queue [channel message-payload] (let [{:keys [exchange-name]} (:instant (rabbitmq-config)) - topic-entity (:topic-entity message-payload) - exchange-name (util/prefixed-channel-name topic-entity channel exchange-name)] + topic-entity (:topic-entity message-payload) + exchange-name (util/prefixed-channel-name topic-entity channel exchange-name)] (publish exchange-name message-payload))) (defn retry [{:keys [retry-count] :as message-payload}] (when (-> (ziggurat-config) :retry :enabled) (cond - (nil? retry-count) (publish-to-delay-queue (assoc message-payload :retry-count (dec (-> (ziggurat-config) :retry :count)))) - (pos? retry-count) (publish-to-delay-queue (assoc message-payload :retry-count (dec retry-count))) + (nil? retry-count) (publish-to-delay-queue (assoc message-payload :retry-count (dec (-> (ziggurat-config) :retry :count)))) + (pos? retry-count) (publish-to-delay-queue (assoc message-payload :retry-count (dec retry-count))) (zero? retry-count) (publish-to-dead-queue (assoc message-payload :retry-count (-> (ziggurat-config) :retry :count)))))) (defn retry-for-channel [{:keys [retry-count topic-entity] :as message-payload} channel] (when (channel-retries-enabled topic-entity channel) (cond - (nil? retry-count) (publish-to-channel-delay-queue channel (assoc message-payload :retry-count (dec (get-channel-retry-count topic-entity channel)))) - (pos? retry-count) (publish-to-channel-delay-queue channel (assoc message-payload :retry-count (dec retry-count))) + (nil? retry-count) (publish-to-channel-delay-queue channel (assoc message-payload :retry-count (dec (get-channel-retry-count topic-entity channel)))) + (pos? retry-count) (publish-to-channel-delay-queue channel (assoc message-payload :retry-count (dec retry-count))) (zero? retry-count) (publish-to-channel-dead-queue channel (assoc message-payload :retry-count (get-channel-retry-count topic-entity channel)))))) (defn- make-delay-queue [topic-entity] (let [{:keys [queue-name exchange-name dead-letter-exchange]} (:delay (rabbitmq-config)) - queue-name (delay-queue-name topic-entity queue-name) - exchange-name (util/prefixed-queue-name topic-entity exchange-name) - dead-letter-exchange-name (util/prefixed-queue-name topic-entity dead-letter-exchange)] + queue-name (delay-queue-name topic-entity queue-name) + exchange-name (util/prefixed-queue-name topic-entity exchange-name) + dead-letter-exchange-name (util/prefixed-queue-name topic-entity dead-letter-exchange)] (create-and-bind-queue queue-name exchange-name dead-letter-exchange-name))) (defn- make-delay-queue-with-retry-count [topic-entity retry-count] (let [{:keys [queue-name exchange-name dead-letter-exchange]} (:delay (rabbitmq-config)) - queue-name (delay-queue-name topic-entity queue-name) - exchange-name (util/prefixed-queue-name topic-entity exchange-name) - dead-letter-exchange-name (util/prefixed-queue-name topic-entity dead-letter-exchange) - sequence (min MAX_EXPONENTIAL_RETRIES (inc retry-count))] + queue-name (delay-queue-name topic-entity queue-name) + exchange-name (util/prefixed-queue-name topic-entity exchange-name) + dead-letter-exchange-name (util/prefixed-queue-name topic-entity dead-letter-exchange) + sequence (min MAX_EXPONENTIAL_RETRIES (inc retry-count))] (doseq [s (range 1 sequence)] (create-and-bind-queue (util/prefixed-queue-name queue-name s) (util/prefixed-queue-name exchange-name s) dead-letter-exchange-name)))) @@ -298,8 +304,8 @@ (defn- make-queue [topic-identifier queue-type] (let [{:keys [queue-name exchange-name]} (queue-type (rabbitmq-config)) - queue-name (util/prefixed-queue-name topic-identifier queue-name) - exchange-name (util/prefixed-queue-name topic-identifier exchange-name)] + queue-name (util/prefixed-queue-name topic-identifier queue-name) + exchange-name (util/prefixed-queue-name topic-identifier exchange-name)] (create-and-bind-queue queue-name exchange-name))) (defn- make-channel-queue [topic-entity channel-name queue-type] @@ -317,15 +323,15 @@ "Please use it only after understanding its risks and implications." "Its contract can change in the future releases of Ziggurat.") (make-channel-delay-queue-with-retry-count topic-entity channel (get-channel-retry-count topic-entity channel))) - (= :linear channel-retry-type) (make-channel-delay-queue topic-entity channel) - (nil? channel-retry-type) (do - (log/warn "[Deprecation Notice]: Please note that the configuration for channel retries has changed." - "Please look at the upgrade guide for details: https://github.com/gojek/ziggurat/wiki/Upgrade-guide" - "Use :type to specify the type of retry mechanism in the channel config.") - (make-channel-delay-queue topic-entity channel)) - :else (do - (log/warn "Incorrect keyword for type passed, falling back to linear backoff for channel: " channel) - (make-channel-delay-queue topic-entity channel))))))) + (= :linear channel-retry-type) (make-channel-delay-queue topic-entity channel) + (nil? channel-retry-type) (do + (log/warn "[Deprecation Notice]: Please note that the configuration for channel retries has changed." + "Please look at the upgrade guide for details: https://github.com/gojek/ziggurat/wiki/Upgrade-guide" + "Use :type to specify the type of retry mechanism in the channel config.") + (make-channel-delay-queue topic-entity channel)) + :else (do + (log/warn "Incorrect keyword for type passed, falling back to linear backoff for channel: " channel) + (make-channel-delay-queue topic-entity channel))))))) (defn make-queues [routes] (when (is-connection-required?) @@ -342,12 +348,12 @@ "Please use it only after understanding its risks and implications." "Its contract can change in the future releases of Ziggurat.") (make-delay-queue-with-retry-count topic-entity (-> (ziggurat-config) :retry :count))) - (= :linear retry-type) (make-delay-queue topic-entity) - (nil? retry-type) (do - (log/warn "[Deprecation Notice]: Please note that the configuration for retries has changed." - "Please look at the upgrade guide for details: https://github.com/gojek/ziggurat/wiki/Upgrade-guide" - "Use :type to specify the type of retry mechanism in the config.") - (make-delay-queue topic-entity)) - :else (do - (log/warn "Incorrect keyword for type passed, falling back to linear backoff for topic Entity: " topic-entity) - (make-delay-queue topic-entity)))))))) + (= :linear retry-type) (make-delay-queue topic-entity) + (nil? retry-type) (do + (log/warn "[Deprecation Notice]: Please note that the configuration for retries has changed." + "Please look at the upgrade guide for details: https://github.com/gojek/ziggurat/wiki/Upgrade-guide" + "Use :type to specify the type of retry mechanism in the config.") + (make-delay-queue topic-entity)) + :else (do + (log/warn "Incorrect keyword for type passed, falling back to linear backoff for topic Entity: " topic-entity) + (make-delay-queue topic-entity)))))))) diff --git a/test/ziggurat/messaging/connection_test.clj b/test/ziggurat/messaging/connection_test.clj index 8b0c22a3..37cad06d 100644 --- a/test/ziggurat/messaging/connection_test.clj +++ b/test/ziggurat/messaging/connection_test.clj @@ -21,10 +21,10 @@ (reset! executor-present? true)) (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} - :tracer {:enabled false}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} + :tracer {:enabled false}))] (-> (mount/only #{#'consumer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -41,10 +41,10 @@ (reset! executor-present? true)) (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} - :tracer {:enabled false}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} + :tracer {:enabled false}))] (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -88,10 +88,10 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} - :tracer {:enabled false}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} + :tracer {:enabled false}))] (-> (mount/only #{#'consumer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -150,7 +150,6 @@ (mount/stop #'consumer-connection) (is (not @rmq-connect-called?))))) - (testing "if retry is disabled and channels are present it should create connection" (let [rmq-connect-called? (atom false) orig-rmq-connect rmq/connect @@ -217,9 +216,9 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}}))] (-> (mount/only #{#'consumer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -236,7 +235,7 @@ (reset! create-connect-called? true) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled true}))] + :retry {:enabled true}))] (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -252,7 +251,7 @@ (reset! create-connect-called? true) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false}))] + :retry {:enabled false}))] (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -271,7 +270,7 @@ (reset! create-connect-called? true) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false}))] + :retry {:enabled false}))] (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -287,10 +286,10 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 5} - :channel-2 {:worker-count 10}}}}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 5} + :channel-2 {:worker-count 10}}}}))] (-> (mount/only #{#'consumer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -305,9 +304,9 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {}}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {}}))] (mount/start (mount/only [#'consumer-connection])) (mount/stop #'consumer-connection) (is (= @thread-count 4))))) @@ -320,10 +319,10 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}} - :default-1 {:channels {:channel-1 {:worker-count 8}}}}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}} + :default-1 {:channels {:channel-1 {:worker-count 8}}}}))] (mount/start (mount/only [#'consumer-connection])) (mount/stop #'consumer-connection) (is (= @thread-count 26)))))) From 118abc403df1f4415486eb224fc52c502bd6c770 Mon Sep 17 00:00:00 2001 From: "shubhang.balkundi" Date: Wed, 6 Apr 2022 11:50:11 +0530 Subject: [PATCH 07/24] Adds tests for channel_pool --- .../channel_pool/RabbitMQChannelFactory.java | 17 +- src/ziggurat/config.clj | 2 +- src/ziggurat/messaging/channel_pool.clj | 50 +++-- src/ziggurat/messaging/connection.clj | 38 ++-- src/ziggurat/messaging/consumer.clj | 2 +- src/ziggurat/messaging/producer.clj | 3 +- test/ziggurat/fixtures.clj | 23 +- test/ziggurat/messaging/channel_pool_test.clj | 62 ++++++ test/ziggurat/messaging/connection_test.clj | 206 +++++++++--------- test/ziggurat/messaging/producer_test.clj | 86 ++++---- 10 files changed, 285 insertions(+), 204 deletions(-) create mode 100644 test/ziggurat/messaging/channel_pool_test.clj diff --git a/src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java b/src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java index e9358a1f..018eef40 100644 --- a/src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java +++ b/src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java @@ -20,15 +20,24 @@ public RabbitMQChannelFactory(Connection connection) { @Override public Channel create() throws Exception { - log.info("Creating a new channel"); - return connection.createChannel(); + Channel channel = connection.createChannel(); + log.info("Created a new channel with id: " + channel.getChannelNumber()); + return channel; } + @Override - public PooledObject wrap(Channel channel) { - return new DefaultPooledObject(channel); + public boolean validateObject(PooledObject p) { + super.validateObject(p); + return p.getObject().isOpen(); } + @Override + public PooledObject wrap(Channel obj) { + return new DefaultPooledObject<>(obj); + } + + @Override public void destroyObject(PooledObject p, DestroyMode destroyMode) throws Exception { super.destroyObject(p, destroyMode); diff --git a/src/ziggurat/config.clj b/src/ziggurat/config.clj index a7a9053e..e4db88aa 100644 --- a/src/ziggurat/config.clj +++ b/src/ziggurat/config.clj @@ -12,7 +12,7 @@ ^{:static true} [getIn [java.lang.Iterable] Object]] :name tech.gojek.ziggurat.internal.Config)) -(def config-file "config.edn") +(def config-file "config.test.edn") (def default-config {:ziggurat {:nrepl-server {:port 70171} :statsd {:port 8125 diff --git a/src/ziggurat/messaging/channel_pool.clj b/src/ziggurat/messaging/channel_pool.clj index f1a5985c..91e2987b 100644 --- a/src/ziggurat/messaging/channel_pool.clj +++ b/src/ziggurat/messaging/channel_pool.clj @@ -2,34 +2,36 @@ (:require [mount.core :refer [defstate]] [ziggurat.config :as zc] [ziggurat.messaging.connection :as c] + [cambium.core :as clog] [clojure.tools.logging :as log]) (:import (com.rabbitmq.client Connection) (org.apache.commons.pool2.impl GenericObjectPool GenericObjectPoolConfig) - (gojek.rabbitmq.channel_pool RabbitMQChannelFactory) - (java.time Duration))) - -(def default-config - {:max-wait-ms 5000 - :min-idle 8 - :max-idle 8}) + (java.time Duration) + (gojek.rabbitmq.channel_pool RabbitMQChannelFactory))) (defn calc-total-thread-count [] - (let [buffer-threads 10 - channel-thread-count (c/total-thread-count) - - stream-router-config (get (zc/ziggurat-config) :stream-router) - stream-threads-count (reduce (fn [sum config] - (+ sum (:stream-threads-count config))) 0 (vals stream-router-config))] - (+ stream-threads-count channel-thread-count buffer-threads))) + (let [rmq-thread-count (c/total-thread-count) + stream-router-config (get (zc/ziggurat-config) :stream-router) + batch-routes-config (get (zc/ziggurat-config) :batch-routes) + batch-consumer-thread-count (reduce (fn [sum config] + (+ sum (:thread-count config))) 0 (vals batch-routes-config)) + stream-thread-count (reduce (fn [sum config] + (+ sum (:stream-threads-count config))) 0 (vals stream-router-config))] + (clog/info {:channel-threads rmq-thread-count + :batch-consumer-threads batch-consumer-thread-count + :stream-threads stream-thread-count} "Thread counts") + (+ stream-thread-count rmq-thread-count batch-consumer-thread-count))) (defn create-object-pool-config [config] - (let [merged-config (merge config default-config)] - (println merged-config) + (let [standby-size 10 + total-thread-count (calc-total-thread-count) + merged-config (merge {:max-wait-ms 5000 :min-idle standby-size :max-idle total-thread-count} config)] (doto (GenericObjectPoolConfig.) - (.setMaxWait (Duration/ofMillis (get merged-config :max-wait-ms))) - (.setMinIdle (get merged-config :min-idle)) - (.setMaxIdle (get merged-config :max-idle)) - (.setMaxTotal (calc-total-thread-count))))) + (.setMaxWait (Duration/ofMillis (:max-wait-ms merged-config))) + (.setMinIdle (:min-idle merged-config)) + (.setMaxIdle (:max-idle merged-config)) + (.setMaxTotal (+ (:min-idle merged-config) total-thread-count)) + (.setTestOnBorrow true)))) (defn create-channel-pool [^Connection connection] (let [pool-config (create-object-pool-config (get-in zc/ziggurat-config [:rabbit-mq-connection :channel-pool])) @@ -37,7 +39,7 @@ rmq-chan-pool)) (defstate channel-pool - :start (do (log/info "Creating channel pool") - (create-channel-pool c/connection)) - :stop (do (log/info "Stopping channel pool") - (.close channel-pool))) + :start (do (log/info "Creating channel pool") + (create-channel-pool c/connection)) + :stop (do (log/info "Stopping channel pool") + (.close channel-pool))) diff --git a/src/ziggurat/messaging/connection.clj b/src/ziggurat/messaging/connection.clj index 24ac8cfc..77aa059d 100644 --- a/src/ziggurat/messaging/connection.clj +++ b/src/ziggurat/messaging/connection.clj @@ -29,10 +29,14 @@ (+ sum (:worker-count channel-config))) 0 channels)) (defn total-thread-count [] - (let [stream-routes (:stream-router (ziggurat-config)) - worker-count (get-in (ziggurat-config) [:jobs :instant :worker-count])] + (let [stream-routes (:stream-router (ziggurat-config)) + batch-route-count (count (:batch-routes (ziggurat-config))) + worker-count (get-in (ziggurat-config) [:jobs :instant :worker-count]) + batch-routes-instant-workers (* batch-route-count worker-count)] (reduce (fn [sum [_ route-config]] - (+ sum (channel-threads (:channels route-config)) worker-count)) 0 stream-routes))) + (+ sum (channel-threads (:channels route-config)) worker-count)) + batch-routes-instant-workers + stream-routes))) (defn- create-traced-connection [config] (let [connection-factory (TracingConnectionFactory. tracer)] @@ -54,7 +58,7 @@ (if is-producer? (:rabbit-mq-connection (ziggurat-config)) (assoc (:rabbit-mq-connection (ziggurat-config)) - :executor (Executors/newFixedThreadPool (total-thread-count))))) + :executor (Executors/newFixedThreadPool (total-thread-count))))) (defn- start-connection [is-producer?] @@ -62,14 +66,14 @@ (when (is-connection-required?) (try (let - [connection (create-connection (get-connection-config is-producer?) (get-tracer-config))] + [connection (create-connection (get-connection-config is-producer?) (get-tracer-config))] (println "Connection created " connection) (doto connection (.addShutdownListener - (reify ShutdownListener - (shutdownCompleted [_ cause] - (when-not (.isInitiatedByApplication cause) - (log/error cause "RabbitMQ connection shut down due to error"))))))) + (reify ShutdownListener + (shutdownCompleted [_ cause] + (when-not (.isInitiatedByApplication cause) + (log/error cause "RabbitMQ connection shut down due to error"))))))) (catch Exception e (report-error e "Error while starting RabbitMQ connection") (throw e))))) @@ -80,13 +84,13 @@ (rmq/close conn))) (defstate consumer-connection - :start (do (log/info "Creating consumer connection") - (start-connection false)) - :stop (do (log/info "Stopping consume connection") - (stop-connection consumer-connection))) + :start (do (log/info "Creating consumer connection") + (start-connection false)) + :stop (do (log/info "Stopping consume connection") + (stop-connection consumer-connection))) (defstate connection - :start (do (log/info "Creating producer connection") - (start-connection true)) - :stop (do (log/info "Stopping producer connection") - (stop-connection connection))) + :start (do (log/info "Creating producer connection") + (start-connection true)) + :stop (do (log/info "Stopping producer connection") + (stop-connection connection))) diff --git a/src/ziggurat/messaging/consumer.clj b/src/ziggurat/messaging/consumer.clj index 33e59e50..c7769b9a 100644 --- a/src/ziggurat/messaging/consumer.clj +++ b/src/ziggurat/messaging/consumer.clj @@ -28,7 +28,7 @@ (catch Exception e (log/error e "Exception was encountered while publishing to RabbitMQ") (reject-message ch delivery-tag)) - (finally (.returnObject cpool/channel-pool))))) + (finally (.returnObject cpool/channel-pool ch))))) (defn convert-and-ack-message "De-serializes the message payload (`payload`) using `nippy/thaw` and converts it to `MessagePayload`. Acks the message diff --git a/src/ziggurat/messaging/producer.clj b/src/ziggurat/messaging/producer.clj index 06181125..02981a38 100644 --- a/src/ziggurat/messaging/producer.clj +++ b/src/ziggurat/messaging/producer.clj @@ -127,7 +127,8 @@ (finally (.returnObject cpool/channel-pool ch)))) (catch Exception e (log/error "Exception occurred while borrowing a channel from the pool") - (metrics/increment-count ["rabbitmq" "publish" "channel_borrow"] {:topic-entity (name (:topic-entity message-payload))})))) + (metrics/increment-count ["rabbitmq" "publish" "channel_borrow"] {:topic-entity (name (:topic-entity message-payload))}) + true))) (defn publish ([exchange message-payload] diff --git a/test/ziggurat/fixtures.clj b/test/ziggurat/fixtures.clj index d1685d81..415a8ff0 100644 --- a/test/ziggurat/fixtures.clj +++ b/test/ziggurat/fixtures.clj @@ -7,6 +7,7 @@ [ziggurat.config :as config] [ziggurat.kafka-consumer.executor-service :refer [thread-pool]] [ziggurat.messaging.connection :refer [connection consumer-connection]] + [ziggurat.messaging.channel_pool :refer [channel-pool]] [ziggurat.messaging.producer :as pr] [ziggurat.messaging.util :as util] [ziggurat.metrics :as metrics] @@ -18,10 +19,10 @@ (org.apache.kafka.clients.consumer ConsumerConfig) (org.apache.kafka.clients.producer ProducerConfig)) (:gen-class - :methods [^{:static true} [mountConfig [] void] - ^{:static true} [mountProducer [] void] - ^{:static true} [unmountAll [] void]] - :name tech.gojek.ziggurat.internal.test.Fixtures)) + :methods [^{:static true} [mountConfig [] void] + ^{:static true} [mountProducer [] void] + ^{:static true} [unmountAll [] void]] + :name tech.gojek.ziggurat.internal.test.Fixtures)) (def test-config-file-name "config.test.edn") @@ -64,6 +65,8 @@ (f) (mount/stop #'metrics/statsd-reporter)) + + (defn mount-tracer [] (with-redefs [tracer/create-tracer (fn [] (MockTracer.))] (-> (mount/only [#'tracer/tracer]) @@ -119,18 +122,18 @@ (mount-tracer) (-> - (mount/only [#'connection #'consumer-connection]) - (mount/with-args {:stream-routes stream-routes}) - (mount/start)) + (mount/only [#'connection #'consumer-connection #'channel-pool]) + (mount/with-args {:stream-routes stream-routes}) + (mount/start)) (f) (mount/stop))) (defn with-start-server* [stream-routes f] (mount-config) (-> - (mount/only [#'server]) - (mount/with-args {:stream-routes stream-routes}) - (mount/start)) + (mount/only [#'server]) + (mount/with-args {:stream-routes stream-routes}) + (mount/start)) (f) (mount/stop)) diff --git a/test/ziggurat/messaging/channel_pool_test.clj b/test/ziggurat/messaging/channel_pool_test.clj new file mode 100644 index 00000000..b00fc648 --- /dev/null +++ b/test/ziggurat/messaging/channel_pool_test.clj @@ -0,0 +1,62 @@ +(ns ziggurat.messaging.channel-pool-test + (:require [clojure.test :refer :all] + [ziggurat.config :as zc] + [ziggurat.messaging.channel_pool :as cpool] + [ziggurat.messaging.connection :refer [connection]] + [ziggurat.fixtures :as fix] + [clojure.tools.logging :as log]) + (:import (org.apache.commons.pool2.impl GenericObjectPoolConfig GenericObjectPool) + (java.time Duration) + (com.rabbitmq.client Channel))) + +(use-fixtures :once (join-fixtures [fix/mount-only-config + fix/mount-config-with-tracer])) + + + +(deftest calc-total-threads-test + (testing "it should calculate the total threads configured for RabbitMQ, Kafka streams and Batch consumers" + (let [expected-count 44 + actual-count (cpool/calc-total-thread-count)] + (is (= expected-count actual-count))))) + +(deftest create-object-pool-config-test + (testing "it should create a PoolConfig with default values" + (let [expected-config {:min-idle 10 :max-idle 44 :max-total 54 :max-wait-ms 5000} + pool-config-object ^GenericObjectPoolConfig (cpool/create-object-pool-config {}) + min-idle (.getMinIdle pool-config-object) + max-idle (.getMaxIdle pool-config-object) + max-wait-ms (.getMaxWaitDuration pool-config-object) + test-on-borrow (.getTestOnBorrow pool-config-object) + max-total (.getMaxTotal pool-config-object)] + (is (= (:min-idle expected-config) min-idle)) + (is (= (:max-idle expected-config) max-idle)) + (is test-on-borrow) + (is (= (Duration/ofMillis (:max-wait-ms expected-config)) max-wait-ms)) + (is (= (:max-total expected-config) max-total)))) + (testing "it should override the default config with the user provided config" + (let [expected-config {:min-idle 5 :max-idle 15 :max-total 49 :max-wait-ms 1000} + user-config {:min-idle 5 :max-idle 15 :max-wait-ms 1000} + pool-config-object ^GenericObjectPoolConfig (cpool/create-object-pool-config user-config) + min-idle (.getMinIdle pool-config-object) + max-idle (.getMaxIdle pool-config-object) + test-on-borrow (.getTestOnBorrow pool-config-object) + max-total (.getMaxTotal pool-config-object) + max-wait-ms (.getMaxWaitDuration pool-config-object)] + (is (= (:min-idle expected-config) min-idle)) + (is (= (:max-idle expected-config) max-idle)) + (is (= (Duration/ofMillis (:max-wait-ms expected-config)) max-wait-ms)) + (is test-on-borrow) + (is (= (:max-total expected-config) max-total))))) + +(deftest pool-borrow-return-test + (testing "it should invalidate a closed channel and return a new channel on borrow" + (mount.core/start #'connection) + (let [channel-pool ^GenericObjectPool (cpool/create-channel-pool connection) + rmq-chan ^Channel (.borrowObject channel-pool) + _ (.close rmq-chan) + _ (.returnObject channel-pool rmq-chan) + rmq-chan-2 ^Channel (.borrowObject channel-pool)] + (is (.isOpen rmq-chan-2))))) + + diff --git a/test/ziggurat/messaging/connection_test.clj b/test/ziggurat/messaging/connection_test.clj index 37cad06d..0967216d 100644 --- a/test/ziggurat/messaging/connection_test.clj +++ b/test/ziggurat/messaging/connection_test.clj @@ -11,40 +11,40 @@ (deftest connection-test (testing "creates thread-pool for consumer connection" - (let [executor-present? (atom false) - orig-rmq-connect rmq/connect - ziggurat-config (config/ziggurat-config) - stream-routes {:default {:handler-fn (constantly :channel-1) - :channel-1 (constantly :success)}}] - (with-redefs [rmq/connect (fn [provided-config] - (when (some? (:executor provided-config)) - (reset! executor-present? true)) - (orig-rmq-connect provided-config)) + (let [executor-present? (atom false) + orig-rmq-connect rmq/connect + ziggurat-config (config/ziggurat-config) + stream-routes {:default {:handler-fn (constantly :channel-1) + :channel-1 (constantly :success)}}] + (with-redefs [rmq/connect (fn [provided-config] + (when (some? (:executor provided-config)) + (reset! executor-present? true)) + (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} - :tracer {:enabled false}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} + :tracer {:enabled false}))] (-> (mount/only #{#'consumer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) (mount/stop #'consumer-connection) (is @executor-present?)))) (testing "does not create thread-pool for producer connection" - (let [executor-present? (atom false) - orig-rmq-connect rmq/connect - ziggurat-config (config/ziggurat-config) - stream-routes {:default {:handler-fn (constantly :channel-1) - :channel-1 (constantly :success)}}] - (with-redefs [rmq/connect (fn [provided-config] - (when (some? (:executor provided-config)) - (reset! executor-present? true)) - (orig-rmq-connect provided-config)) + (let [executor-present? (atom false) + orig-rmq-connect rmq/connect + ziggurat-config (config/ziggurat-config) + stream-routes {:default {:handler-fn (constantly :channel-1) + :channel-1 (constantly :success)}}] + (with-redefs [rmq/connect (fn [provided-config] + (when (some? (:executor provided-config)) + (reset! executor-present? true)) + (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} - :tracer {:enabled false}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} + :tracer {:enabled false}))] (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -59,21 +59,21 @@ ziggurat-config (config/ziggurat-config) stream-routes {:default {:handler-fn (constantly :channel-1) :channel-1 (constantly :success)}}] - (with-redefs [rmq/connect (fn [provided-config] - (reset! rmq-connect-called? true) - (reset! thread-count (.getCorePoolSize (:executor provided-config))) - (orig-rmq-connect provided-config)) + (with-redefs [rmq/connect (fn [provided-config] + (reset! rmq-connect-called? true) + (reset! thread-count (.getCorePoolSize (:executor provided-config))) + (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} - :tracer {}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} + :tracer {}))] (-> (mount/only #{#'consumer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) (mount/stop #'consumer-connection) (is @rmq-connect-called?) - (is (= 14 @thread-count))))) + (is (= 22 @thread-count))))) (testing "[consumer-connection] if retry is enabled and channels are not present it should create connection" (let [thread-count (atom 0) @@ -81,35 +81,35 @@ orig-rmq-connect rmq/connect ziggurat-config (config/ziggurat-config) stream-routes {:default {:handler-fn (constantly :success)}}] - (with-redefs [rmq/connect (fn [provided-config] - (println "Tracer Disabled ********** " provided-config) - (println "Tracer Disabled ********** " (.getCorePoolSize (:executor provided-config))) - (reset! rmq-connect-called? true) - (reset! thread-count (.getCorePoolSize (:executor provided-config))) - (orig-rmq-connect provided-config)) + (with-redefs [rmq/connect (fn [provided-config] + (println "Tracer Disabled ********** " provided-config) + (println "Tracer Disabled ********** " (.getCorePoolSize (:executor provided-config))) + (reset! rmq-connect-called? true) + (reset! thread-count (.getCorePoolSize (:executor provided-config))) + (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} - :tracer {:enabled false}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} + :tracer {:enabled false}))] (-> (mount/only #{#'consumer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) (mount/stop #'consumer-connection) (is @rmq-connect-called?) - (is (= 14 @thread-count))))) + (is (= 22 @thread-count))))) (testing "if retry is enabled and channels are not present it should create connection" (let [rmq-connect-called? (atom false) orig-rmq-connect rmq/connect ziggurat-config (config/ziggurat-config) stream-routes {:default {:handler-fn (constantly :success)}}] - (with-redefs [rmq/connect (fn [provided-config] - (reset! rmq-connect-called? true) - (orig-rmq-connect provided-config)) + (with-redefs [rmq/connect (fn [provided-config] + (reset! rmq-connect-called? true) + (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled true} - :tracer {:enabled false}))] + :retry {:enabled true} + :tracer {:enabled false}))] (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -121,9 +121,9 @@ orig-rmq-connect rmq/connect ziggurat-config (config/ziggurat-config) stream-routes {:default {:handler-fn (constantly :success)}}] - (with-redefs [rmq/connect (fn [provided-config] - (reset! rmq-connect-called? true) - (orig-rmq-connect provided-config)) + (with-redefs [rmq/connect (fn [provided-config] + (reset! rmq-connect-called? true) + (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (-> ziggurat-config (assoc :retry {:enabled false}) (dissoc :tracer)))] @@ -138,9 +138,9 @@ orig-rmq-connect rmq/connect ziggurat-config (config/ziggurat-config) stream-routes {:default {:handler-fn (constantly :success)}}] - (with-redefs [rmq/connect (fn [provided-config] - (reset! rmq-connect-called? true) - (orig-rmq-connect provided-config)) + (with-redefs [rmq/connect (fn [provided-config] + (reset! rmq-connect-called? true) + (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (-> ziggurat-config (assoc :retry {:enabled false}) (dissoc :tracer)))] @@ -158,12 +158,12 @@ :channel-1 (constantly :success)} :default-1 {:handler-fn (constantly :channel-3) :channel-3 (constantly :success)}}] - (with-redefs [rmq/connect (fn [provided-config] - (reset! rmq-connect-called? true) - (orig-rmq-connect provided-config)) + (with-redefs [rmq/connect (fn [provided-config] + (reset! rmq-connect-called? true) + (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false} - :tracer {:enabled false}))] + :retry {:enabled false} + :tracer {:enabled false}))] (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -174,34 +174,34 @@ (let [thread-count (atom 0) orig-rmq-connect rmq/connect ziggurat-config (config/ziggurat-config)] - (with-redefs [rmq/connect (fn [provided-config] - (reset! thread-count (.getCorePoolSize (:executor provided-config))) - (orig-rmq-connect provided-config)) + (with-redefs [rmq/connect (fn [provided-config] + (reset! thread-count (.getCorePoolSize (:executor provided-config))) + (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {}} - :tracer {:enabled false}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {}} + :tracer {:enabled false}))] (mount/start (mount/only [#'consumer-connection])) (mount/stop #'consumer-connection) - (is (= @thread-count 4))))) + (is (= 12 @thread-count))))) (testing "should provide the correct number of threads for the thread pool for multiple stream routes" (let [thread-count (atom 0) orig-rmq-connect rmq/connect ziggurat-config (config/ziggurat-config)] - (with-redefs [rmq/connect (fn [provided-config] - (reset! thread-count (.getCorePoolSize (:executor provided-config))) - (orig-rmq-connect provided-config)) + (with-redefs [rmq/connect (fn [provided-config] + (reset! thread-count (.getCorePoolSize (:executor provided-config))) + (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}} - :default-1 {:channels {:channel-1 {:worker-count 8}}}} - :tracer {:enabled false}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}} + :default-1 {:channels {:channel-1 {:worker-count 8}}}} + :tracer {:enabled false}))] (mount/start (mount/only [#'consumer-connection])) (mount/stop #'consumer-connection) - (is (= @thread-count 26)))))) + (is (= 34 @thread-count)))))) (deftest start-connection-test-with-tracer-enabled (testing "[consumer-connection] should provide the correct number of threads for the thread pool if channels are present" @@ -216,14 +216,14 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}}))] (-> (mount/only #{#'consumer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) (mount/stop #'consumer-connection) - (is (= @thread-count 14)) + (is (= 22 @thread-count)) (is @create-connect-called?)))) (testing "if retry is enabled and channels are not present it should create connection" @@ -235,7 +235,7 @@ (reset! create-connect-called? true) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled true}))] + :retry {:enabled true}))] (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -251,7 +251,7 @@ (reset! create-connect-called? true) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false}))] + :retry {:enabled false}))] (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -270,7 +270,7 @@ (reset! create-connect-called? true) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false}))] + :retry {:enabled false}))] (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -286,15 +286,15 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 5} - :channel-2 {:worker-count 10}}}}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 5} + :channel-2 {:worker-count 10}}}}))] (-> (mount/only #{#'consumer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) (mount/stop #'consumer-connection) - (is (= @thread-count 19))))) + (is (= 27 @thread-count))))) (testing "should provide the correct number of threads for the thread pool when channels are not present" (let [thread-count (atom 0) @@ -304,12 +304,12 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {}}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {}}))] (mount/start (mount/only [#'consumer-connection])) (mount/stop #'consumer-connection) - (is (= @thread-count 4))))) + (is (= 12 @thread-count))))) (testing "should provide the correct number of threads for the thread pool for multiple stream routes" (let [thread-count (atom 0) @@ -319,21 +319,21 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}} - :default-1 {:channels {:channel-1 {:worker-count 8}}}}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}} + :default-1 {:channels {:channel-1 {:worker-count 8}}}}))] (mount/start (mount/only [#'consumer-connection])) (mount/stop #'consumer-connection) - (is (= @thread-count 26)))))) + (is (= 34 @thread-count)))))) (deftest start-connection-test-with-errors (testing "if rabbitmq connect throws an error, it gets reported" - (let [stream-routes {:default {:handler-fn (constantly :success)}} - report-fn-called? (atom false)] + (let [stream-routes {:default {:handler-fn (constantly :success)}} + report-fn-called? (atom false)] (with-redefs [create-connection (fn [_ _] (throw (Exception. "Error"))) - report-error (fn [_ _] (reset! report-fn-called? true))] + report-error (fn [_ _] (reset! report-fn-called? true))] (try (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) diff --git a/test/ziggurat/messaging/producer_test.clj b/test/ziggurat/messaging/producer_test.clj index 77f4145f..2d5c05a5 100644 --- a/test/ziggurat/messaging/producer_test.clj +++ b/test/ziggurat/messaging/producer_test.clj @@ -71,14 +71,14 @@ (testing "message in channel will be retried in delay queue with suffix 1 if message retry-count exceeds retry count in channel config" (with-redefs [ziggurat-config (constantly (assoc (ziggurat-config) - :stream-router - {:default - {:channels - {:exponential-retry - {:retry {:count 5 - :enabled true - :type :exponential - :queue-timeout-ms 1000}}}}}))] + :stream-router + {:default + {:channels + {:exponential-retry + {:retry {:count 5 + :enabled true + :type :exponential + :queue-timeout-ms 1000}}}}}))] (fix/with-queues {:default {:handler-fn #(constantly nil) :exponential-retry #(constantly nil)}} @@ -91,9 +91,9 @@ (testing "message in channel will be retried with linear queue timeout" (with-redefs [ziggurat-config (constantly (assoc (ziggurat-config) - :stream-router {:default {:channels {:linear-retry {:retry {:count 5 - :enabled true - :queue-timeout-ms 2000}}}}}))] + :stream-router {:default {:channels {:linear-retry {:retry {:count 5 + :enabled true + :queue-timeout-ms 2000}}}}}))] (fix/with-queues {:default {:handler-fn #(constantly nil) :linear-retry #(constantly nil)}} @@ -113,10 +113,10 @@ (testing "message in channel will be retried with exponential timeout calculated from channel specific queue-timeout-ms value" (with-redefs [ziggurat-config (constantly (assoc (ziggurat-config) - :stream-router {:default {:channels {:exponential-retry {:retry {:count 5 - :enabled true - :type :exponential - :queue-timeout-ms 1000}}}}}))] + :stream-router {:default {:channels {:exponential-retry {:retry {:count 5 + :enabled true + :type :exponential + :queue-timeout-ms 1000}}}}}))] (fix/with-queues {:default {:handler-fn #(constantly nil) :exponential-retry #(constantly nil)}} @@ -232,9 +232,9 @@ (deftest retry-with-exponential-backoff-test (testing "message will publish to delay with retry count queue when exponential backoff enabled" (with-redefs [ziggurat-config (constantly (assoc (ziggurat-config) - :retry {:count 5 - :enabled true - :type :exponential}))] + :retry {:count 5 + :enabled true + :type :exponential}))] (testing "message with no retry count will publish to delay queue with suffix 1" (fix/with-queues {:default {:handler-fn #(constantly nil)}} @@ -274,8 +274,8 @@ (let [ziggurat-config (ziggurat-config)] (testing "When retries are enabled" (with-redefs [config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled true - :type :linear}))] + :retry {:enabled true + :type :linear}))] (testing "it does not create queues when stream-routes are not passed" (let [counter (atom 0)] (with-redefs [producer/create-and-bind-queue (fn @@ -436,7 +436,7 @@ (testing "when retries are disabled" (with-redefs [config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false}))] + :retry {:enabled false}))] (testing "it does not create queues when stream-routes are not passed" (let [counter (atom 0)] (with-redefs [producer/create-and-bind-queue (fn @@ -477,8 +477,8 @@ (testing "when retries are disabled" (with-redefs [config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false} - :stream-router {:default {:channels {:channel-1 {:retry {:enabled false}}}}}))] + :retry {:enabled false} + :stream-router {:default {:channels {:channel-1 {:retry {:enabled false}}}}}))] (testing "it creates instant queues with topic entity for channels only" (with-open [ch (lch/open connection)] @@ -583,50 +583,50 @@ (testing "when retries are enabled" (let [channel :linear-retry] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :stream-router {topic-entity {:channels {channel {:retry {:count 5 - :enabled true - :queue-timeout-ms 2000}}}}}))] + :stream-router {topic-entity {:channels {channel {:retry {:count 5 + :enabled true + :queue-timeout-ms 2000}}}}}))] (is (= 2000 (producer/get-channel-queue-timeout-ms topic-entity channel message)))))) (testing "when exponential backoff are enabled and channel queue timeout defined" (let [channel :exponential-retry] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :stream-router {topic-entity {:channels {channel {:retry {:count 5 - :enabled true - :type :exponential - :queue-timeout-ms 1000}}}}}))] + :stream-router {topic-entity {:channels {channel {:retry {:count 5 + :enabled true + :type :exponential + :queue-timeout-ms 1000}}}}}))] (is (= 7000 (producer/get-channel-queue-timeout-ms topic-entity channel message)))))) (testing "when exponential backoff are enabled and channel queue timeout is not defined" (let [channel :exponential-retry] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :stream-router {topic-entity {:channels {channel {:retry {:count 5 - :enabled true - :type :exponential}}}}}))] + :stream-router {topic-entity {:channels {channel {:retry {:count 5 + :enabled true + :type :exponential}}}}}))] (is (= 700 (producer/get-channel-queue-timeout-ms topic-entity channel message)))))))) (deftest get-queue-timeout-ms-test (testing "when exponential retries are enabled" (let [message (assoc message-payload :retry-count 2)] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :retry {:enabled true - :count 5 - :type :exponential}))] + :retry {:enabled true + :count 5 + :type :exponential}))] (is (= 700 (producer/get-queue-timeout-ms message)))))) (testing "when exponential retries are enabled and retry-count exceeds 25, the max possible timeouts are calculated using 25 as the retry-count" (let [message (assoc message-payload :retry-count 20)] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :retry {:enabled true - :count 50 - :type :exponential}))] + :retry {:enabled true + :count 50 + :type :exponential}))] ;; For 25 max exponential retries, exponent comes to 25-20=5, which makes timeout = 100*(2^5-1) = 3100 (is (= 3100 (producer/get-queue-timeout-ms message)))))) (testing "when exponential retries are enabled with total retries as 25 and if the message has already been retried 24 times, then the queue-timeout is calculated without any failure" (let [message (assoc message-payload :retry-count 1)] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :retry {:enabled true - :count 25 - :type :exponential} - :rabbit-mq {:delay {:queue-timeout-ms 5000}}))] + :retry {:enabled true + :count 25 + :type :exponential} + :rabbit-mq {:delay {:queue-timeout-ms 5000}}))] ;; For 25 max exponential retries, exponent comes to 25-1=24, which makes timeout = 5000*(2^24-1) = 83886075000 (is (= 83886075000 (producer/get-queue-timeout-ms message))))))) From aa6099310ec3c13d2b01a098444680454a2e5715 Mon Sep 17 00:00:00 2001 From: "shubhang.balkundi" Date: Wed, 6 Apr 2022 11:50:38 +0530 Subject: [PATCH 08/24] adds tests for channel_pool --- src/ziggurat/messaging/channel_pool.clj | 8 +- src/ziggurat/messaging/connection.clj | 28 +++--- test/ziggurat/fixtures.clj | 22 ++--- test/ziggurat/messaging/channel_pool_test.clj | 2 - test/ziggurat/messaging/connection_test.clj | 92 +++++++++---------- test/ziggurat/messaging/producer_test.clj | 86 ++++++++--------- 6 files changed, 117 insertions(+), 121 deletions(-) diff --git a/src/ziggurat/messaging/channel_pool.clj b/src/ziggurat/messaging/channel_pool.clj index 91e2987b..4bd45cd2 100644 --- a/src/ziggurat/messaging/channel_pool.clj +++ b/src/ziggurat/messaging/channel_pool.clj @@ -39,7 +39,7 @@ rmq-chan-pool)) (defstate channel-pool - :start (do (log/info "Creating channel pool") - (create-channel-pool c/connection)) - :stop (do (log/info "Stopping channel pool") - (.close channel-pool))) + :start (do (log/info "Creating channel pool") + (create-channel-pool c/connection)) + :stop (do (log/info "Stopping channel pool") + (.close channel-pool))) diff --git a/src/ziggurat/messaging/connection.clj b/src/ziggurat/messaging/connection.clj index 77aa059d..121e18a1 100644 --- a/src/ziggurat/messaging/connection.clj +++ b/src/ziggurat/messaging/connection.clj @@ -58,7 +58,7 @@ (if is-producer? (:rabbit-mq-connection (ziggurat-config)) (assoc (:rabbit-mq-connection (ziggurat-config)) - :executor (Executors/newFixedThreadPool (total-thread-count))))) + :executor (Executors/newFixedThreadPool (total-thread-count))))) (defn- start-connection [is-producer?] @@ -66,14 +66,14 @@ (when (is-connection-required?) (try (let - [connection (create-connection (get-connection-config is-producer?) (get-tracer-config))] + [connection (create-connection (get-connection-config is-producer?) (get-tracer-config))] (println "Connection created " connection) (doto connection (.addShutdownListener - (reify ShutdownListener - (shutdownCompleted [_ cause] - (when-not (.isInitiatedByApplication cause) - (log/error cause "RabbitMQ connection shut down due to error"))))))) + (reify ShutdownListener + (shutdownCompleted [_ cause] + (when-not (.isInitiatedByApplication cause) + (log/error cause "RabbitMQ connection shut down due to error"))))))) (catch Exception e (report-error e "Error while starting RabbitMQ connection") (throw e))))) @@ -84,13 +84,13 @@ (rmq/close conn))) (defstate consumer-connection - :start (do (log/info "Creating consumer connection") - (start-connection false)) - :stop (do (log/info "Stopping consume connection") - (stop-connection consumer-connection))) + :start (do (log/info "Creating consumer connection") + (start-connection false)) + :stop (do (log/info "Stopping consume connection") + (stop-connection consumer-connection))) (defstate connection - :start (do (log/info "Creating producer connection") - (start-connection true)) - :stop (do (log/info "Stopping producer connection") - (stop-connection connection))) + :start (do (log/info "Creating producer connection") + (start-connection true)) + :stop (do (log/info "Stopping producer connection") + (stop-connection connection))) diff --git a/test/ziggurat/fixtures.clj b/test/ziggurat/fixtures.clj index 415a8ff0..2d9ca414 100644 --- a/test/ziggurat/fixtures.clj +++ b/test/ziggurat/fixtures.clj @@ -19,10 +19,10 @@ (org.apache.kafka.clients.consumer ConsumerConfig) (org.apache.kafka.clients.producer ProducerConfig)) (:gen-class - :methods [^{:static true} [mountConfig [] void] - ^{:static true} [mountProducer [] void] - ^{:static true} [unmountAll [] void]] - :name tech.gojek.ziggurat.internal.test.Fixtures)) + :methods [^{:static true} [mountConfig [] void] + ^{:static true} [mountProducer [] void] + ^{:static true} [unmountAll [] void]] + :name tech.gojek.ziggurat.internal.test.Fixtures)) (def test-config-file-name "config.test.edn") @@ -65,8 +65,6 @@ (f) (mount/stop #'metrics/statsd-reporter)) - - (defn mount-tracer [] (with-redefs [tracer/create-tracer (fn [] (MockTracer.))] (-> (mount/only [#'tracer/tracer]) @@ -122,18 +120,18 @@ (mount-tracer) (-> - (mount/only [#'connection #'consumer-connection #'channel-pool]) - (mount/with-args {:stream-routes stream-routes}) - (mount/start)) + (mount/only [#'connection #'consumer-connection #'channel-pool]) + (mount/with-args {:stream-routes stream-routes}) + (mount/start)) (f) (mount/stop))) (defn with-start-server* [stream-routes f] (mount-config) (-> - (mount/only [#'server]) - (mount/with-args {:stream-routes stream-routes}) - (mount/start)) + (mount/only [#'server]) + (mount/with-args {:stream-routes stream-routes}) + (mount/start)) (f) (mount/stop)) diff --git a/test/ziggurat/messaging/channel_pool_test.clj b/test/ziggurat/messaging/channel_pool_test.clj index b00fc648..2e0414d1 100644 --- a/test/ziggurat/messaging/channel_pool_test.clj +++ b/test/ziggurat/messaging/channel_pool_test.clj @@ -12,8 +12,6 @@ (use-fixtures :once (join-fixtures [fix/mount-only-config fix/mount-config-with-tracer])) - - (deftest calc-total-threads-test (testing "it should calculate the total threads configured for RabbitMQ, Kafka streams and Batch consumers" (let [expected-count 44 diff --git a/test/ziggurat/messaging/connection_test.clj b/test/ziggurat/messaging/connection_test.clj index 0967216d..e83c1cc5 100644 --- a/test/ziggurat/messaging/connection_test.clj +++ b/test/ziggurat/messaging/connection_test.clj @@ -21,10 +21,10 @@ (reset! executor-present? true)) (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} - :tracer {:enabled false}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} + :tracer {:enabled false}))] (-> (mount/only #{#'consumer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -41,10 +41,10 @@ (reset! executor-present? true)) (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} - :tracer {:enabled false}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} + :tracer {:enabled false}))] (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -64,10 +64,10 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} - :tracer {}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} + :tracer {}))] (-> (mount/only #{#'consumer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -88,10 +88,10 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} - :tracer {:enabled false}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} + :tracer {:enabled false}))] (-> (mount/only #{#'consumer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -108,8 +108,8 @@ (reset! rmq-connect-called? true) (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled true} - :tracer {:enabled false}))] + :retry {:enabled true} + :tracer {:enabled false}))] (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -162,8 +162,8 @@ (reset! rmq-connect-called? true) (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false} - :tracer {:enabled false}))] + :retry {:enabled false} + :tracer {:enabled false}))] (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -178,10 +178,10 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {}} - :tracer {:enabled false}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {}} + :tracer {:enabled false}))] (mount/start (mount/only [#'consumer-connection])) (mount/stop #'consumer-connection) (is (= 12 @thread-count))))) @@ -194,11 +194,11 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-rmq-connect provided-config)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}} - :default-1 {:channels {:channel-1 {:worker-count 8}}}} - :tracer {:enabled false}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}} + :default-1 {:channels {:channel-1 {:worker-count 8}}}} + :tracer {:enabled false}))] (mount/start (mount/only [#'consumer-connection])) (mount/stop #'consumer-connection) (is (= 34 @thread-count)))))) @@ -216,9 +216,9 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}}))] (-> (mount/only #{#'consumer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -235,7 +235,7 @@ (reset! create-connect-called? true) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled true}))] + :retry {:enabled true}))] (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -251,7 +251,7 @@ (reset! create-connect-called? true) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false}))] + :retry {:enabled false}))] (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -270,7 +270,7 @@ (reset! create-connect-called? true) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false}))] + :retry {:enabled false}))] (-> (mount/only #{#'connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -286,10 +286,10 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 5} - :channel-2 {:worker-count 10}}}}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 5} + :channel-2 {:worker-count 10}}}}))] (-> (mount/only #{#'consumer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) @@ -304,9 +304,9 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {}}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {}}))] (mount/start (mount/only [#'consumer-connection])) (mount/stop #'consumer-connection) (is (= 12 @thread-count))))) @@ -319,10 +319,10 @@ (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config - :jobs {:instant {:worker-count 4}} - :retry {:enabled true} - :stream-router {:default {:channels {:channel-1 {:worker-count 10}}} - :default-1 {:channels {:channel-1 {:worker-count 8}}}}))] + :jobs {:instant {:worker-count 4}} + :retry {:enabled true} + :stream-router {:default {:channels {:channel-1 {:worker-count 10}}} + :default-1 {:channels {:channel-1 {:worker-count 8}}}}))] (mount/start (mount/only [#'consumer-connection])) (mount/stop #'consumer-connection) (is (= 34 @thread-count)))))) diff --git a/test/ziggurat/messaging/producer_test.clj b/test/ziggurat/messaging/producer_test.clj index 2d5c05a5..77f4145f 100644 --- a/test/ziggurat/messaging/producer_test.clj +++ b/test/ziggurat/messaging/producer_test.clj @@ -71,14 +71,14 @@ (testing "message in channel will be retried in delay queue with suffix 1 if message retry-count exceeds retry count in channel config" (with-redefs [ziggurat-config (constantly (assoc (ziggurat-config) - :stream-router - {:default - {:channels - {:exponential-retry - {:retry {:count 5 - :enabled true - :type :exponential - :queue-timeout-ms 1000}}}}}))] + :stream-router + {:default + {:channels + {:exponential-retry + {:retry {:count 5 + :enabled true + :type :exponential + :queue-timeout-ms 1000}}}}}))] (fix/with-queues {:default {:handler-fn #(constantly nil) :exponential-retry #(constantly nil)}} @@ -91,9 +91,9 @@ (testing "message in channel will be retried with linear queue timeout" (with-redefs [ziggurat-config (constantly (assoc (ziggurat-config) - :stream-router {:default {:channels {:linear-retry {:retry {:count 5 - :enabled true - :queue-timeout-ms 2000}}}}}))] + :stream-router {:default {:channels {:linear-retry {:retry {:count 5 + :enabled true + :queue-timeout-ms 2000}}}}}))] (fix/with-queues {:default {:handler-fn #(constantly nil) :linear-retry #(constantly nil)}} @@ -113,10 +113,10 @@ (testing "message in channel will be retried with exponential timeout calculated from channel specific queue-timeout-ms value" (with-redefs [ziggurat-config (constantly (assoc (ziggurat-config) - :stream-router {:default {:channels {:exponential-retry {:retry {:count 5 - :enabled true - :type :exponential - :queue-timeout-ms 1000}}}}}))] + :stream-router {:default {:channels {:exponential-retry {:retry {:count 5 + :enabled true + :type :exponential + :queue-timeout-ms 1000}}}}}))] (fix/with-queues {:default {:handler-fn #(constantly nil) :exponential-retry #(constantly nil)}} @@ -232,9 +232,9 @@ (deftest retry-with-exponential-backoff-test (testing "message will publish to delay with retry count queue when exponential backoff enabled" (with-redefs [ziggurat-config (constantly (assoc (ziggurat-config) - :retry {:count 5 - :enabled true - :type :exponential}))] + :retry {:count 5 + :enabled true + :type :exponential}))] (testing "message with no retry count will publish to delay queue with suffix 1" (fix/with-queues {:default {:handler-fn #(constantly nil)}} @@ -274,8 +274,8 @@ (let [ziggurat-config (ziggurat-config)] (testing "When retries are enabled" (with-redefs [config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled true - :type :linear}))] + :retry {:enabled true + :type :linear}))] (testing "it does not create queues when stream-routes are not passed" (let [counter (atom 0)] (with-redefs [producer/create-and-bind-queue (fn @@ -436,7 +436,7 @@ (testing "when retries are disabled" (with-redefs [config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false}))] + :retry {:enabled false}))] (testing "it does not create queues when stream-routes are not passed" (let [counter (atom 0)] (with-redefs [producer/create-and-bind-queue (fn @@ -477,8 +477,8 @@ (testing "when retries are disabled" (with-redefs [config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false} - :stream-router {:default {:channels {:channel-1 {:retry {:enabled false}}}}}))] + :retry {:enabled false} + :stream-router {:default {:channels {:channel-1 {:retry {:enabled false}}}}}))] (testing "it creates instant queues with topic entity for channels only" (with-open [ch (lch/open connection)] @@ -583,50 +583,50 @@ (testing "when retries are enabled" (let [channel :linear-retry] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :stream-router {topic-entity {:channels {channel {:retry {:count 5 - :enabled true - :queue-timeout-ms 2000}}}}}))] + :stream-router {topic-entity {:channels {channel {:retry {:count 5 + :enabled true + :queue-timeout-ms 2000}}}}}))] (is (= 2000 (producer/get-channel-queue-timeout-ms topic-entity channel message)))))) (testing "when exponential backoff are enabled and channel queue timeout defined" (let [channel :exponential-retry] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :stream-router {topic-entity {:channels {channel {:retry {:count 5 - :enabled true - :type :exponential - :queue-timeout-ms 1000}}}}}))] + :stream-router {topic-entity {:channels {channel {:retry {:count 5 + :enabled true + :type :exponential + :queue-timeout-ms 1000}}}}}))] (is (= 7000 (producer/get-channel-queue-timeout-ms topic-entity channel message)))))) (testing "when exponential backoff are enabled and channel queue timeout is not defined" (let [channel :exponential-retry] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :stream-router {topic-entity {:channels {channel {:retry {:count 5 - :enabled true - :type :exponential}}}}}))] + :stream-router {topic-entity {:channels {channel {:retry {:count 5 + :enabled true + :type :exponential}}}}}))] (is (= 700 (producer/get-channel-queue-timeout-ms topic-entity channel message)))))))) (deftest get-queue-timeout-ms-test (testing "when exponential retries are enabled" (let [message (assoc message-payload :retry-count 2)] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :retry {:enabled true - :count 5 - :type :exponential}))] + :retry {:enabled true + :count 5 + :type :exponential}))] (is (= 700 (producer/get-queue-timeout-ms message)))))) (testing "when exponential retries are enabled and retry-count exceeds 25, the max possible timeouts are calculated using 25 as the retry-count" (let [message (assoc message-payload :retry-count 20)] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :retry {:enabled true - :count 50 - :type :exponential}))] + :retry {:enabled true + :count 50 + :type :exponential}))] ;; For 25 max exponential retries, exponent comes to 25-20=5, which makes timeout = 100*(2^5-1) = 3100 (is (= 3100 (producer/get-queue-timeout-ms message)))))) (testing "when exponential retries are enabled with total retries as 25 and if the message has already been retried 24 times, then the queue-timeout is calculated without any failure" (let [message (assoc message-payload :retry-count 1)] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :retry {:enabled true - :count 25 - :type :exponential} - :rabbit-mq {:delay {:queue-timeout-ms 5000}}))] + :retry {:enabled true + :count 25 + :type :exponential} + :rabbit-mq {:delay {:queue-timeout-ms 5000}}))] ;; For 25 max exponential retries, exponent comes to 25-1=24, which makes timeout = 5000*(2^24-1) = 83886075000 (is (= 83886075000 (producer/get-queue-timeout-ms message))))))) From 30bce66d0d18be02bc2c2adcae59ee1ee2972c42 Mon Sep 17 00:00:00 2001 From: "shubhang.balkundi" Date: Wed, 6 Apr 2022 15:26:04 +0530 Subject: [PATCH 09/24] adds logs while invalidating the object --- .../rabbitmq/channel_pool/RabbitMQChannelFactory.java | 7 +++++-- test/ziggurat/messaging/channel_pool_test.clj | 4 +--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java b/src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java index 018eef40..1ef1deaa 100644 --- a/src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java +++ b/src/com/gojek/rabbitmq/channel_pool/RabbitMQChannelFactory.java @@ -28,8 +28,11 @@ public Channel create() throws Exception { @Override public boolean validateObject(PooledObject p) { - super.validateObject(p); - return p.getObject().isOpen(); + boolean open = p.getObject().isOpen(); + if (!open) { + log.info("Channel is closed, invalidating channel with id " + p.getObject().getChannelNumber()); + } + return open; } @Override diff --git a/test/ziggurat/messaging/channel_pool_test.clj b/test/ziggurat/messaging/channel_pool_test.clj index 2e0414d1..3646dfa9 100644 --- a/test/ziggurat/messaging/channel_pool_test.clj +++ b/test/ziggurat/messaging/channel_pool_test.clj @@ -1,10 +1,8 @@ (ns ziggurat.messaging.channel-pool-test (:require [clojure.test :refer :all] - [ziggurat.config :as zc] [ziggurat.messaging.channel_pool :as cpool] [ziggurat.messaging.connection :refer [connection]] - [ziggurat.fixtures :as fix] - [clojure.tools.logging :as log]) + [ziggurat.fixtures :as fix]) (:import (org.apache.commons.pool2.impl GenericObjectPoolConfig GenericObjectPool) (java.time Duration) (com.rabbitmq.client Channel))) From 65edd7325098650553a1b54ac63ba027bd556b9a Mon Sep 17 00:00:00 2001 From: "shubhang.balkundi" Date: Thu, 7 Apr 2022 14:53:03 +0530 Subject: [PATCH 10/24] do not retry publish on channel borrow exceptions --- src/ziggurat/messaging/producer.clj | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ziggurat/messaging/producer.clj b/src/ziggurat/messaging/producer.clj index 02981a38..06181125 100644 --- a/src/ziggurat/messaging/producer.clj +++ b/src/ziggurat/messaging/producer.clj @@ -127,8 +127,7 @@ (finally (.returnObject cpool/channel-pool ch)))) (catch Exception e (log/error "Exception occurred while borrowing a channel from the pool") - (metrics/increment-count ["rabbitmq" "publish" "channel_borrow"] {:topic-entity (name (:topic-entity message-payload))}) - true))) + (metrics/increment-count ["rabbitmq" "publish" "channel_borrow"] {:topic-entity (name (:topic-entity message-payload))})))) (defn publish ([exchange message-payload] From 7f7832d75bb364e34e2826163cb466b92d598d0e Mon Sep 17 00:00:00 2001 From: "shubhang.balkundi" Date: Fri, 8 Apr 2022 12:39:57 +0530 Subject: [PATCH 11/24] adds a test for channel pool publish --- src/ziggurat/init.clj | 7 +- src/ziggurat/messaging/channel_pool.clj | 8 +- src/ziggurat/messaging/producer.clj | 10 +- test/ziggurat/messaging/channel_pool_test.clj | 5 + test/ziggurat/messaging/producer_test.clj | 97 ++++++++++--------- 5 files changed, 72 insertions(+), 55 deletions(-) diff --git a/src/ziggurat/init.clj b/src/ziggurat/init.clj index ef9e6184..6842bfac 100644 --- a/src/ziggurat/init.clj +++ b/src/ziggurat/init.clj @@ -15,15 +15,16 @@ [ziggurat.messaging.producer :as messaging-producer] [ziggurat.metrics :as metrics] [ziggurat.nrepl-server :as nrepl-server] - [ziggurat.producer :as producer :refer [kafka-producers]] + [ziggurat.producer :refer [kafka-producers]] [ziggurat.sentry :refer [sentry-reporter]] [ziggurat.server :as server] [ziggurat.streams :as streams] [ziggurat.tracer :as tracer] [ziggurat.util.java-util :as util]) (:gen-class - :methods [^{:static true} [init [java.util.Map] void]] - :name tech.gojek.ziggurat.internal.Init)) + :methods [^{:static true} [init [java.util.Map] void]] + :name tech.gojek.ziggurat.internal.Init) + (:import (clojure.lang ExceptionInfo))) (defn- event-routes [args] (merge (:stream-routes args) (:batch-routes args))) diff --git a/src/ziggurat/messaging/channel_pool.clj b/src/ziggurat/messaging/channel_pool.clj index 4bd45cd2..91e2987b 100644 --- a/src/ziggurat/messaging/channel_pool.clj +++ b/src/ziggurat/messaging/channel_pool.clj @@ -39,7 +39,7 @@ rmq-chan-pool)) (defstate channel-pool - :start (do (log/info "Creating channel pool") - (create-channel-pool c/connection)) - :stop (do (log/info "Stopping channel pool") - (.close channel-pool))) + :start (do (log/info "Creating channel pool") + (create-channel-pool c/connection)) + :stop (do (log/info "Stopping channel pool") + (.close channel-pool))) diff --git a/src/ziggurat/messaging/producer.clj b/src/ziggurat/messaging/producer.clj index 06181125..d9587ace 100644 --- a/src/ziggurat/messaging/producer.clj +++ b/src/ziggurat/messaging/producer.clj @@ -11,8 +11,9 @@ [ziggurat.messaging.connection :refer [connection is-connection-required?]] [ziggurat.messaging.util :as util] [ziggurat.metrics :as metrics]) - (:import (com.rabbitmq.client AlreadyClosedException) - (java.io IOException))) + (:import (com.rabbitmq.client AlreadyClosedException Channel) + (java.io IOException) + (org.apache.commons.pool2.impl GenericObjectPool))) (def MAX_EXPONENTIAL_RETRIES 25) @@ -108,6 +109,9 @@ (metrics/increment-count ["rabbitmq" "publish" "network"] "exception" {:topic-entity (name (:topic-entity message-payload))}) true) +(defn return-to-pool [^GenericObjectPool pool ^Channel ch] + (.returnObject pool ch)) + (defn- publish-internal [exchange message-payload expiration] (try @@ -124,7 +128,7 @@ (log/error e "Exception was encountered while publishing to RabbitMQ") (metrics/increment-count ["rabbitmq" "publish"] "exception" {:topic-entity (name (:topic-entity message-payload))}) false) - (finally (.returnObject cpool/channel-pool ch)))) + (finally (return-to-pool cpool/channel-pool ch)))) (catch Exception e (log/error "Exception occurred while borrowing a channel from the pool") (metrics/increment-count ["rabbitmq" "publish" "channel_borrow"] {:topic-entity (name (:topic-entity message-payload))})))) diff --git a/test/ziggurat/messaging/channel_pool_test.clj b/test/ziggurat/messaging/channel_pool_test.clj index 3646dfa9..50f76e19 100644 --- a/test/ziggurat/messaging/channel_pool_test.clj +++ b/test/ziggurat/messaging/channel_pool_test.clj @@ -49,10 +49,15 @@ (testing "it should invalidate a closed channel and return a new channel on borrow" (mount.core/start #'connection) (let [channel-pool ^GenericObjectPool (cpool/create-channel-pool connection) + _ (doto channel-pool + (.setMaxTotal 1) + (.setMinIdle 0) + (.setMaxIdle 1)) rmq-chan ^Channel (.borrowObject channel-pool) _ (.close rmq-chan) _ (.returnObject channel-pool rmq-chan) rmq-chan-2 ^Channel (.borrowObject channel-pool)] + (is (not (.equals rmq-chan-2 rmq-chan))) (is (.isOpen rmq-chan-2))))) diff --git a/test/ziggurat/messaging/producer_test.clj b/test/ziggurat/messaging/producer_test.clj index 77f4145f..8f17f430 100644 --- a/test/ziggurat/messaging/producer_test.clj +++ b/test/ziggurat/messaging/producer_test.clj @@ -16,7 +16,6 @@ [ziggurat.metrics :as metrics]) (:import [org.apache.kafka.common.header.internals RecordHeaders RecordHeader] (com.rabbitmq.client Channel Connection ShutdownSignalException AlreadyClosedException) - (org.apache.kafka.common.header Header) (java.io IOException))) (use-fixtures :once (join-fixtures [fix/init-rabbit-mq @@ -71,14 +70,14 @@ (testing "message in channel will be retried in delay queue with suffix 1 if message retry-count exceeds retry count in channel config" (with-redefs [ziggurat-config (constantly (assoc (ziggurat-config) - :stream-router - {:default - {:channels - {:exponential-retry - {:retry {:count 5 - :enabled true - :type :exponential - :queue-timeout-ms 1000}}}}}))] + :stream-router + {:default + {:channels + {:exponential-retry + {:retry {:count 5 + :enabled true + :type :exponential + :queue-timeout-ms 1000}}}}}))] (fix/with-queues {:default {:handler-fn #(constantly nil) :exponential-retry #(constantly nil)}} @@ -91,9 +90,9 @@ (testing "message in channel will be retried with linear queue timeout" (with-redefs [ziggurat-config (constantly (assoc (ziggurat-config) - :stream-router {:default {:channels {:linear-retry {:retry {:count 5 - :enabled true - :queue-timeout-ms 2000}}}}}))] + :stream-router {:default {:channels {:linear-retry {:retry {:count 5 + :enabled true + :queue-timeout-ms 2000}}}}}))] (fix/with-queues {:default {:handler-fn #(constantly nil) :linear-retry #(constantly nil)}} @@ -113,10 +112,10 @@ (testing "message in channel will be retried with exponential timeout calculated from channel specific queue-timeout-ms value" (with-redefs [ziggurat-config (constantly (assoc (ziggurat-config) - :stream-router {:default {:channels {:exponential-retry {:retry {:count 5 - :enabled true - :type :exponential - :queue-timeout-ms 1000}}}}}))] + :stream-router {:default {:channels {:exponential-retry {:retry {:count 5 + :enabled true + :type :exponential + :queue-timeout-ms 1000}}}}}))] (fix/with-queues {:default {:handler-fn #(constantly nil) :exponential-retry #(constantly nil)}} @@ -232,9 +231,9 @@ (deftest retry-with-exponential-backoff-test (testing "message will publish to delay with retry count queue when exponential backoff enabled" (with-redefs [ziggurat-config (constantly (assoc (ziggurat-config) - :retry {:count 5 - :enabled true - :type :exponential}))] + :retry {:count 5 + :enabled true + :type :exponential}))] (testing "message with no retry count will publish to delay queue with suffix 1" (fix/with-queues {:default {:handler-fn #(constantly nil)}} @@ -274,8 +273,8 @@ (let [ziggurat-config (ziggurat-config)] (testing "When retries are enabled" (with-redefs [config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled true - :type :linear}))] + :retry {:enabled true + :type :linear}))] (testing "it does not create queues when stream-routes are not passed" (let [counter (atom 0)] (with-redefs [producer/create-and-bind-queue (fn @@ -436,7 +435,7 @@ (testing "when retries are disabled" (with-redefs [config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false}))] + :retry {:enabled false}))] (testing "it does not create queues when stream-routes are not passed" (let [counter (atom 0)] (with-redefs [producer/create-and-bind-queue (fn @@ -477,8 +476,8 @@ (testing "when retries are disabled" (with-redefs [config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false} - :stream-router {:default {:channels {:channel-1 {:retry {:enabled false}}}}}))] + :retry {:enabled false} + :stream-router {:default {:channels {:channel-1 {:retry {:enabled false}}}}}))] (testing "it creates instant queues with topic entity for channels only" (with-open [ch (lch/open connection)] @@ -583,52 +582,60 @@ (testing "when retries are enabled" (let [channel :linear-retry] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :stream-router {topic-entity {:channels {channel {:retry {:count 5 - :enabled true - :queue-timeout-ms 2000}}}}}))] + :stream-router {topic-entity {:channels {channel {:retry {:count 5 + :enabled true + :queue-timeout-ms 2000}}}}}))] (is (= 2000 (producer/get-channel-queue-timeout-ms topic-entity channel message)))))) (testing "when exponential backoff are enabled and channel queue timeout defined" (let [channel :exponential-retry] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :stream-router {topic-entity {:channels {channel {:retry {:count 5 - :enabled true - :type :exponential - :queue-timeout-ms 1000}}}}}))] + :stream-router {topic-entity {:channels {channel {:retry {:count 5 + :enabled true + :type :exponential + :queue-timeout-ms 1000}}}}}))] (is (= 7000 (producer/get-channel-queue-timeout-ms topic-entity channel message)))))) (testing "when exponential backoff are enabled and channel queue timeout is not defined" (let [channel :exponential-retry] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :stream-router {topic-entity {:channels {channel {:retry {:count 5 - :enabled true - :type :exponential}}}}}))] + :stream-router {topic-entity {:channels {channel {:retry {:count 5 + :enabled true + :type :exponential}}}}}))] (is (= 700 (producer/get-channel-queue-timeout-ms topic-entity channel message)))))))) (deftest get-queue-timeout-ms-test (testing "when exponential retries are enabled" (let [message (assoc message-payload :retry-count 2)] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :retry {:enabled true - :count 5 - :type :exponential}))] + :retry {:enabled true + :count 5 + :type :exponential}))] (is (= 700 (producer/get-queue-timeout-ms message)))))) (testing "when exponential retries are enabled and retry-count exceeds 25, the max possible timeouts are calculated using 25 as the retry-count" (let [message (assoc message-payload :retry-count 20)] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :retry {:enabled true - :count 50 - :type :exponential}))] + :retry {:enabled true + :count 50 + :type :exponential}))] ;; For 25 max exponential retries, exponent comes to 25-20=5, which makes timeout = 100*(2^5-1) = 3100 (is (= 3100 (producer/get-queue-timeout-ms message)))))) (testing "when exponential retries are enabled with total retries as 25 and if the message has already been retried 24 times, then the queue-timeout is calculated without any failure" (let [message (assoc message-payload :retry-count 1)] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :retry {:enabled true - :count 25 - :type :exponential} - :rabbit-mq {:delay {:queue-timeout-ms 5000}}))] + :retry {:enabled true + :count 25 + :type :exponential} + :rabbit-mq {:delay {:queue-timeout-ms 5000}}))] ;; For 25 max exponential retries, exponent comes to 25-1=24, which makes timeout = 5000*(2^24-1) = 83886075000 (is (= 83886075000 (producer/get-queue-timeout-ms message))))))) - +;(testing "the channel borrowed from the pool should always be returned back to the pool once the publish lb/publish returns" +; (let [call-count (atom 0) +; expected-call-count 10] +; (with-redefs [producer/return-to-pool (fn [_ _] (swap! call-count inc)) +; lb/publish (fn [_ _ _ _ _])] +; (let [futures (take expected-call-count (repeatedly #(future (producer/publish "pool-test" {:a "hello world"}))))] +; (doall futures) +; (map #(deref %) futures))) +; (is (= @call-count expected-call-count)))) From 2f661fab072e04d7e21621ebb7a6cf64d8ba9daa Mon Sep 17 00:00:00 2001 From: "shubhang.balkundi" Date: Mon, 11 Apr 2022 12:47:17 +0530 Subject: [PATCH 12/24] fixes existing tests --- src/ziggurat/init.clj | 8 +- src/ziggurat/messaging/channel_pool.clj | 8 +- src/ziggurat/messaging/connection.clj | 2 + test/ziggurat/init_test.clj | 96 ++++++++++----------- test/ziggurat/messaging/connection_test.clj | 3 +- test/ziggurat/messaging/producer_test.clj | 86 +++++++++--------- 6 files changed, 102 insertions(+), 101 deletions(-) diff --git a/src/ziggurat/init.clj b/src/ziggurat/init.clj index 6842bfac..6f4ee830 100644 --- a/src/ziggurat/init.clj +++ b/src/ziggurat/init.clj @@ -22,8 +22,8 @@ [ziggurat.tracer :as tracer] [ziggurat.util.java-util :as util]) (:gen-class - :methods [^{:static true} [init [java.util.Map] void]] - :name tech.gojek.ziggurat.internal.Init) + :methods [^{:static true} [init [java.util.Map] void]] + :name tech.gojek.ziggurat.internal.Init) (:import (clojure.lang ExceptionInfo))) (defn- event-routes [args] @@ -226,8 +226,8 @@ (throw (IllegalArgumentException. "Either :stream-routes or :batch-routes should be present in init args"))) (cond-> base-modes (some? stream-routes) (conj :stream-worker) - (some? batch-routes) (conj :batch-worker) - (some? actor-routes) (conj :api-server)))) + (some? batch-routes) (conj :batch-worker) + (some? actor-routes) (conj :api-server)))) (defn validate-modes [modes stream-routes batch-routes actor-routes] (let [derived-modes (if-not (empty? modes) diff --git a/src/ziggurat/messaging/channel_pool.clj b/src/ziggurat/messaging/channel_pool.clj index 91e2987b..4bd45cd2 100644 --- a/src/ziggurat/messaging/channel_pool.clj +++ b/src/ziggurat/messaging/channel_pool.clj @@ -39,7 +39,7 @@ rmq-chan-pool)) (defstate channel-pool - :start (do (log/info "Creating channel pool") - (create-channel-pool c/connection)) - :stop (do (log/info "Stopping channel pool") - (.close channel-pool))) + :start (do (log/info "Creating channel pool") + (create-channel-pool c/connection)) + :stop (do (log/info "Stopping channel pool") + (.close channel-pool))) diff --git a/src/ziggurat/messaging/connection.clj b/src/ziggurat/messaging/connection.clj index 121e18a1..1a70c4fa 100644 --- a/src/ziggurat/messaging/connection.clj +++ b/src/ziggurat/messaging/connection.clj @@ -61,6 +61,8 @@ :executor (Executors/newFixedThreadPool (total-thread-count))))) (defn- start-connection + "is-producer? - defines whether the connection is being created for producers or consumers + producer connections do not require the :executor option" [is-producer?] (log/info "Connecting to RabbitMQ") (when (is-connection-required?) diff --git a/test/ziggurat/init_test.clj b/test/ziggurat/init_test.clj index 5fde85c7..be99d685 100644 --- a/test/ziggurat/init_test.clj +++ b/test/ziggurat/init_test.clj @@ -23,12 +23,12 @@ (deftest start-calls-actor-start-fn-test (testing "The actor start fn starts before the ziggurat state and can read config" (let [result (atom 1)] - (with-redefs [streams/start-streams (fn [_ _] (reset! result (* @result 2))) - streams/stop-streams (constantly nil) + (with-redefs [streams/start-streams (fn [_ _] (reset! result (* @result 2))) + streams/stop-streams (constantly nil) ;; will be called valid modes number of times - rmqc/start-connection (fn [] (reset! result (* @result 2))) + rmqc/start-connection (fn [_] (reset! result (* @result 2))) rmqc/stop-connection (constantly nil) - tracer/create-tracer (fn [] (MockTracer.))] + tracer/create-tracer (fn [] (MockTracer.))] (with-config (do (init/start #(reset! result (+ @result 3)) {} {} [] nil) (init/stop #() nil) @@ -48,10 +48,10 @@ (deftest stop-calls-idempotentcy-test (testing "The stop function should be idempotent" (let [result (atom 1)] - (with-redefs [streams/start-streams (constantly nil) - streams/stop-streams (constantly nil) - rmqc/stop-connection (fn [_] (reset! result (* @result 2))) - tracer/create-tracer (fn [] (MockTracer.))] + (with-redefs [streams/start-streams (constantly nil) + streams/stop-streams (constantly nil) + rmqc/stop-connection (fn [_] (reset! result (* @result 2))) + tracer/create-tracer (fn [] (MockTracer.))] (with-config (do (init/start #() {} {} [] nil) (init/stop #(reset! result (+ @result 3)) nil) @@ -61,7 +61,7 @@ (testing "Start calls make queues with both streams and batch routes" (let [make-queues-called (atom 0) expected-stream-routes {:default {:handler-fn #()}} - expected-batch-routes {:consumer-1 {:handler-fn #()}}] + expected-batch-routes {:consumer-1 {:handler-fn #()}}] (with-redefs [streams/start-streams (constantly nil) streams/stop-streams (constantly nil) messaging-producer/make-queues (fn [all-routes] @@ -79,7 +79,7 @@ (testing "Start calls start subscribers" (let [start-subscriber-called (atom 0) expected-stream-routes {:default {:handler-fn #()}} - expected-batch-routes {:consumer-1 {:handler-fn #()}}] + expected-batch-routes {:consumer-1 {:handler-fn #()}}] (with-redefs [streams/start-streams (constantly nil) streams/stop-streams (constantly nil) messaging-consumer/start-subscribers (fn [stream-routes batch-routes] @@ -99,28 +99,28 @@ (testing "Main function should call start (arity: 3)" (let [start-was-called (atom false) expected-stream-routes {:default {:handler-fn #(constantly nil)}}] - (with-redefs [init/add-shutdown-hook (fn [_ _] (constantly nil)) - init/initialize-config (constantly nil) + (with-redefs [init/add-shutdown-hook (fn [_ _] (constantly nil)) + init/initialize-config (constantly nil) init/validate-routes-against-config (constantly nil) - init/start (fn [_ stream-router _ _ _] - (swap! start-was-called not) - (is (= expected-stream-routes stream-router)))] + init/start (fn [_ stream-router _ _ _] + (swap! start-was-called not) + (is (= expected-stream-routes stream-router)))] (init/main #() #() expected-stream-routes) (is @start-was-called)))) (testing "Flat Json Layout decoder is set if log format is json" (let [start-was-called (atom false) decoder-was-set (atom false) expected-stream-routes {:default {:handler-fn #(constantly nil)}} - config config/default-config] - (with-redefs [init/add-shutdown-hook (fn [_ _] (constantly nil)) - config/config-file "config.test.edn" - config/ziggurat-config (fn [] (assoc config :log-format "json")) + config config/default-config] + (with-redefs [init/add-shutdown-hook (fn [_ _] (constantly nil)) + config/config-file "config.test.edn" + config/ziggurat-config (fn [] (assoc config :log-format "json")) init/validate-routes-against-config (constantly nil) - init/start (fn [_ stream-router _ _ _] - (swap! start-was-called not) - (is (= expected-stream-routes stream-router))) - flat/set-decoder! (fn [decoder] (is (= decoder codec/destringify-val)) - (reset! decoder-was-set true))] + init/start (fn [_ stream-router _ _ _] + (swap! start-was-called not) + (is (= expected-stream-routes stream-router))) + flat/set-decoder! (fn [decoder] (is (= decoder codec/destringify-val)) + (reset! decoder-was-set true))] (init/main #() #() expected-stream-routes) (is @start-was-called) (is @decoder-was-set))))) @@ -128,31 +128,31 @@ (def mock-modes {:api-server {:start-fn (constantly nil) :stop-fn (constantly nil)} :stream-worker {:start-fn (constantly nil) :stop-fn (constantly nil)} :worker {:start-fn (constantly nil) :stop-fn (constantly nil)} - :batch-worker {:start-fn (constantly nil) :stop-fn (constantly nil)} + :batch-worker {:start-fn (constantly nil) :stop-fn (constantly nil)} :management-api {:start-fn (constantly nil) :stop-fn (constantly nil)}}) (deftest batch-routes-test (testing "Main function should start batch consumption if batch-routes are provided and the modes vector is empty (arity: 1)" - (let [start-batch-consumers-was-called (atom false) - expected-stream-routes {:default {:handler-fn #(constantly nil)}} - batch-routes {:consumer-1 {:handler-fn #(constantly nil)}}] - (with-redefs [init/add-shutdown-hook (constantly nil) - init/start-common-states (constantly nil) - init/initialize-config (constantly nil) + (let [start-batch-consumers-was-called (atom false) + expected-stream-routes {:default {:handler-fn #(constantly nil)}} + batch-routes {:consumer-1 {:handler-fn #(constantly nil)}}] + (with-redefs [init/add-shutdown-hook (constantly nil) + init/start-common-states (constantly nil) + init/initialize-config (constantly nil) init/validate-routes-against-config (constantly nil) - init/valid-modes-fns (assoc-in mock-modes [:batch-worker :start-fn] (fn [_] (reset! start-batch-consumers-was-called true)))] + init/valid-modes-fns (assoc-in mock-modes [:batch-worker :start-fn] (fn [_] (reset! start-batch-consumers-was-called true)))] (init/main {:start-fn #() :stop-fn #() :stream-routes expected-stream-routes :batch-routes batch-routes :actor-routes []}) (is @start-batch-consumers-was-called))))) (deftest stream-routes-test (testing "Main function should call the start the streams when the (arity: 4)" - (let [start-streams-called (atom false) - expected-stream-routes {:default {:handler-fn #(constantly nil)}}] - (with-redefs [init/add-shutdown-hook (constantly nil) - init/start-common-states (constantly nil) - init/initialize-config (constantly nil) + (let [start-streams-called (atom false) + expected-stream-routes {:default {:handler-fn #(constantly nil)}}] + (with-redefs [init/add-shutdown-hook (constantly nil) + init/start-common-states (constantly nil) + init/initialize-config (constantly nil) init/validate-routes-against-config (constantly nil) - init/valid-modes-fns (assoc-in mock-modes [:stream-worker :start-fn] (fn [_] (reset! start-streams-called true)))] + init/valid-modes-fns (assoc-in mock-modes [:stream-worker :start-fn] (fn [_] (reset! start-streams-called true)))] (init/main #() #() expected-stream-routes) (is @start-streams-called))))) @@ -293,12 +293,12 @@ (with-config (let [stream-routes {:test-router {:handler-fn #(constantly nil)}} batch-routes {:test-consumer {:handler-fn #(constantly nil)}} - modes [:stream-worker :batch-worker]] + modes [:stream-worker :batch-worker]] (with-redefs [config/ziggurat-config (fn [] (-> config/config :ziggurat (assoc-in [:stream-router] {:test-router {:handler-fn #(constantly nil) - :channels {:channel-1 {}}}}) + :channels {:channel-1 {}}}}) (assoc-in [:batch-routes] {:test-consumer {:handler-fn #(constantly nil)}})))] (testing "when routes which are present in the config are passed, there isn't any exception" (init/validate-routes stream-routes batch-routes modes)) @@ -306,29 +306,29 @@ (let [stream-routes {:test-router-invalid {:handler-fn #(constantly nil)}}] (is (thrown? IllegalArgumentException (init/validate-routes stream-routes batch-routes modes))))) (testing "when invalid stream-route is passed along with a valid one, there is an exception" - (let [stream-routes {:test-router {:handler-fn #(constantly nil)} + (let [stream-routes {:test-router {:handler-fn #(constantly nil)} :test-router-invalid {:handler-fn #(constantly nil)}}] (is (thrown? IllegalArgumentException (init/validate-routes stream-routes batch-routes modes))))) (testing "when invalid batch-routes is passed, there is an exception" (let [batch-routes {:test-consumer-invalid {:handler-fn #(constantly nil)}}] (is (thrown? IllegalArgumentException (init/validate-routes stream-routes batch-routes modes))))) (testing "when invalid batch-routes is passed along with a valid one, there is an exception" - (let [batch-routes {:test-consumer {:handler-fn #(constantly nil)} + (let [batch-routes {:test-consumer {:handler-fn #(constantly nil)} :test-consumer-invalid {:handler-fn #(constantly nil)}}] (is (thrown? IllegalArgumentException (init/validate-routes stream-routes batch-routes modes))))) (testing "when invalid channels in stream-routes is passed, there is an exception" (let [stream-routes {:test-router {:handler-fn #(constantly nil) - :channel-1 #() - :channel-2 #()}}] + :channel-1 #() + :channel-2 #()}}] (is (thrown? IllegalArgumentException (init/validate-routes stream-routes batch-routes modes))))))))) (deftest stop-test (testing "the following components execute-function -> actor-stop-fn -> stop-common-states should be stopped" - (let [is-execute-function-called? (atom false) - is-actor-stop-fn-called? (atom false) + (let [is-execute-function-called? (atom false) + is-actor-stop-fn-called? (atom false) is-stop-common-states-called? (atom false) - actor-stop-fn (fn [] (reset! is-actor-stop-fn-called? true))] - (with-redefs [init/execute-function (fn [_ _] (reset! is-execute-function-called? true)) + actor-stop-fn (fn [] (reset! is-actor-stop-fn-called? true))] + (with-redefs [init/execute-function (fn [_ _] (reset! is-execute-function-called? true)) init/stop-common-states (fn [] (reset! is-stop-common-states-called? true))] (init/stop actor-stop-fn {}) (is (true? @is-execute-function-called?)) diff --git a/test/ziggurat/messaging/connection_test.clj b/test/ziggurat/messaging/connection_test.clj index e83c1cc5..ec0a31c5 100644 --- a/test/ziggurat/messaging/connection_test.clj +++ b/test/ziggurat/messaging/connection_test.clj @@ -82,8 +82,7 @@ ziggurat-config (config/ziggurat-config) stream-routes {:default {:handler-fn (constantly :success)}}] (with-redefs [rmq/connect (fn [provided-config] - (println "Tracer Disabled ********** " provided-config) - (println "Tracer Disabled ********** " (.getCorePoolSize (:executor provided-config))) + (.getCorePoolSize (:executor provided-config)) (reset! rmq-connect-called? true) (reset! thread-count (.getCorePoolSize (:executor provided-config))) (orig-rmq-connect provided-config)) diff --git a/test/ziggurat/messaging/producer_test.clj b/test/ziggurat/messaging/producer_test.clj index 8f17f430..8ba103df 100644 --- a/test/ziggurat/messaging/producer_test.clj +++ b/test/ziggurat/messaging/producer_test.clj @@ -70,14 +70,14 @@ (testing "message in channel will be retried in delay queue with suffix 1 if message retry-count exceeds retry count in channel config" (with-redefs [ziggurat-config (constantly (assoc (ziggurat-config) - :stream-router - {:default - {:channels - {:exponential-retry - {:retry {:count 5 - :enabled true - :type :exponential - :queue-timeout-ms 1000}}}}}))] + :stream-router + {:default + {:channels + {:exponential-retry + {:retry {:count 5 + :enabled true + :type :exponential + :queue-timeout-ms 1000}}}}}))] (fix/with-queues {:default {:handler-fn #(constantly nil) :exponential-retry #(constantly nil)}} @@ -90,9 +90,9 @@ (testing "message in channel will be retried with linear queue timeout" (with-redefs [ziggurat-config (constantly (assoc (ziggurat-config) - :stream-router {:default {:channels {:linear-retry {:retry {:count 5 - :enabled true - :queue-timeout-ms 2000}}}}}))] + :stream-router {:default {:channels {:linear-retry {:retry {:count 5 + :enabled true + :queue-timeout-ms 2000}}}}}))] (fix/with-queues {:default {:handler-fn #(constantly nil) :linear-retry #(constantly nil)}} @@ -112,10 +112,10 @@ (testing "message in channel will be retried with exponential timeout calculated from channel specific queue-timeout-ms value" (with-redefs [ziggurat-config (constantly (assoc (ziggurat-config) - :stream-router {:default {:channels {:exponential-retry {:retry {:count 5 - :enabled true - :type :exponential - :queue-timeout-ms 1000}}}}}))] + :stream-router {:default {:channels {:exponential-retry {:retry {:count 5 + :enabled true + :type :exponential + :queue-timeout-ms 1000}}}}}))] (fix/with-queues {:default {:handler-fn #(constantly nil) :exponential-retry #(constantly nil)}} @@ -231,9 +231,9 @@ (deftest retry-with-exponential-backoff-test (testing "message will publish to delay with retry count queue when exponential backoff enabled" (with-redefs [ziggurat-config (constantly (assoc (ziggurat-config) - :retry {:count 5 - :enabled true - :type :exponential}))] + :retry {:count 5 + :enabled true + :type :exponential}))] (testing "message with no retry count will publish to delay queue with suffix 1" (fix/with-queues {:default {:handler-fn #(constantly nil)}} @@ -273,8 +273,8 @@ (let [ziggurat-config (ziggurat-config)] (testing "When retries are enabled" (with-redefs [config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled true - :type :linear}))] + :retry {:enabled true + :type :linear}))] (testing "it does not create queues when stream-routes are not passed" (let [counter (atom 0)] (with-redefs [producer/create-and-bind-queue (fn @@ -435,7 +435,7 @@ (testing "when retries are disabled" (with-redefs [config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false}))] + :retry {:enabled false}))] (testing "it does not create queues when stream-routes are not passed" (let [counter (atom 0)] (with-redefs [producer/create-and-bind-queue (fn @@ -476,8 +476,8 @@ (testing "when retries are disabled" (with-redefs [config/ziggurat-config (constantly (assoc ziggurat-config - :retry {:enabled false} - :stream-router {:default {:channels {:channel-1 {:retry {:enabled false}}}}}))] + :retry {:enabled false} + :stream-router {:default {:channels {:channel-1 {:retry {:enabled false}}}}}))] (testing "it creates instant queues with topic entity for channels only" (with-open [ch (lch/open connection)] @@ -582,50 +582,50 @@ (testing "when retries are enabled" (let [channel :linear-retry] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :stream-router {topic-entity {:channels {channel {:retry {:count 5 - :enabled true - :queue-timeout-ms 2000}}}}}))] + :stream-router {topic-entity {:channels {channel {:retry {:count 5 + :enabled true + :queue-timeout-ms 2000}}}}}))] (is (= 2000 (producer/get-channel-queue-timeout-ms topic-entity channel message)))))) (testing "when exponential backoff are enabled and channel queue timeout defined" (let [channel :exponential-retry] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :stream-router {topic-entity {:channels {channel {:retry {:count 5 - :enabled true - :type :exponential - :queue-timeout-ms 1000}}}}}))] + :stream-router {topic-entity {:channels {channel {:retry {:count 5 + :enabled true + :type :exponential + :queue-timeout-ms 1000}}}}}))] (is (= 7000 (producer/get-channel-queue-timeout-ms topic-entity channel message)))))) (testing "when exponential backoff are enabled and channel queue timeout is not defined" (let [channel :exponential-retry] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :stream-router {topic-entity {:channels {channel {:retry {:count 5 - :enabled true - :type :exponential}}}}}))] + :stream-router {topic-entity {:channels {channel {:retry {:count 5 + :enabled true + :type :exponential}}}}}))] (is (= 700 (producer/get-channel-queue-timeout-ms topic-entity channel message)))))))) (deftest get-queue-timeout-ms-test (testing "when exponential retries are enabled" (let [message (assoc message-payload :retry-count 2)] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :retry {:enabled true - :count 5 - :type :exponential}))] + :retry {:enabled true + :count 5 + :type :exponential}))] (is (= 700 (producer/get-queue-timeout-ms message)))))) (testing "when exponential retries are enabled and retry-count exceeds 25, the max possible timeouts are calculated using 25 as the retry-count" (let [message (assoc message-payload :retry-count 20)] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :retry {:enabled true - :count 50 - :type :exponential}))] + :retry {:enabled true + :count 50 + :type :exponential}))] ;; For 25 max exponential retries, exponent comes to 25-20=5, which makes timeout = 100*(2^5-1) = 3100 (is (= 3100 (producer/get-queue-timeout-ms message)))))) (testing "when exponential retries are enabled with total retries as 25 and if the message has already been retried 24 times, then the queue-timeout is calculated without any failure" (let [message (assoc message-payload :retry-count 1)] (with-redefs [config/ziggurat-config (constantly (assoc (config/ziggurat-config) - :retry {:enabled true - :count 25 - :type :exponential} - :rabbit-mq {:delay {:queue-timeout-ms 5000}}))] + :retry {:enabled true + :count 25 + :type :exponential} + :rabbit-mq {:delay {:queue-timeout-ms 5000}}))] ;; For 25 max exponential retries, exponent comes to 25-1=24, which makes timeout = 5000*(2^24-1) = 83886075000 (is (= 83886075000 (producer/get-queue-timeout-ms message))))))) From b497612a9c20918ab210d7d436ea5f2bebfcd777 Mon Sep 17 00:00:00 2001 From: "shubhang.balkundi" Date: Mon, 11 Apr 2022 13:40:52 +0530 Subject: [PATCH 13/24] fixes ziggurat.init tests --- src/ziggurat/init.clj | 9 +++++++-- src/ziggurat/messaging/channel_pool.clj | 5 ++++- src/ziggurat/messaging/connection.clj | 2 +- test/ziggurat/init_test.clj | 17 ++++++++++------- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/ziggurat/init.clj b/src/ziggurat/init.clj index 6f4ee830..c9e3b5dd 100644 --- a/src/ziggurat/init.clj +++ b/src/ziggurat/init.clj @@ -8,6 +8,7 @@ [mount.core :as mount] [schema.core :as s] [ziggurat.config :refer [ziggurat-config] :as config] + [ziggurat.messaging.channel_pool :as cpool] [ziggurat.kafka-consumer.consumer-driver :as consumer-driver] [ziggurat.kafka-consumer.executor-service :as executor-service] [ziggurat.messaging.connection :as messaging-connection :refer [connection]] @@ -38,7 +39,9 @@ (mount/start)))) (defn- start-rabbitmq-connection [args] - (start* #{#'messaging-connection/connection} args)) + (start* #{#'messaging-connection/consumer-connection} args) + (start* #{#'messaging-connection/connection} args) + (start* #{#'cpool/channel-pool} args)) (defn- start-rabbitmq-consumers [args] (start-rabbitmq-connection args) @@ -77,7 +80,9 @@ (start-rabbitmq-consumers args)) (defn- stop-rabbitmq-connection [] - (mount/stop #'connection)) + (mount/stop #'cpool/channel-pool) + (mount/stop #'connection) + (mount/stop #'messaging-connection/consumer-connection)) (defn stop-kafka-producers [] (mount/stop #'kafka-producers)) diff --git a/src/ziggurat/messaging/channel_pool.clj b/src/ziggurat/messaging/channel_pool.clj index 4bd45cd2..d7dcc190 100644 --- a/src/ziggurat/messaging/channel_pool.clj +++ b/src/ziggurat/messaging/channel_pool.clj @@ -38,8 +38,11 @@ rmq-chan-pool (GenericObjectPool. (RabbitMQChannelFactory. connection) pool-config)] rmq-chan-pool)) +(defn destroy-channel-pool [channel-pool] + (.close channel-pool)) + (defstate channel-pool :start (do (log/info "Creating channel pool") (create-channel-pool c/connection)) :stop (do (log/info "Stopping channel pool") - (.close channel-pool))) + (destroy-channel-pool channel-pool))) diff --git a/src/ziggurat/messaging/connection.clj b/src/ziggurat/messaging/connection.clj index 1a70c4fa..9d139496 100644 --- a/src/ziggurat/messaging/connection.clj +++ b/src/ziggurat/messaging/connection.clj @@ -31,7 +31,7 @@ (defn total-thread-count [] (let [stream-routes (:stream-router (ziggurat-config)) batch-route-count (count (:batch-routes (ziggurat-config))) - worker-count (get-in (ziggurat-config) [:jobs :instant :worker-count]) + worker-count (get-in (ziggurat-config) [:jobs :instant :worker-count] 0) batch-routes-instant-workers (* batch-route-count worker-count)] (reduce (fn [sum [_ route-config]] (+ sum (channel-threads (:channels route-config)) worker-count)) diff --git a/test/ziggurat/init_test.clj b/test/ziggurat/init_test.clj index be99d685..9d1daaa9 100644 --- a/test/ziggurat/init_test.clj +++ b/test/ziggurat/init_test.clj @@ -6,6 +6,7 @@ [ziggurat.messaging.connection :as rmqc] [ziggurat.messaging.consumer :as messaging-consumer] [ziggurat.messaging.producer :as messaging-producer] + [ziggurat.messaging.channel_pool :as cpool] [ziggurat.streams :as streams] [ziggurat.server.test-utils :as tu] [ziggurat.tracer :as tracer] @@ -23,16 +24,18 @@ (deftest start-calls-actor-start-fn-test (testing "The actor start fn starts before the ziggurat state and can read config" (let [result (atom 1)] - (with-redefs [streams/start-streams (fn [_ _] (reset! result (* @result 2))) - streams/stop-streams (constantly nil) + (with-redefs [streams/start-streams (fn [_ _] (reset! result (* @result 2))) + streams/stop-streams (constantly nil) ;; will be called valid modes number of times - rmqc/start-connection (fn [_] (reset! result (* @result 2))) - rmqc/stop-connection (constantly nil) - tracer/create-tracer (fn [] (MockTracer.))] + cpool/create-channel-pool (fn [_] (reset! result (* @result 3))) + rmqc/start-connection (fn [_] (reset! result (* @result 2))) + rmqc/stop-connection (constantly nil) + cpool/destroy-channel-pool (constantly nil) + tracer/create-tracer (fn [] (MockTracer.))] (with-config (do (init/start #(reset! result (+ @result 3)) {} {} [] nil) (init/stop #() nil) - (is (= 16 @result)))))))) + (is (= 96 @result)))))))) (deftest stop-calls-actor-stop-fn-test (testing "The actor stop fn stops before the ziggurat state" @@ -55,7 +58,7 @@ (with-config (do (init/start #() {} {} [] nil) (init/stop #(reset! result (+ @result 3)) nil) - (is (= 5 @result)))))))) + (is (= 7 @result)))))))) (deftest start-calls-make-queues-with-both-streams-and-batch-routes-test (testing "Start calls make queues with both streams and batch routes" From 7e7caa99c0ecc4472f32d8e53fef2906f196e1df Mon Sep 17 00:00:00 2001 From: "shubhang.balkundi" Date: Mon, 11 Apr 2022 17:15:34 +0530 Subject: [PATCH 14/24] fixes config file name --- project.clj | 2 +- src/ziggurat/config.clj | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/project.clj b/project.clj index dda2aa5e..1b88160e 100644 --- a/project.clj +++ b/project.clj @@ -2,7 +2,7 @@ (cemerick.pomegranate.aether/register-wagon-factory! "http" #(org.apache.maven.wagon.providers.http.HttpWagon.)) -(defproject tech.gojek/ziggurat "4.5.3" +(defproject tech.gojek/ziggurat "4.6.0" :description "A stream processing framework to build stateless applications on kafka" :url "https://github.com/gojektech/ziggurat" :license {:name "Apache License, Version 2.0" diff --git a/src/ziggurat/config.clj b/src/ziggurat/config.clj index e4db88aa..a7a9053e 100644 --- a/src/ziggurat/config.clj +++ b/src/ziggurat/config.clj @@ -12,7 +12,7 @@ ^{:static true} [getIn [java.lang.Iterable] Object]] :name tech.gojek.ziggurat.internal.Config)) -(def config-file "config.test.edn") +(def config-file "config.edn") (def default-config {:ziggurat {:nrepl-server {:port 70171} :statsd {:port 8125 From 52f595cbdcf0868131355b7e017f01ec90388b95 Mon Sep 17 00:00:00 2001 From: "shubhang.balkundi" Date: Mon, 11 Apr 2022 17:15:54 +0530 Subject: [PATCH 15/24] fixes channel leak while creating and binding queues --- src/ziggurat/messaging/producer.clj | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ziggurat/messaging/producer.clj b/src/ziggurat/messaging/producer.clj index d9587ace..e4767211 100644 --- a/src/ziggurat/messaging/producer.clj +++ b/src/ziggurat/messaging/producer.clj @@ -78,12 +78,12 @@ (try (let [props (if dead-letter-exchange {"x-dead-letter-exchange" dead-letter-exchange} - {}) - ch (lch/open connection)] - (create-queue queue-name props ch) - (declare-exchange ch exchange-name) - (bind-queue-to-exchange ch queue-name exchange-name) - (set-ha-policy queue-name exchange-name (get-in config [:ziggurat :rabbit-mq-connection]))) + {})] + (with-open [ch (lch/open connection)] + (create-queue queue-name props ch) + (declare-exchange ch exchange-name) + (bind-queue-to-exchange ch queue-name exchange-name) + (set-ha-policy queue-name exchange-name (get-in config [:ziggurat :rabbit-mq-connection])))) (catch Exception e (log/error e "Error while declaring RabbitMQ queues") (throw e))))) From 09ac035f06d7431fb36ee1dcdaa55193d083469f Mon Sep 17 00:00:00 2001 From: "shubhang.balkundi" Date: Mon, 11 Apr 2022 17:29:00 +0530 Subject: [PATCH 16/24] adds a connection-name prop while creating rabbitmq connections --- src/ziggurat/messaging/connection.clj | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ziggurat/messaging/connection.clj b/src/ziggurat/messaging/connection.clj index 9d139496..e6611c6e 100644 --- a/src/ziggurat/messaging/connection.clj +++ b/src/ziggurat/messaging/connection.clj @@ -56,9 +56,11 @@ (defn- get-connection-config [is-producer?] (if is-producer? - (:rabbit-mq-connection (ziggurat-config)) (assoc (:rabbit-mq-connection (ziggurat-config)) - :executor (Executors/newFixedThreadPool (total-thread-count))))) + :connection-name "producer") + (assoc (:rabbit-mq-connection (ziggurat-config)) + :executor (Executors/newFixedThreadPool (total-thread-count)) + :connection-name "consumer"))) (defn- start-connection "is-producer? - defines whether the connection is being created for producers or consumers From d47042b19c0401412eff830c8bd2b6c32dd60cdb Mon Sep 17 00:00:00 2001 From: "shubhang.balkundi" Date: Mon, 11 Apr 2022 17:36:41 +0530 Subject: [PATCH 17/24] removes unused test --- test/ziggurat/messaging/producer_test.clj | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/test/ziggurat/messaging/producer_test.clj b/test/ziggurat/messaging/producer_test.clj index 8ba103df..b8ad3bdc 100644 --- a/test/ziggurat/messaging/producer_test.clj +++ b/test/ziggurat/messaging/producer_test.clj @@ -628,14 +628,3 @@ :rabbit-mq {:delay {:queue-timeout-ms 5000}}))] ;; For 25 max exponential retries, exponent comes to 25-1=24, which makes timeout = 5000*(2^24-1) = 83886075000 (is (= 83886075000 (producer/get-queue-timeout-ms message))))))) - -;(testing "the channel borrowed from the pool should always be returned back to the pool once the publish lb/publish returns" -; (let [call-count (atom 0) -; expected-call-count 10] -; (with-redefs [producer/return-to-pool (fn [_ _] (swap! call-count inc)) -; lb/publish (fn [_ _ _ _ _])] -; (let [futures (take expected-call-count (repeatedly #(future (producer/publish "pool-test" {:a "hello world"}))))] -; (doall futures) -; (map #(deref %) futures))) -; (is (= @call-count expected-call-count)))) - From e2541265321aabb54f641c639fdc04882788f8b5 Mon Sep 17 00:00:00 2001 From: "shubhang.balkundi" Date: Mon, 11 Apr 2022 20:41:19 +0530 Subject: [PATCH 18/24] uses mount/only to start and stop rabbit-mq states --- src/ziggurat/init.clj | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/ziggurat/init.clj b/src/ziggurat/init.clj index c9e3b5dd..ac2298c3 100644 --- a/src/ziggurat/init.clj +++ b/src/ziggurat/init.clj @@ -39,9 +39,11 @@ (mount/start)))) (defn- start-rabbitmq-connection [args] - (start* #{#'messaging-connection/consumer-connection} args) - (start* #{#'messaging-connection/connection} args) - (start* #{#'cpool/channel-pool} args)) + (-> (mount/only #{#'messaging-connection/consumer-connection + #'messaging-connection/connection + #'cpool/channel-pool}) + (mount/with-args args) + (mount/start))) (defn- start-rabbitmq-consumers [args] (start-rabbitmq-connection args) @@ -80,9 +82,10 @@ (start-rabbitmq-consumers args)) (defn- stop-rabbitmq-connection [] - (mount/stop #'cpool/channel-pool) - (mount/stop #'connection) - (mount/stop #'messaging-connection/consumer-connection)) + (-> (mount/only #{#'connection + #'cpool/channel-pool + #'messaging-connection/consumer-connection}) + (mount/stop))) (defn stop-kafka-producers [] (mount/stop #'kafka-producers)) From 027aa2b0e582d759fb08b98f722ebea60e0a85f7 Mon Sep 17 00:00:00 2001 From: "shubhang.balkundi" Date: Mon, 11 Apr 2022 20:43:40 +0530 Subject: [PATCH 19/24] enables jmx for rabbit-mq channel pool --- src/ziggurat/messaging/channel_pool.clj | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ziggurat/messaging/channel_pool.clj b/src/ziggurat/messaging/channel_pool.clj index d7dcc190..89aaacec 100644 --- a/src/ziggurat/messaging/channel_pool.clj +++ b/src/ziggurat/messaging/channel_pool.clj @@ -31,7 +31,10 @@ (.setMinIdle (:min-idle merged-config)) (.setMaxIdle (:max-idle merged-config)) (.setMaxTotal (+ (:min-idle merged-config) total-thread-count)) - (.setTestOnBorrow true)))) + (.setTestOnBorrow true) + (.setJmxEnabled true) + (.setJmxNameBase "rabbitmq-producer-channel-pool") + (.setJmxNamePrefix "ziggurat")))) (defn create-channel-pool [^Connection connection] (let [pool-config (create-object-pool-config (get-in zc/ziggurat-config [:rabbit-mq-connection :channel-pool])) From 6a7221b6397db7f5a9e7755db6bd3bcecf9b47d5 Mon Sep 17 00:00:00 2001 From: Anmol Vijaywargiya Date: Tue, 12 Apr 2022 12:30:02 +0530 Subject: [PATCH 20/24] [Shubhang|Anmol] Rename connection for rabbitmq producers to producer-connection --- src/ziggurat/init.clj | 9 ++--- src/ziggurat/messaging/channel_pool.clj | 2 +- src/ziggurat/messaging/connection.clj | 6 ++-- src/ziggurat/messaging/producer.clj | 8 +++-- test/ziggurat/fixtures.clj | 8 ++--- test/ziggurat/mapper_test.clj | 2 +- test/ziggurat/messaging/channel_pool_test.clj | 6 ++-- test/ziggurat/messaging/connection_test.clj | 34 +++++++++---------- test/ziggurat/messaging/producer_test.clj | 12 +++---- test/ziggurat/util/rabbitmq.clj | 6 ++-- 10 files changed, 49 insertions(+), 44 deletions(-) diff --git a/src/ziggurat/init.clj b/src/ziggurat/init.clj index ac2298c3..287bd7ed 100644 --- a/src/ziggurat/init.clj +++ b/src/ziggurat/init.clj @@ -11,7 +11,7 @@ [ziggurat.messaging.channel_pool :as cpool] [ziggurat.kafka-consumer.consumer-driver :as consumer-driver] [ziggurat.kafka-consumer.executor-service :as executor-service] - [ziggurat.messaging.connection :as messaging-connection :refer [connection]] + [ziggurat.messaging.connection :as messaging-connection :refer [producer-connection, consumer-connection]] [ziggurat.messaging.consumer :as messaging-consumer] [ziggurat.messaging.producer :as messaging-producer] [ziggurat.metrics :as metrics] @@ -40,7 +40,7 @@ (defn- start-rabbitmq-connection [args] (-> (mount/only #{#'messaging-connection/consumer-connection - #'messaging-connection/connection + #'messaging-connection/producer-connection #'cpool/channel-pool}) (mount/with-args args) (mount/start))) @@ -82,7 +82,7 @@ (start-rabbitmq-consumers args)) (defn- stop-rabbitmq-connection [] - (-> (mount/only #{#'connection + (-> (mount/only #{#'producer-connection #'cpool/channel-pool #'messaging-connection/consumer-connection}) (mount/stop))) @@ -158,7 +158,8 @@ (defn stop-common-states [] (mount/stop #'config/config #'metrics/statsd-reporter - #'connection + #'producer-connection + #'consumer-connection #'nrepl-server/server #'tracer/tracer)) diff --git a/src/ziggurat/messaging/channel_pool.clj b/src/ziggurat/messaging/channel_pool.clj index 89aaacec..47457aab 100644 --- a/src/ziggurat/messaging/channel_pool.clj +++ b/src/ziggurat/messaging/channel_pool.clj @@ -46,6 +46,6 @@ (defstate channel-pool :start (do (log/info "Creating channel pool") - (create-channel-pool c/connection)) + (create-channel-pool c/producer-connection)) :stop (do (log/info "Stopping channel pool") (destroy-channel-pool channel-pool))) diff --git a/src/ziggurat/messaging/connection.clj b/src/ziggurat/messaging/connection.clj index e6611c6e..d81ad10f 100644 --- a/src/ziggurat/messaging/connection.clj +++ b/src/ziggurat/messaging/connection.clj @@ -87,14 +87,16 @@ (log/info "Closing the RabbitMQ connection") (rmq/close conn))) +(declare consumer-connection) (defstate consumer-connection :start (do (log/info "Creating consumer connection") (start-connection false)) :stop (do (log/info "Stopping consume connection") (stop-connection consumer-connection))) -(defstate connection +(declare producer-connection) +(defstate producer-connection :start (do (log/info "Creating producer connection") (start-connection true)) :stop (do (log/info "Stopping producer connection") - (stop-connection connection))) + (stop-connection producer-connection))) diff --git a/src/ziggurat/messaging/producer.clj b/src/ziggurat/messaging/producer.clj index e4767211..46cb3d43 100644 --- a/src/ziggurat/messaging/producer.clj +++ b/src/ziggurat/messaging/producer.clj @@ -8,7 +8,7 @@ [ziggurat.messaging.channel_pool :as cpool] [taoensso.nippy :as nippy] [ziggurat.config :refer [config ziggurat-config rabbitmq-config channel-retry-config]] - [ziggurat.messaging.connection :refer [connection is-connection-required?]] + [ziggurat.messaging.connection :refer [producer-connection is-connection-required?]] [ziggurat.messaging.util :as util] [ziggurat.metrics :as metrics]) (:import (com.rabbitmq.client AlreadyClosedException Channel) @@ -79,7 +79,7 @@ (let [props (if dead-letter-exchange {"x-dead-letter-exchange" dead-letter-exchange} {})] - (with-open [ch (lch/open connection)] + (with-open [ch (lch/open producer-connection)] (create-queue queue-name props ch) (declare-exchange ch exchange-name) (bind-queue-to-exchange ch queue-name exchange-name) @@ -131,7 +131,9 @@ (finally (return-to-pool cpool/channel-pool ch)))) (catch Exception e (log/error "Exception occurred while borrowing a channel from the pool") - (metrics/increment-count ["rabbitmq" "publish" "channel_borrow"] {:topic-entity (name (:topic-entity message-payload))})))) + (metrics/increment-count ["rabbitmq" "publish" "channel_borrow"] {:topic-entity (name (:topic-entity message-payload))}) + true ;TODO Evaluate whether message needs to be retried in case of borrow exceptions + ))) (defn publish ([exchange message-payload] diff --git a/test/ziggurat/fixtures.clj b/test/ziggurat/fixtures.clj index 2d9ca414..ec574ad6 100644 --- a/test/ziggurat/fixtures.clj +++ b/test/ziggurat/fixtures.clj @@ -6,7 +6,7 @@ [mount.core :as mount] [ziggurat.config :as config] [ziggurat.kafka-consumer.executor-service :refer [thread-pool]] - [ziggurat.messaging.connection :refer [connection consumer-connection]] + [ziggurat.messaging.connection :refer [producer-connection consumer-connection]] [ziggurat.messaging.channel_pool :refer [channel-pool]] [ziggurat.messaging.producer :as pr] [ziggurat.messaging.util :as util] @@ -88,7 +88,7 @@ (:exchange-name (exchange-type (config/rabbitmq-config)))) (defn delete-queues [stream-routes] - (with-open [ch (lch/open connection)] + (with-open [ch (lch/open producer-connection)] (doseq [topic-entity (keys stream-routes)] (let [topic-identifier (name topic-entity) channels (util/get-channel-names stream-routes topic-entity)] @@ -101,7 +101,7 @@ (lq/delete ch (util/prefixed-channel-name topic-identifier channel (get-queue-name :delay)))))))) (defn delete-exchanges [stream-routes] - (with-open [ch (lch/open connection)] + (with-open [ch (lch/open producer-connection)] (doseq [topic-entity (keys stream-routes)] (let [topic-identifier (name topic-entity) channels (util/get-channel-names stream-routes topic-entity)] @@ -120,7 +120,7 @@ (mount-tracer) (-> - (mount/only [#'connection #'consumer-connection #'channel-pool]) + (mount/only [#'producer-connection #'consumer-connection #'channel-pool]) (mount/with-args {:stream-routes stream-routes}) (mount/start)) (f) diff --git a/test/ziggurat/mapper_test.clj b/test/ziggurat/mapper_test.clj index e7ea4d8f..4b64b007 100644 --- a/test/ziggurat/mapper_test.clj +++ b/test/ziggurat/mapper_test.clj @@ -4,7 +4,7 @@ [ziggurat.config :refer [ziggurat-config]] [ziggurat.fixtures :as fix] [ziggurat.mapper :refer :all] - [ziggurat.messaging.connection :refer [connection]] + [ziggurat.messaging.connection :refer [producer-connection]] [ziggurat.metrics :as metrics] [ziggurat.util.rabbitmq :as rmq] [ziggurat.message-payload :as mp] diff --git a/test/ziggurat/messaging/channel_pool_test.clj b/test/ziggurat/messaging/channel_pool_test.clj index 50f76e19..db2fb61d 100644 --- a/test/ziggurat/messaging/channel_pool_test.clj +++ b/test/ziggurat/messaging/channel_pool_test.clj @@ -1,7 +1,7 @@ (ns ziggurat.messaging.channel-pool-test (:require [clojure.test :refer :all] [ziggurat.messaging.channel_pool :as cpool] - [ziggurat.messaging.connection :refer [connection]] + [ziggurat.messaging.connection :refer [producer-connection]] [ziggurat.fixtures :as fix]) (:import (org.apache.commons.pool2.impl GenericObjectPoolConfig GenericObjectPool) (java.time Duration) @@ -47,8 +47,8 @@ (deftest pool-borrow-return-test (testing "it should invalidate a closed channel and return a new channel on borrow" - (mount.core/start #'connection) - (let [channel-pool ^GenericObjectPool (cpool/create-channel-pool connection) + (mount.core/start #'producer-connection) + (let [channel-pool ^GenericObjectPool (cpool/create-channel-pool producer-connection) _ (doto channel-pool (.setMaxTotal 1) (.setMinIdle 0) diff --git a/test/ziggurat/messaging/connection_test.clj b/test/ziggurat/messaging/connection_test.clj index ec0a31c5..10dc8e71 100644 --- a/test/ziggurat/messaging/connection_test.clj +++ b/test/ziggurat/messaging/connection_test.clj @@ -4,7 +4,7 @@ [langohr.core :as rmq] [mount.core :as mount] [ziggurat.config :as config] - [ziggurat.messaging.connection :as mc :refer [connection, consumer-connection, create-connection]] + [ziggurat.messaging.connection :as mc :refer [producer-connection, consumer-connection, create-connection]] [ziggurat.util.error :refer [report-error]])) (use-fixtures :once fix/mount-config-with-tracer) @@ -45,10 +45,10 @@ :retry {:enabled true} :stream-router {:default {:channels {:channel-1 {:worker-count 10}}}} :tracer {:enabled false}))] - (-> (mount/only #{#'connection}) + (-> (mount/only #{#'producer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) - (mount/stop #'connection) + (mount/stop #'producer-connection) (is (false? @executor-present?)))))) (deftest start-connection-test-with-tracer-disabled @@ -109,10 +109,10 @@ config/ziggurat-config (constantly (assoc ziggurat-config :retry {:enabled true} :tracer {:enabled false}))] - (-> (mount/only #{#'connection}) + (-> (mount/only #{#'producer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) - (mount/stop #'connection) + (mount/stop #'producer-connection) (is @rmq-connect-called?)))) (testing "if retry is disabled and channels are not present it should not create connection" @@ -126,10 +126,10 @@ config/ziggurat-config (constantly (-> ziggurat-config (assoc :retry {:enabled false}) (dissoc :tracer)))] - (-> (mount/only #{#'connection}) + (-> (mount/only #{#'producer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) - (mount/stop #'connection) + (mount/stop #'producer-connection) (is (not @rmq-connect-called?))))) (testing "[consumer-connection] if retry is disabled and channels are not present it should not create connection" @@ -163,10 +163,10 @@ config/ziggurat-config (constantly (assoc ziggurat-config :retry {:enabled false} :tracer {:enabled false}))] - (-> (mount/only #{#'connection}) + (-> (mount/only #{#'producer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) - (mount/stop #'connection) + (mount/stop #'producer-connection) (is @rmq-connect-called?)))) (testing "[consumer-connection] should provide the correct number of threads for the thread pool when channels are not present" @@ -235,10 +235,10 @@ (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config :retry {:enabled true}))] - (-> (mount/only #{#'connection}) + (-> (mount/only #{#'producer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) - (mount/stop #'connection) + (mount/stop #'producer-connection) (is @create-connect-called?)))) (testing "if retry is disabled and channels are not present it should not create connection" @@ -251,10 +251,10 @@ (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config :retry {:enabled false}))] - (-> (mount/only #{#'connection}) + (-> (mount/only #{#'producer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) - (mount/stop #'connection) + (mount/stop #'producer-connection) (is (not @create-connect-called?))))) (testing "if retry is disabled and channels are present it should create connection" @@ -270,10 +270,10 @@ (orig-create-conn provided-config tracer-enabled)) config/ziggurat-config (constantly (assoc ziggurat-config :retry {:enabled false}))] - (-> (mount/only #{#'connection}) + (-> (mount/only #{#'producer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) - (mount/stop #'connection) + (mount/stop #'producer-connection) (is @create-connect-called?)))) (testing "should provide the correct number of threads for the thread pool for multiple channels" @@ -334,9 +334,9 @@ (throw (Exception. "Error"))) report-error (fn [_ _] (reset! report-fn-called? true))] (try - (-> (mount/only #{#'connection}) + (-> (mount/only #{#'producer-connection}) (mount/with-args {:stream-routes stream-routes}) (mount/start)) (catch Exception e - (mount/stop #'connection))) + (mount/stop #'producer-connection))) (is @report-fn-called?))))) \ No newline at end of file diff --git a/test/ziggurat/messaging/producer_test.clj b/test/ziggurat/messaging/producer_test.clj index b8ad3bdc..f3ea7b33 100644 --- a/test/ziggurat/messaging/producer_test.clj +++ b/test/ziggurat/messaging/producer_test.clj @@ -5,7 +5,7 @@ [langohr.queue :as lq] [ziggurat.config :refer [rabbitmq-config ziggurat-config channel-retry-config]] [ziggurat.fixtures :as fix] - [ziggurat.messaging.connection :refer [connection]] + [ziggurat.messaging.connection :refer [producer-connection]] [ziggurat.messaging.producer :as producer] [ziggurat.messaging.util :as util] [ziggurat.util.rabbitmq :as rmq] @@ -313,7 +313,7 @@ (is (= (* (count stream-routes) 3) @counter))))) (testing "it creates queues with topic entity from stream routes" - (with-open [ch (lch/open connection)] + (with-open [ch (lch/open producer-connection)] (let [stream-routes {:default {:handler-fn #(constantly :success)}} instant-queue-name (util/prefixed-queue-name "default" (:queue-name (:instant (rabbitmq-config)))) @@ -340,7 +340,7 @@ (le/delete ch instant-exchange-name) (le/delete ch dead-exchange-name)))) (testing "it creates queues with suffixes in the range [1, retry-count] when exponential backoff is enabled" - (with-open [ch (lch/open connection)] + (with-open [ch (lch/open producer-connection)] (let [stream-routes {:default {:handler-fn #(constantly :success)}} retry-count (get-in ziggurat-config [:retry :count]) instant-queue-name (util/prefixed-queue-name "default" (:queue-name (:instant (rabbitmq-config)))) @@ -369,7 +369,7 @@ (lq/delete ch (exponential-delay-queue-name s)) (le/delete ch (exponential-delay-exchange-name s))))))) (testing "it creates queues with suffixes in the range [1, 25] when exponential backoff is enabled and retry-count is more than 25" - (with-open [ch (lch/open connection)] + (with-open [ch (lch/open producer-connection)] (let [stream-routes {:default {:handler-fn #(constantly :success)}} instant-queue-name (util/prefixed-queue-name "default" (:queue-name (:instant (rabbitmq-config)))) instant-exchange-name (util/prefixed-queue-name "default" (:exchange-name (:instant (rabbitmq-config)))) @@ -445,7 +445,7 @@ (is (= 0 @counter))))) (testing "it creates queues with topic entity for channels only" - (with-open [ch (lch/open connection)] + (with-open [ch (lch/open producer-connection)] (let [stream-routes {:default {:handler-fn #(constantly :success) :channel-1 #(constantly :success)}} instant-queue-suffix (:queue-name (:instant (rabbitmq-config))) instant-exchange-suffix (:exchange-name (:instant (rabbitmq-config))) @@ -480,7 +480,7 @@ :stream-router {:default {:channels {:channel-1 {:retry {:enabled false}}}}}))] (testing "it creates instant queues with topic entity for channels only" - (with-open [ch (lch/open connection)] + (with-open [ch (lch/open producer-connection)] (let [stream-routes {:default {:handler-fn #(constantly :success) :channel-1 #(constantly :success)}} instant-queue-suffix (:queue-name (:instant (rabbitmq-config))) instant-exchange-suffix (:exchange-name (:instant (rabbitmq-config))) diff --git a/test/ziggurat/util/rabbitmq.clj b/test/ziggurat/util/rabbitmq.clj index 8cbd67fe..5092bb57 100644 --- a/test/ziggurat/util/rabbitmq.clj +++ b/test/ziggurat/util/rabbitmq.clj @@ -3,7 +3,7 @@ [langohr.channel :as lch] [langohr.basic :as lb] [ziggurat.config :refer [rabbitmq-config]] - [ziggurat.messaging.connection :refer [connection]] + [ziggurat.messaging.connection :refer [producer-connection]] [ziggurat.messaging.consumer :as consumer] [ziggurat.messaging.util :refer [prefixed-channel-name]] [ziggurat.messaging.producer :refer [delay-queue-name]] @@ -13,7 +13,7 @@ (:import (com.rabbitmq.client AlreadyClosedException Channel))) (defn- get-msg-from-rabbitmq [queue-name topic-name] - (with-open [ch (lch/open connection)] + (with-open [ch (lch/open producer-connection)] (try (let [[meta payload] (lb/get ch queue-name false)] (when (seq payload) @@ -22,7 +22,7 @@ nil)))) (defn- get-msg-from-rabbitmq-without-ack [queue-name topic-name] - (with-open [ch (lch/open connection)] + (with-open [ch (lch/open producer-connection)] (try (let [[meta payload] (lb/get ch queue-name false)] (when (seq payload) From 555def17df0f10884d108c67853d04253b00a56f Mon Sep 17 00:00:00 2001 From: Anmol Vijaywargiya Date: Tue, 12 Apr 2022 12:41:08 +0530 Subject: [PATCH 21/24] [Shubhang|Anmol] Address PR comments --- src/ziggurat/init.clj | 10 ++++------ src/ziggurat/messaging/channel_pool.clj | 2 ++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ziggurat/init.clj b/src/ziggurat/init.clj index 287bd7ed..ecfc29fa 100644 --- a/src/ziggurat/init.clj +++ b/src/ziggurat/init.clj @@ -39,11 +39,9 @@ (mount/start)))) (defn- start-rabbitmq-connection [args] - (-> (mount/only #{#'messaging-connection/consumer-connection - #'messaging-connection/producer-connection - #'cpool/channel-pool}) - (mount/with-args args) - (mount/start))) + (start* #{#'consumer-connection + #'producer-connection + #'cpool/channel-pool} args)) (defn- start-rabbitmq-consumers [args] (start-rabbitmq-connection args) @@ -84,7 +82,7 @@ (defn- stop-rabbitmq-connection [] (-> (mount/only #{#'producer-connection #'cpool/channel-pool - #'messaging-connection/consumer-connection}) + #'consumer-connection}) (mount/stop))) (defn stop-kafka-producers [] diff --git a/src/ziggurat/messaging/channel_pool.clj b/src/ziggurat/messaging/channel_pool.clj index 47457aab..5e29d194 100644 --- a/src/ziggurat/messaging/channel_pool.clj +++ b/src/ziggurat/messaging/channel_pool.clj @@ -44,6 +44,8 @@ (defn destroy-channel-pool [channel-pool] (.close channel-pool)) +(declare channel-pool) + (defstate channel-pool :start (do (log/info "Creating channel pool") (create-channel-pool c/producer-connection)) From 8cca3ab25ae49a6e3a1abe09b85b3d1360721e34 Mon Sep 17 00:00:00 2001 From: Anmol Vijaywargiya Date: Tue, 12 Apr 2022 14:25:17 +0530 Subject: [PATCH 22/24] Use producer connection --- src/ziggurat/messaging/consumer.clj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ziggurat/messaging/consumer.clj b/src/ziggurat/messaging/consumer.clj index c7769b9a..f2e5f035 100644 --- a/src/ziggurat/messaging/consumer.clj +++ b/src/ziggurat/messaging/consumer.clj @@ -8,7 +8,7 @@ [ziggurat.messaging.channel_pool :as cpool] [ziggurat.kafka-consumer.consumer-handler :as ch] [ziggurat.mapper :as mpr] - [ziggurat.messaging.connection :refer [consumer-connection]] + [ziggurat.messaging.connection :refer [consumer-connection, producer-connection]] [ziggurat.messaging.util :as util] [ziggurat.metrics :as metrics] [ziggurat.util.error :refer [report-error]] @@ -109,7 +109,7 @@ "This method deletes `count` number of messages from RabbitMQ dead-letter queue for topic `topic-entity` and channel `channel`." [topic-entity channel count] - (with-open [ch (lch/open connection)] + (with-open [ch (lch/open producer-connection)] (let [queue-name (construct-queue-name topic-entity channel)] (doall (for [_ (range count)] (lb/get ch queue-name true)))))) From 5f73745292d1cd06c66b1206b3d7e28cb0d4622b Mon Sep 17 00:00:00 2001 From: "shubhang.balkundi" Date: Tue, 12 Apr 2022 15:22:03 +0530 Subject: [PATCH 23/24] sets JMX prefix name for GenericObjectPool --- src/ziggurat/messaging/channel_pool.clj | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/ziggurat/messaging/channel_pool.clj b/src/ziggurat/messaging/channel_pool.clj index 5e29d194..2b36e546 100644 --- a/src/ziggurat/messaging/channel_pool.clj +++ b/src/ziggurat/messaging/channel_pool.clj @@ -32,9 +32,7 @@ (.setMaxIdle (:max-idle merged-config)) (.setMaxTotal (+ (:min-idle merged-config) total-thread-count)) (.setTestOnBorrow true) - (.setJmxEnabled true) - (.setJmxNameBase "rabbitmq-producer-channel-pool") - (.setJmxNamePrefix "ziggurat")))) + (.setJmxNamePrefix "zig-rabbitmq-ch-pool")))) (defn create-channel-pool [^Connection connection] (let [pool-config (create-object-pool-config (get-in zc/ziggurat-config [:rabbit-mq-connection :channel-pool])) From 23165fca2e3fca62f9aa3457ff4ecb96cda9827e Mon Sep 17 00:00:00 2001 From: "shubhang.balkundi" Date: Wed, 13 Apr 2022 15:51:36 +0530 Subject: [PATCH 24/24] fixes code formatting --- src/ziggurat/messaging/producer.clj | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ziggurat/messaging/producer.clj b/src/ziggurat/messaging/producer.clj index 46cb3d43..7493cacb 100644 --- a/src/ziggurat/messaging/producer.clj +++ b/src/ziggurat/messaging/producer.clj @@ -55,8 +55,7 @@ (let [host-endpoint (str "http://" (first hosts) ":" (get rmq-config :admin-port 15672)) resp (set-ha-policy-on-host host-endpoint username password ha-policy-body exchange-name queue-name) remaining-hosts (rest hosts)] - (when (and (nil? resp) - (pos? (count remaining-hosts))) + (when (and (nil? resp) (pos? (count remaining-hosts))) (recur remaining-hosts)))))) (defn- declare-exchange [ch exchange] @@ -83,7 +82,8 @@ (create-queue queue-name props ch) (declare-exchange ch exchange-name) (bind-queue-to-exchange ch queue-name exchange-name) - (set-ha-policy queue-name exchange-name (get-in config [:ziggurat :rabbit-mq-connection])))) + (set-ha-policy queue-name exchange-name (get-in config [:ziggurat :rabbit-mq-connection])) ;TODO remove this + )) (catch Exception e (log/error e "Error while declaring RabbitMQ queues") (throw e)))))