From dd4d88dd085b00fd81ce1456b27997fe30d4e650 Mon Sep 17 00:00:00 2001
From: Zohar Jackson <zoharj@gmail.com>
Date: Fri, 31 Jul 2026 18:45:06 -0400
Subject: [PATCH] Raise the urllib3 pool maxsize to 10 and make it configurable

BaseClient builds its PoolManager with num_pools but never maxsize, so urllib3's
default of ONE connection per host applies. num_pools caps the number of distinct
HOST pools, which does not help a client that talks to a single host from many
threads -- the two are often conflated, and passing a large num_pools looks like
it should raise concurrency but does nothing.

The effect under threads is a TLS handshake storm. Threads that lose the race for
the single pooled connection open their own, cannot return it ("Connection pool is
full, discarding connection"), so it is closed and the next request renegotiates
TLS. Measured against the trades endpoint over 240 calls:

    single-threaded    0.0074 CPU-s per call,   0 pool-full warnings
    20 threads         0.1309 CPU-s per call,  70 pool-full warnings  (18x CPU)

and 40 threads were SLOWER than 20. With maxsize raised the warnings disappear,
CPU per call drops 2.9x, and throughput scales with threads again (25.7 -> 66.3
requests/sec in that benchmark).

Adds a `maxsize` parameter to RESTClient, threaded through to the PoolManager,
defaulting to 10.

The new default intentionally changes behaviour for every user, including those
who never touch the parameter -- that is the point of the change. maxsize=1 is not
a sensible ceiling for a client whose own constructor advertises pooling, and a
user hitting it has no way to discover why their threads are slow: the symptom
looks like a slow API, not a client setting. Fixing it only for people who find
the new argument would leave the common case broken.

10 matches both the existing num_pools default and requests' HTTPAdapter default,
so it is a conventional value rather than a tuned one. maxsize is a cap and not a
preallocation, so a single-threaded client still opens exactly one connection and
sees no difference; the change is felt only by concurrent callers, who are the
ones currently paying for it.

On BaseClient it is an optional keyword rather than a required one, so existing
subclasses that call super().__init__ keep working.

Tests cover the default, an explicit value, and that it is independent of
num_pools.
---
 massive/rest/__init__.py          |  5 +++++
 massive/rest/base.py              |  6 ++++++
 test_rest/test_connection_pool.py | 22 ++++++++++++++++++++++
 3 files changed, 33 insertions(+)
 create mode 100644 test_rest/test_connection_pool.py

diff --git a/massive/rest/__init__.py b/massive/rest/__init__.py
index 5a00da5a..0fc42de3 100644
--- a/massive/rest/__init__.py
+++ b/massive/rest/__init__.py
@@ -54,6 +54,9 @@ def __init__(
         connect_timeout: float = 10.0,
         read_timeout: float = 10.0,
         num_pools: int = 10,
+        # Per-host connection limit. 10 rather than urllib3's default of 1 so that
+        # threaded clients work sensibly out of the box; see BaseClient.
+        maxsize: int = 10,
         retries: int = 3,
         base: str = BASE,
         pagination: bool = True,
@@ -66,6 +69,7 @@ def __init__(
             connect_timeout=connect_timeout,
             read_timeout=read_timeout,
             num_pools=num_pools,
+            maxsize=maxsize,
             retries=retries,
             base=base,
             pagination=pagination,
@@ -78,6 +82,7 @@ def __init__(
             connect_timeout=connect_timeout,
             read_timeout=read_timeout,
             num_pools=num_pools,
+            maxsize=maxsize,
             retries=retries,
             base=base,
             pagination=pagination,
diff --git a/massive/rest/base.py b/massive/rest/base.py
index 3349d7ef..05c5694a 100644
--- a/massive/rest/base.py
+++ b/massive/rest/base.py
@@ -34,6 +34,7 @@ def __init__(
         verbose: bool,
         trace: bool,
         custom_json: Optional[Any] = None,
+        maxsize: int = 10,
     ):
         if api_key is None:
             raise AuthError(
@@ -75,6 +76,11 @@ def __init__(
         # https://urllib3.readthedocs.io/en/stable/reference/urllib3.connectionpool.html#urllib3.HTTPConnectionPool
         self.client = urllib3.PoolManager(
             num_pools=num_pools,
+            # Connections kept per HOST. urllib3 defaults this to 1, which serialises
+            # any threaded client onto a single connection and renegotiates TLS for
+            # every request that cannot get it. num_pools does not help: it caps the
+            # number of distinct host pools, and this client talks to one host.
+            maxsize=maxsize,
             headers=self.headers,  # default headers sent with each request.
             ca_certs=certifi.where(),
             cert_reqs="CERT_REQUIRED",
diff --git a/test_rest/test_connection_pool.py b/test_rest/test_connection_pool.py
new file mode 100644
index 00000000..f8f81196
--- /dev/null
+++ b/test_rest/test_connection_pool.py
@@ -0,0 +1,22 @@
+import unittest
+
+from massive import RESTClient
+
+
+class ConnectionPoolTest(unittest.TestCase):
+    def test_maxsize_defaults_to_ten(self):
+        client = RESTClient(api_key="test")
+
+        assert client.client.connection_pool_kw["maxsize"] == 10
+
+    def test_maxsize_is_configurable(self):
+        client = RESTClient(api_key="test", maxsize=64)
+
+        assert client.client.connection_pool_kw["maxsize"] == 64
+
+    def test_maxsize_is_independent_of_num_pools(self):
+        # num_pools caps the number of distinct HOST pools, which does not help a client
+        # that talks to one host from many threads -- that is what maxsize controls.
+        client = RESTClient(api_key="test", num_pools=50, maxsize=32)
+
+        assert client.client.connection_pool_kw["maxsize"] == 32