Skip to content

Commit 3054b8f

Browse files
feat: supported attach hint (#833)
1 parent bc2150e commit 3054b8f

6 files changed

Lines changed: 243 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
* Query session attach stream handles `NodeShutdown` and `SessionShutdown` session hints: on `NodeShutdown` the session's node connection is pessimized and the session is retired, on `SessionShutdown` the session is retired without touching the node
12
* Support the timezone-carrying types `TzDate`, `TzDatetime` and `TzTimestamp` in query results and parameters (reading them previously raised `AttributeError`)
23
* Drop support for Python 3.8 and 3.9; the minimum supported version is now Python 3.10
34
* Accept native `datetime.datetime` values for `Datetime` and `Datetime64` query parameters (previously only integer seconds since the epoch were accepted)
Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
import asyncio
2+
from unittest import mock
3+
4+
import pytest
5+
6+
from ydb._grpc.common.protos import ydb_query_pb2
7+
from ydb._grpc.grpcwrapper import common_utils
8+
from ydb.aio.pool import ConnectionPool as AsyncConnectionPool
9+
from ydb.pool import ConnectionPool, ConnectionsCache
10+
from ydb.query.session import QuerySession
11+
12+
13+
def _make_session(node_id=42):
14+
driver = mock.Mock()
15+
driver._pessimize_node = mock.Mock()
16+
session = QuerySession(driver)
17+
session._session_id = "test-session"
18+
session._node_id = node_id
19+
return session, driver
20+
21+
22+
class TestQuerySessionAttachHints:
23+
def test_node_shutdown_pessimizes_node_and_invalidates_session(self):
24+
session, driver = _make_session(node_id=42)
25+
26+
session._handle_attach_session_state(
27+
ydb_query_pb2.SessionState(
28+
status=0,
29+
node_shutdown=ydb_query_pb2.NodeShutdownHint(),
30+
)
31+
)
32+
33+
driver._pessimize_node.assert_called_once_with(42)
34+
assert session._invalidated
35+
assert session._closed
36+
37+
def test_session_shutdown_invalidates_without_pessimizing_node(self):
38+
session, driver = _make_session(node_id=42)
39+
40+
session._handle_attach_session_state(
41+
ydb_query_pb2.SessionState(
42+
status=0,
43+
session_shutdown=ydb_query_pb2.SessionShutdownHint(),
44+
)
45+
)
46+
47+
driver._pessimize_node.assert_not_called()
48+
assert session._invalidated
49+
assert session._closed
50+
51+
def test_node_shutdown_with_zero_node_id_delegates_to_driver(self):
52+
session, driver = _make_session(node_id=0)
53+
54+
session._handle_attach_session_state(
55+
ydb_query_pb2.SessionState(
56+
status=0,
57+
node_shutdown=ydb_query_pb2.NodeShutdownHint(),
58+
)
59+
)
60+
61+
driver._pessimize_node.assert_called_once_with(0)
62+
assert session._invalidated
63+
64+
def test_node_shutdown_without_node_id_skips_pessimization(self):
65+
session, driver = _make_session(node_id=None)
66+
67+
session._handle_attach_session_state(
68+
ydb_query_pb2.SessionState(
69+
status=0,
70+
node_shutdown=ydb_query_pb2.NodeShutdownHint(),
71+
)
72+
)
73+
74+
driver._pessimize_node.assert_not_called()
75+
assert session._invalidated
76+
assert session._closed
77+
78+
def test_no_hint_does_not_invalidate(self):
79+
session, driver = _make_session()
80+
81+
session._handle_attach_session_state(
82+
ydb_query_pb2.SessionState(status=0),
83+
)
84+
85+
driver._pessimize_node.assert_not_called()
86+
assert not session._invalidated
87+
assert not session._closed
88+
89+
def test_none_response_does_not_invalidate(self):
90+
session, driver = _make_session()
91+
92+
session._handle_attach_session_state(None)
93+
94+
driver._pessimize_node.assert_not_called()
95+
assert not session._invalidated
96+
assert not session._closed
97+
98+
def test_attach_stream_wrapper_handles_hint_and_returns_status(self):
99+
session, driver = _make_session(node_id=42)
100+
response = ydb_query_pb2.SessionState(
101+
status=0,
102+
session_shutdown=ydb_query_pb2.SessionShutdownHint(),
103+
)
104+
105+
status = session._attach_stream_wrapper(response)
106+
107+
driver._pessimize_node.assert_not_called()
108+
assert session._invalidated
109+
assert isinstance(status, common_utils.ServerStatus)
110+
111+
def test_attach_stream_wrapper_returns_status_without_hint(self):
112+
session, driver = _make_session()
113+
response = ydb_query_pb2.SessionState(status=0)
114+
115+
status = session._attach_stream_wrapper(response)
116+
117+
driver._pessimize_node.assert_not_called()
118+
assert not session._invalidated
119+
assert isinstance(status, common_utils.ServerStatus)
120+
121+
122+
class TestConnectionPoolAttachHintPessimization:
123+
def test_connections_cache_get_connection_by_node_id(self):
124+
cache = ConnectionsCache()
125+
connection = mock.Mock()
126+
connection.endpoint = "grpc://localhost:2135"
127+
connection.node_id = 42
128+
connection.add_cleanup_callback = mock.Mock()
129+
130+
cache.add(connection)
131+
132+
assert cache.get_connection_by_node_id(42) is connection
133+
assert cache.get_connection_by_node_id(99) is None
134+
135+
def test_sync_pool_pessimizes_node_connection(self):
136+
pool = ConnectionPool.__new__(ConnectionPool)
137+
connection = mock.Mock()
138+
pool._store = mock.Mock()
139+
pool._store.get_connection_by_node_id.return_value = connection
140+
pool._on_disconnected = mock.Mock()
141+
142+
pool._pessimize_node(42)
143+
144+
pool._store.get_connection_by_node_id.assert_called_once_with(42)
145+
pool._on_disconnected.assert_called_once_with(connection)
146+
147+
def test_sync_pool_ignores_missing_node_connection(self):
148+
pool = ConnectionPool.__new__(ConnectionPool)
149+
pool._store = mock.Mock()
150+
pool._store.get_connection_by_node_id.return_value = None
151+
pool._on_disconnected = mock.Mock()
152+
153+
pool._pessimize_node(42)
154+
pool._pessimize_node(0)
155+
pool._pessimize_node(-1)
156+
157+
pool._store.get_connection_by_node_id.assert_called_once_with(42)
158+
pool._on_disconnected.assert_not_called()
159+
160+
@pytest.mark.asyncio
161+
async def test_async_pool_ignores_non_positive_node_id(self):
162+
pool = AsyncConnectionPool.__new__(AsyncConnectionPool)
163+
pool._store = mock.Mock()
164+
pool._on_disconnected = mock.Mock()
165+
166+
pool._pessimize_node(0)
167+
pool._pessimize_node(-1)
168+
169+
pool._store.get_connection_by_node_id.assert_not_called()
170+
pool._on_disconnected.assert_not_called()
171+
172+
@pytest.mark.asyncio
173+
async def test_async_pool_ignores_missing_node_connection(self):
174+
pool = AsyncConnectionPool.__new__(AsyncConnectionPool)
175+
pool._store = mock.Mock()
176+
pool._store.get_connection_by_node_id.return_value = None
177+
pool._on_disconnected = mock.Mock()
178+
179+
pool._pessimize_node(42)
180+
await asyncio.sleep(0)
181+
182+
pool._store.get_connection_by_node_id.assert_called_once_with(42)
183+
pool._on_disconnected.assert_not_called()
184+
185+
@pytest.mark.asyncio
186+
async def test_async_pool_pessimizes_node_connection(self):
187+
pool = AsyncConnectionPool.__new__(AsyncConnectionPool)
188+
connection = mock.Mock()
189+
disconnect = mock.AsyncMock()
190+
pool._store = mock.Mock()
191+
pool._store.get_connection_by_node_id.return_value = connection
192+
pool._on_disconnected = mock.Mock(return_value=disconnect)
193+
194+
pool._pessimize_node(42)
195+
await asyncio.sleep(0)
196+
197+
pool._store.get_connection_by_node_id.assert_called_once_with(42)
198+
pool._on_disconnected.assert_called_once_with(connection)
199+
disconnect.assert_awaited_once_with()

ydb/aio/pool.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import asyncio
44
import logging
55
import random
6-
from typing import Any, Callable, Optional, Tuple, TYPE_CHECKING
6+
from typing import Any, Callable, Optional, Tuple, TYPE_CHECKING, cast
77

88
from ydb import issues
99
from ydb.opentelemetry.tracing import SpanName, create_ydb_span
@@ -311,6 +311,15 @@ async def __wrapper__() -> None:
311311

312312
return __wrapper__
313313

314+
def _pessimize_node(self, node_id: int) -> None:
315+
"""Deprioritize the connection attached to the given YDB node."""
316+
if node_id <= 0:
317+
return
318+
319+
connection = cast(Optional[Connection], self._store.get_connection_by_node_id(node_id))
320+
if connection is not None:
321+
asyncio.get_running_loop().create_task(self._on_disconnected(connection)())
322+
314323
async def wait(self, timeout: Optional[float] = 7.0, fail_fast: bool = False) -> None: # type: ignore[override] # async override of sync method
315324
with create_ydb_span(SpanName.DRIVER_INITIALIZE, self._driver_config, kind="internal").attach_context():
316325
await self._store.get(fast_fail=fail_fast, wait_timeout=timeout if timeout is not None else 7.0)

ydb/aio/query/session.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
from .. import _utilities
1515
from ... import issues
1616
from ...settings import BaseRequestSettings
17-
from ..._grpc.grpcwrapper import common_utils
1817
from ..._grpc.grpcwrapper import ydb_query_public_types as _ydb_query_public
1918

2019
from ...query import base
@@ -53,7 +52,7 @@ async def _attach(self) -> None:
5352
self._stream = await self._attach_call()
5453
self._status_stream = _utilities.AsyncResponseIterator(
5554
self._stream,
56-
lambda response: common_utils.ServerStatus.from_proto(response),
55+
self._attach_stream_wrapper,
5756
)
5857

5958
try:

ydb/pool.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,10 @@ def remove(self, connection: Connection) -> None:
160160
self.connections.pop(connection.endpoint, None)
161161
self.outdated.pop(connection.endpoint, None)
162162

163+
def get_connection_by_node_id(self, node_id: Optional[int]) -> Optional[Connection]:
164+
with self.lock:
165+
return self.connections_by_node_id.get(node_id)
166+
163167

164168
class Discovery(threading.Thread):
165169
def __init__(self, store: ConnectionsCache, driver_config: "DriverConfig") -> None:
@@ -494,6 +498,15 @@ def _on_disconnected(self, connection: Connection) -> None:
494498
if self._discovery_thread:
495499
self._discovery_thread.notify_disconnected()
496500

501+
def _pessimize_node(self, node_id: int) -> None:
502+
"""Deprioritize the connection attached to the given YDB node."""
503+
if node_id <= 0:
504+
return
505+
506+
connection = self._store.get_connection_by_node_id(node_id)
507+
if connection is not None:
508+
self._on_disconnected(connection)
509+
497510
def discovery_debug_details(self) -> str:
498511
"""
499512
Returns debug string about last errors

ydb/query/session.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,24 @@ def _check_session_ready_to_use(self) -> None:
156156
if self._closed:
157157
raise RuntimeError(f"Session is not active, session_id: {self._session_id}, closed: {self._closed}")
158158

159+
def _attach_stream_wrapper(self, response_pb):
160+
"""Map attach-stream protobuf frames to ServerStatus and handle session hints."""
161+
self._handle_attach_session_state(response_pb)
162+
return common_utils.ServerStatus.from_proto(response_pb)
163+
164+
def _handle_attach_session_state(self, response_pb) -> None:
165+
"""Retire the session when the server sends a shutdown hint on the attach stream."""
166+
if response_pb is None:
167+
return
168+
169+
hint = response_pb.WhichOneof("session_hint")
170+
if hint == "node_shutdown":
171+
if self._node_id is not None:
172+
self._driver._pessimize_node(self._node_id)
173+
self._close_session(invalidate=True)
174+
elif hint == "session_shutdown":
175+
self._close_session(invalidate=True)
176+
159177
def _close_session(self, invalidate: bool = False) -> None:
160178
if self._closed:
161179
return
@@ -362,7 +380,7 @@ def _attach(self, first_resp_timeout: int = DEFAULT_INITIAL_RESPONSE_TIMEOUT) ->
362380
self._stream = self._attach_call()
363381
status_stream = _utilities.SyncResponseIterator(
364382
self._stream,
365-
lambda response: common_utils.ServerStatus.from_proto(response),
383+
self._attach_stream_wrapper,
366384
)
367385

368386
try:

0 commit comments

Comments
 (0)