Skip to content

Commit da9d5b5

Browse files
committed
fix(arrow): drain connection before release to avoid poisoning the pool
1 parent 8b0152a commit da9d5b5

2 files changed

Lines changed: 33 additions & 6 deletions

File tree

‎hotdata/arrow.py‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,22 @@ def __init__(self, status: str, result_id: str) -> None:
5050
)
5151

5252

53+
def _release_conn(response: Any) -> None:
54+
"""Return a urllib3 connection to the pool without poisoning it.
55+
56+
pyarrow stops reading at the Arrow end-of-stream marker, which can leave
57+
bytes unconsumed at the HTTP layer (e.g. the terminating zero-length chunk
58+
of a chunked response). ``release_conn`` on a partially-read connection
59+
hands a tainted socket back to the pool — the next request to reuse it
60+
fails with ``http.client.ResponseNotReady`` ("Request-sent"). Draining any
61+
unread bytes first makes the connection safe to reuse.
62+
"""
63+
try:
64+
response.drain_conn()
65+
finally:
66+
response.release_conn()
67+
68+
5369
def _import_pyarrow() -> Any:
5470
try:
5571
import pyarrow.ipc as ipc # type: ignore[import-untyped]
@@ -95,7 +111,7 @@ def get_result_arrow(
95111
try:
96112
return ipc.open_stream(response).read_all()
97113
finally:
98-
response.release_conn()
114+
_release_conn(response)
99115

100116
@contextmanager
101117
def stream_result_arrow(
@@ -108,8 +124,10 @@ def stream_result_arrow(
108124
) -> Iterator["pa.RecordBatchStreamReader"]:
109125
"""Yield a :class:`pyarrow.RecordBatchStreamReader` for a ready result.
110126
111-
The HTTP connection is released when the context exits. Iterate the
112-
reader to consume :class:`pyarrow.RecordBatch` messages, or call
127+
The HTTP connection is drained and released when the context exits, so
128+
exiting early (before the reader is exhausted) reads and discards the
129+
remaining body to keep the connection reusable. Iterate the reader to
130+
consume :class:`pyarrow.RecordBatch` messages, or call
113131
``reader.read_all()`` for a full :class:`pyarrow.Table`.
114132
115133
Example::
@@ -127,7 +145,7 @@ def stream_result_arrow(
127145
try:
128146
yield ipc.open_stream(response)
129147
finally:
130-
response.release_conn()
148+
_release_conn(response)
131149

132150
def _call_arrow(
133151
self,

‎tests/test_arrow.py‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ class _FakeUrllib3Response(io.RawIOBase):
4545
4646
pyarrow.ipc.open_stream wants a real file-like object (it checks
4747
``closed`` and ``readable()``); the SDK's RESTResponse needs ``status``,
48-
``reason``, ``data``, and ``headers``. release_conn is recorded.
48+
``reason``, ``data``, and ``headers``. drain_conn / release_conn are
49+
recorded so tests can assert the connection is drained before reuse.
4950
"""
5051

5152
def __init__(self, status: int, body: bytes, headers: Dict[str, str]):
@@ -56,6 +57,7 @@ def __init__(self, status: int, body: bytes, headers: Dict[str, str]):
5657
self._buf = io.BytesIO(body)
5758
self.headers = headers
5859
self.release_conn_called = False
60+
self.drain_conn_called = False
5961

6062
@property
6163
def data(self) -> bytes:
@@ -71,6 +73,9 @@ def read(self, amt: Optional[int] = -1) -> bytes:
7173
def readable(self) -> bool:
7274
return True
7375

76+
def drain_conn(self) -> None:
77+
self.drain_conn_called = True
78+
7479
def release_conn(self) -> None:
7580
self.release_conn_called = True
7681

@@ -146,6 +151,9 @@ def test_get_result_arrow_returns_table(monkeypatch: pytest.MonkeyPatch) -> None
146151
got = results.get_result_arrow("res_123")
147152

148153
assert got.equals(table)
154+
# Connection is drained (not just released) so a partially-read body can't
155+
# poison the pool and break the next request with ResponseNotReady.
156+
assert fake.drain_conn_called
149157
assert fake.release_conn_called
150158

151159
# Single request was made.
@@ -197,7 +205,8 @@ def test_stream_result_arrow_yields_reader(
197205
roundtrip = pa.Table.from_batches(batches, schema=reader.schema)
198206
assert roundtrip.equals(table)
199207

200-
# Connection is released after the context exits.
208+
# Connection is drained and released after the context exits.
209+
assert fake.drain_conn_called
201210
assert fake.release_conn_called
202211

203212

0 commit comments

Comments
 (0)