Skip to content

Commit a9ec7b2

Browse files
jsavyasachidakrone
andauthored
fix(conn-mgr): add :max-total, correct misleading :threads docs (#633) (#663)
The :threads option for make-reusable-conn-manager and make-reusable-async-conn-manager never controlled a thread count; it is passed straight to PoolingHttpClientConnectionManager.setMaxTotal, i.e. the pool's maximum total connections. The docstring described it as a thread count, which is misleading. Add :max-total as an accurately-named option and keep :threads working as a deprecated alias (:max-total wins if both are supplied). Document the distinction. Fixes #633. Co-authored-by: Lee Hinman <dakrone@users.noreply.github.com>
1 parent 87da966 commit a9ec7b2

2 files changed

Lines changed: 24 additions & 7 deletions

File tree

src/clj_http/conn_mgr.clj

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -238,8 +238,11 @@
238238
239239
:timeout - Time that connections are left open before automatically closing
240240
default: 5
241-
:threads - Maximum number of threads that will be used for connecting
241+
:max-total - Maximum number of total connections kept in the pool
242242
default: 4
243+
:threads - Deprecated alias for :max-total. Despite the name, this option
244+
has never controlled a number of threads; it sets the pool's maximum
245+
total connections. Prefer :max-total. If both are given, :max-total wins.
243246
:default-per-route - Maximum number of simultaneous connections per host
244247
default: 2
245248
:insecure? - Boolean flag to specify allowing insecure HTTPS connections
@@ -264,14 +267,14 @@
264267
will be used."
265268
[opts]
266269
(let [timeout (or (:timeout opts) 5)
267-
threads (or (:threads opts) 4)
270+
max-total (or (:max-total opts) (:threads opts) 4)
268271
default-per-route (:default-per-route opts)
269272
insecure? (opt opts :insecure)
270-
leftovers (dissoc opts :timeout :threads :insecure? :insecure)
273+
leftovers (dissoc opts :timeout :threads :max-total :insecure? :insecure)
271274
conn-man (make-reusable-conn-manager* (merge {:timeout timeout
272275
:insecure? insecure?}
273276
leftovers))]
274-
(.setMaxTotal conn-man threads)
277+
(.setMaxTotal conn-man max-total)
275278
(when default-per-route
276279
(.setDefaultMaxPerRoute conn-man default-per-route))
277280
conn-man))
@@ -327,13 +330,13 @@
327330
will be used."
328331
[opts]
329332
(let [timeout (or (:timeout opts) 5)
330-
threads (or (:threads opts) 4)
333+
max-total (or (:max-total opts) (:threads opts) 4)
331334
default-per-route (:default-per-route opts)
332335
insecure? (opt opts :insecure)
333-
leftovers (dissoc opts :timeout :threads :insecure? :insecure)
336+
leftovers (dissoc opts :timeout :threads :max-total :insecure? :insecure)
334337
conn-man (make-reusable-async-conn-manager*
335338
(merge {:timeout timeout :insecure? insecure?} leftovers))]
336-
(.setMaxTotal conn-man threads)
339+
(.setMaxTotal conn-man max-total)
337340
(when default-per-route
338341
(.setDefaultMaxPerRoute conn-man default-per-route))
339342
conn-man))

test/clj_http/test/conn_mgr_test.clj

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,3 +150,17 @@
150150
(is (false? (conn-mgr/reusable? async)))
151151
(is (true? (conn-mgr/reusable? async-reusable)))
152152
(is (true? (conn-mgr/reusable? async-reuseable)))))
153+
154+
(deftest max-total-and-threads-alias
155+
(testing ":threads still sets the pool's max total connections (back-compat)"
156+
(let [cm (conn-mgr/make-reusable-conn-manager {:threads 7})]
157+
(is (= 7 (.getMaxTotal cm)))))
158+
(testing ":max-total sets the pool's max total connections"
159+
(let [cm (conn-mgr/make-reusable-conn-manager {:max-total 9})]
160+
(is (= 9 (.getMaxTotal cm)))))
161+
(testing ":max-total takes precedence when both are given"
162+
(let [cm (conn-mgr/make-reusable-conn-manager {:max-total 9 :threads 3})]
163+
(is (= 9 (.getMaxTotal cm)))))
164+
(testing "default max total is 4"
165+
(let [cm (conn-mgr/make-reusable-conn-manager {})]
166+
(is (= 4 (.getMaxTotal cm))))))

0 commit comments

Comments
 (0)