This is an automated email from the ASF dual-hosted git repository.
janhoy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr-orbit.git
The following commit(s) were added to refs/heads/main by this push:
new f966dabd Pass the configured timeout to the data client (#117)
f966dabd is described below
commit f966dabd7c77c368971d62e0151473efab542084
Author: Jan Høydahl <[email protected]>
AuthorDate: Fri Oct 16 15:41:13 2026 +0200
Pass the configured timeout to the data client (#117)
---
solrorbit/client.py | 42 ++++++++++++++++++++++++++++++++++++++--
tests/unit/solr/test_client.py | 44 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 84 insertions(+), 2 deletions(-)
diff --git a/solrorbit/client.py b/solrorbit/client.py
index 3724cfe4..7f24d244 100644
--- a/solrorbit/client.py
+++ b/solrorbit/client.py
@@ -42,6 +42,14 @@ logger = logging.getLogger(__name__)
# Exceptions
# ---------------------------------------------------------------------------
+#: Socket read timeout used when ``--client-options`` does not set one.
+#:
+#: Fine for searches and bulk updates. Deliberately *not* relied on for
+#: long single operations such as ``optimize``, which a caller is expected to
+#: raise with ``--client-options=timeout:N``.
+DEFAULT_CLIENT_TIMEOUT = 30
+
+
class SolrClientError(Exception):
"""Base exception for all SolrAdminClient errors."""
@@ -71,7 +79,7 @@ class SolrAdminClient:
def __init__(self, host: str, port: int = 8983,
username: str = None, password: str = None,
- tls: bool = False, timeout: int = 30):
+ tls: bool = False, timeout: int = DEFAULT_CLIENT_TIMEOUT):
scheme = "https" if tls else "http"
self.base_url = f"{scheme}://{host}:{port}"
self.api_url = f"{self.base_url}/api"
@@ -476,7 +484,7 @@ class SolrClient(RequestContextHolder): # pylint:
disable=too-many-public-metho
pass
def __init__(self, host="localhost", port=8983, username=None,
password=None,
- tls=False, timeout=30):
+ tls=False, timeout=DEFAULT_CLIENT_TIMEOUT):
self._host = host
self._port = port
self._username = username
@@ -614,7 +622,37 @@ class ClientFactory:
username=self._client_options.get("basic_auth_user"),
password=self._client_options.get("basic_auth_password"),
tls=self._client_options.get("use_ssl", False),
+ timeout=self._read_timeout(),
)
+ def _read_timeout(self) -> int:
+ """Socket read timeout for every request this client makes, in seconds.
+
+ ``--client-options=timeout:N`` used to be dropped here, so the client
+ always ran on the 30-second constructor default no matter what was
+ asked for. That is survivable for searches and bulk updates and fatal
+ for ``optimize``: a force-merge of a 165M-document index is a single
+ HTTP request that runs for many minutes, and it failed at 30 seconds
+ having already done the work -- after, in one observed case, 86
+ minutes of successful indexing.
+
+ Invalid input falls back to the default rather than failing the run:
+ the alternative is aborting a benchmark over a typo in a tuning knob.
+ """
+ raw = self._client_options.get("timeout", DEFAULT_CLIENT_TIMEOUT)
+ try:
+ timeout = int(raw)
+ except (TypeError, ValueError):
+ self.logger.warning(
+ "Ignoring client option timeout=%r, which is not a number;
using %ds",
+ raw, DEFAULT_CLIENT_TIMEOUT)
+ return DEFAULT_CLIENT_TIMEOUT
+ if timeout <= 0:
+ self.logger.warning(
+ "Ignoring client option timeout=%d, which is not positive;
using %ds",
+ timeout, DEFAULT_CLIENT_TIMEOUT)
+ return DEFAULT_CLIENT_TIMEOUT
+ return timeout
+
def create_async(self):
return self.create()
diff --git a/tests/unit/solr/test_client.py b/tests/unit/solr/test_client.py
index bf2e2216..ed512e82 100644
--- a/tests/unit/solr/test_client.py
+++ b/tests/unit/solr/test_client.py
@@ -294,3 +294,47 @@ class TestSolrAdminClientIsCloudMode(unittest.TestCase):
if __name__ == "__main__":
unittest.main()
+
+
+class TestClientFactoryTimeout(unittest.TestCase):
+ """--client-options=timeout:N has to reach the client that uses it.
+
+ It used to be dropped in ClientFactory.create(), so every data operation
+ ran on the 30-second constructor default. Survivable for searches and
+ bulk updates; fatal for optimize, which is one HTTP request that
+ force-merges the whole index. A full nyc_taxis run indexed 165,346,692
+ documents over 86 minutes and then failed there at 30 seconds, having
+ already done the work.
+ """
+
+ @staticmethod
+ def _factory(**options):
+ from solrorbit.client import ClientFactory
+ return ClientFactory([{"host": "localhost", "port": 8983}], options)
+
+ def test_the_configured_timeout_reaches_the_client(self):
+ assert self._factory(timeout=7200).create()._timeout == 7200
+
+ def test_it_also_reaches_the_pysolr_data_client(self):
+ # optimize goes through pysolr, not the admin client, which is why
+ # dropping it here was invisible until a force-merge took minutes.
+ client = self._factory(timeout=1234).create()
+ assert client._get_pysolr("c").timeout == 1234
+
+ def test_and_the_admin_client(self):
+ assert self._factory(timeout=1234).create()._admin.timeout == 1234
+
+ def test_a_string_is_accepted(self):
+ # client_options arrive parsed from a comma-separated CLI string.
+ assert self._factory(timeout="900").create()._timeout == 900
+
+ def test_the_default_applies_when_unset(self):
+ from solrorbit.client import DEFAULT_CLIENT_TIMEOUT
+ assert self._factory().create()._timeout == DEFAULT_CLIENT_TIMEOUT
+
+ def test_nonsense_falls_back_rather_than_failing_the_run(self):
+ # Aborting a benchmark over a typo in a tuning knob is worse than
+ # running it on the default.
+ from solrorbit.client import DEFAULT_CLIENT_TIMEOUT
+ assert self._factory(timeout="soon").create()._timeout ==
DEFAULT_CLIENT_TIMEOUT
+ assert self._factory(timeout=-5).create()._timeout ==
DEFAULT_CLIENT_TIMEOUT
