Skip to content

Commit 2213169

Browse files
authored
feat(kernel): forward _pool_maxsize (#940)
* feat(kernel): forward _pool_maxsize Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> * fix(kernel): gate max connections kwarg Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> * chore(kernel): bump pinned revision Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> * refactor(kernel): pass max connections directly Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> * fix(kernel): gate max connections by wheel support Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> * docs(kernel): clarify max connections version Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> * fix(kernel): normalize zero pool maxsize Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> --------- Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
1 parent a22ac63 commit 2213169

6 files changed

Lines changed: 69 additions & 24 deletions

File tree

CONNECTION_PARAMETERS.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ to change without notice.
102102
| ------------------------------------ | ----------- | :----: | :----: | ------------- | ---------------------------------------------------------------------------------------------------------------------------------------------- |
103103
| `_socket_timeout` | `float` (s) ||| `900` (Thrift); `120` (kernel) | Thrift: socket send/recv/connect timeout. Kernel: total HTTP request deadline from connect through response-body completion. A positive value is forwarded; unset or `0` selects the kernel's 120s default. On the kernel path, `0` is neither unlimited nor an immediate timeout. |
104104
| `_pool_connections` | `int` || ⚠️ | `10` | Number of urllib3 connection pools. Configures the connector's shared Python HTTP client; the kernel's query transport is its own Rust stack. |
105-
| `_pool_maxsize` | `int` || ⚠️ | `20` | Max connections per pool on the shared Python HTTP client. Same kernel caveat as `_pool_connections`. |
105+
| `_pool_maxsize` | `int` || ⚠️ | `20` (Thrift); `100` (kernel when unset) | Max idle connections retained per host. A positive value always configures the shared Python HTTP client and also configures the kernel's Rust HTTP pool with kernel ≥ 1.1.0. Unset or `0` keeps each client's default. |
106106
| `_proxy_auth_method` | `str` || ⚠️ | `None` | `basic` or `negotiate` (Kerberos). Applies to the shared Python HTTP client; not threaded to the kernel query transport. See [`docs/proxy.md`](docs/proxy.md). |
107107
| `_retry_stop_after_attempts_count` | `int` ||| `30` | Max attempts in a retry sequence. Bounded to `[1, 60]` on Thrift; forwarded to the kernel's retry policy. |
108108
| `_retry_stop_after_attempts_duration`| `float` (s) ||| `900` | Max total wall-clock seconds spent retrying. Forwarded to the kernel. |
@@ -202,9 +202,10 @@ None — the kernel's parameter surface is currently a subset of Thrift's.
202202

203203
### Behavioral divergences to watch
204204

205-
- **Connection pooling / proxy** (`_pool_connections`, `_pool_maxsize`,
206-
`_proxy_auth_method`) configure the connector's shared Python HTTP client
207-
(auth/telemetry); the kernel's query traffic uses its own Rust transport.
205+
- **Connection pooling / proxy**: `_pool_connections` and `_proxy_auth_method`
206+
configure only the shared Python HTTP client. `_pool_maxsize` also configures
207+
the kernel's Rust HTTP pool when positive and using kernel ≥ 1.1.0. A value of
208+
`0` is treated as unset: the shared client keeps 20 and the kernel keeps 100.
208209
- **`use_inline_params`** renders parameters inline on Thrift; the kernel uses
209210
native parameter binding.
210211

KERNEL_REV

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
d64009eb59404c1b082cb020296337f96dc0d4d7
1+
167651ecc67143ef258ad70fd2682ced00beaa99

src/databricks/sql/backend/kernel/client.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ def _kernel_session_accepts_kwarg(name: str) -> bool:
156156
``**kwargs`` catch-all), so forwarding a kwarg it doesn't declare raises
157157
``TypeError`` at construction, so we gate kwargs on what the installed
158158
wheel supports. Falls **closed** (returns ``False``) when the signature
159-
can't be introspected because omitting an accepted telemetry kwarg is safer
159+
can't be introspected because omitting an accepted optional kwarg is safer
160160
than forwarding an unsupported one.
161161
"""
162162
try:
@@ -260,6 +260,7 @@ def __init__(
260260
self._retry_options = kwargs.get("retry_options") or {}
261261
# The kernel binding owns type and range validation.
262262
self._request_timeout_secs = kwargs.get("request_timeout_secs")
263+
self._max_connections = kwargs.get("max_connections")
263264
# Kernel telemetry phase 7 adds binding/runtime identity and
264265
# telemetry config kwargs directly to ``databricks_sql_kernel.Session``.
265266
self._telemetry_options = kwargs.get("telemetry_options") or {}
@@ -377,6 +378,9 @@ def open_session(
377378
# kernel's ``retry_*`` kwargs. Empty when at defaults.
378379
retry_kwargs = _kernel_retry_kwargs(self._retry_options)
379380
telemetry_kwargs = _kernel_telemetry_kwargs(self._telemetry_options)
381+
max_connections_kwargs: Dict[str, Any] = {}
382+
if _kernel_session_accepts_kwarg("max_connections"):
383+
max_connections_kwargs["max_connections"] = self._max_connections
380384
# Forward caller / connector HTTP headers. The kernel applies
381385
# them on every request; a caller ``User-Agent`` is appended
382386
# to the kernel's base UA. Only pass the kwarg when there's
@@ -421,6 +425,7 @@ def open_session(
421425
**tls_kwargs,
422426
**retry_kwargs,
423427
**telemetry_kwargs,
428+
**max_connections_kwargs,
424429
**http_headers_kwargs,
425430
)
426431
except Exception as exc:

src/databricks/sql/session.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,7 @@ def _create_backend(
313313
auth_options=kernel_auth_options,
314314
retry_options=kernel_retry_options,
315315
request_timeout_secs=kwargs.get("_socket_timeout"),
316+
max_connections=kwargs.get("_pool_maxsize") or None,
316317
telemetry_options=kernel_telemetry_options,
317318
)
318319

tests/unit/test_kernel_client.py

Lines changed: 40 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,30 @@ def fake_session(**kw):
368368
assert captured["request_timeout_secs"] == timeout
369369

370370

371+
@pytest.mark.parametrize("max_connections", [None, 41])
372+
def test_open_session_passes_max_connections_to_kernel(monkeypatch, max_connections):
373+
captured = {}
374+
375+
def fake_session(**kw):
376+
captured.update(kw)
377+
sess = MagicMock()
378+
sess.session_id = "sess-id"
379+
return sess
380+
381+
monkeypatch.setattr(kernel_client._kernel, "Session", fake_session)
382+
c = kernel_client.KernelDatabricksClient(
383+
server_hostname="example.cloud.databricks.com",
384+
http_path="/sql/1.0/warehouses/abc",
385+
auth_provider=AccessTokenAuthProvider("dapi-test"),
386+
ssl_options=None,
387+
max_connections=max_connections,
388+
)
389+
390+
c.open_session(session_configuration=None, catalog=None, schema=None)
391+
392+
assert captured["max_connections"] == max_connections
393+
394+
371395
def test_open_session_passes_phase_7_telemetry_kwargs_to_kernel(monkeypatch):
372396
"""Kernel telemetry phase 7 added binding/runtime identity and
373397
telemetry config kwargs to ``databricks_sql_kernel.Session``."""
@@ -428,25 +452,20 @@ def fake_session(**kw):
428452
assert captured["telemetry_circuit_breaker_enabled"] is False
429453

430454

431-
def test_open_session_omits_phase_7_kwargs_kernel_does_not_accept(monkeypatch):
432-
"""Phase-7 identity/telemetry kwargs must NOT be forwarded to a kernel
433-
``Session`` whose (fixed, no-``**kwargs``) constructor doesn't declare
434-
them.
455+
def test_open_session_omits_optional_kwargs_kernel_does_not_accept(monkeypatch):
456+
"""Optional kwargs must NOT be forwarded to a kernel ``Session`` whose
457+
fixed constructor doesn't declare them.
435458
436459
The real ``databricks_sql_kernel.Session`` is a PyO3 class with a fixed
437-
signature; the pinned ``^0.2.0`` wheel predates phase 7 and accepts none
438-
of these kwargs, so forwarding them unconditionally raises ``TypeError``
439-
and breaks every ``use_kernel=True`` connection. The other tests here use
440-
a ``**kwargs`` MagicMock that silently swallows the kwargs and hides the
441-
break; this one uses a fixed-signature fake mirroring the real 0.2.0
442-
surface to prove the client gates on what the installed Session supports.
460+
signature. The other tests here use a ``**kwargs`` MagicMock that silently
461+
swallows unsupported kwargs; this fixed-signature fake proves the client
462+
filters optional kwargs.
443463
"""
444464
captured = {}
445465

446-
# Fixed signature mirroring the pinned 0.2.0 kernel Session: it accepts
447-
# the base connection/tls/retry kwargs but NONE of the phase-7 identity
448-
# or telemetry kwargs, and has no **kwargs catch-all.
449-
def fake_session_v0_2_0(
466+
# Accept baseline connection/tls/retry kwargs but no max-connections,
467+
# phase-7 identity, or telemetry kwargs, and no **kwargs catch-all.
468+
def fake_session_without_optional_kwargs(
450469
host,
451470
http_path,
452471
*,
@@ -480,7 +499,9 @@ def fake_session_v0_2_0(
480499
sess.session_id = "sess-id"
481500
return sess
482501

483-
monkeypatch.setattr(kernel_client._kernel, "Session", fake_session_v0_2_0)
502+
monkeypatch.setattr(
503+
kernel_client._kernel, "Session", fake_session_without_optional_kwargs
504+
)
484505
monkeypatch.setattr(
485506
kernel_client.TelemetryHelper,
486507
"get_driver_system_configuration",
@@ -504,17 +525,18 @@ def fake_session_v0_2_0(
504525
kwargs = kernel_client._kernel_telemetry_kwargs(
505526
{"enable_telemetry": True, "telemetry_batch_size": 17}
506527
)
507-
assert kwargs == {}, f"expected no phase-7 kwargs on 0.2.0 Session, got {kwargs}"
528+
assert kwargs == {}, f"expected no unsupported phase-7 kwargs, got {kwargs}"
508529

509530
c = kernel_client.KernelDatabricksClient(
510531
server_hostname="example.cloud.databricks.com",
511532
http_path="/sql/1.0/warehouses/abc",
512533
auth_provider=AccessTokenAuthProvider("dapi-test"),
513534
ssl_options=None,
535+
max_connections=41,
514536
telemetry_options={"enable_telemetry": True, "telemetry_batch_size": 17},
515537
)
516-
# Would raise TypeError: unexpected keyword argument if the client
517-
# forwarded phase-7 kwargs the fixed-signature Session doesn't declare.
538+
# Would raise TypeError if the client forwarded max-connections or
539+
# phase-7 kwargs the fixed-signature Session doesn't declare.
518540
c.open_session(session_configuration=None, catalog=None, schema=None)
519541
assert captured["host"] == "example.cloud.databricks.com"
520542

tests/unit/test_session.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,7 @@ def test_retry_and_socket_timeout_threaded_into_kernel_client(self):
466466
_retry_stop_after_attempts_count=10,
467467
_retry_stop_after_attempts_duration=600.0,
468468
_socket_timeout=12.5,
469+
_pool_maxsize=41,
469470
)
470471
try:
471472
_, kwargs = mock_kernel_client.call_args
@@ -475,6 +476,21 @@ def test_retry_and_socket_timeout_threaded_into_kernel_client(self):
475476
assert opts["retry_stop_after_attempts_count"] == 10
476477
assert opts["retry_stop_after_attempts_duration"] == 600.0
477478
assert kwargs["request_timeout_secs"] == 12.5
479+
assert kwargs["max_connections"] == 41
480+
finally:
481+
conn.close()
482+
483+
conn = databricks.sql.connect(
484+
server_hostname="foo",
485+
http_path="/sql/1.0/warehouses/abc",
486+
use_kernel=True,
487+
access_token="dapi-xyz",
488+
enable_telemetry=False,
489+
_pool_maxsize=0,
490+
)
491+
try:
492+
_, kwargs = mock_kernel_client.call_args
493+
assert kwargs["max_connections"] is None
478494
finally:
479495
conn.close()
480496

0 commit comments

Comments
 (0)