Skip to content

Commit 5317646

Browse files
authored
refactor: move blocking provider I/O off event loop (#5835)
# What does this PR do? This PR moves blocking operations to worker threads, parallelizes independent async calls, and adds reusable OpenAI client caching keyed by `(api_key, base_url)` with shutdown cleanup. It updates zstd request decompression, localfs and S3 file operations, sqlite-vec hybrid search execution, and image localization paths in both the OpenAI mixin and VertexAI adapter to reduce event-loop blocking and improve request throughput. ## Test Plan - `uv run pytest -q tests/unit/providers/utils/inference/test_openai_mixin_models.py tests/unit/providers/utils/inference/test_openai_mixin_provider_data.py tests/unit/providers/inference/vertexai/test_adapter_chat.py tests/unit/files/test_files.py -x --tb=short` - Result: `118 passed in 1.29s` Signed-off-by: Sébastien Han <seb@redhat.com>
1 parent 791a048 commit 5317646

6 files changed

Lines changed: 111 additions & 85 deletions

File tree

src/ogx/core/server/server.py

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -357,18 +357,27 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> Any:
357357

358358
try:
359359
max_decompressed_size = 100 * 1024 * 1024 # 100 MB
360-
decompressor = zstandard.ZstdDecompressor()
361-
# Use streaming decompression to handle frames without content size
362-
reader = decompressor.stream_reader(compressed_body)
363-
decompressed_body = reader.read(max_decompressed_size)
364-
if reader.read(1):
365-
reader.close()
360+
361+
def _decompress_zstd(compressed: bytes, max_size: int) -> tuple[bytes | None, bool]:
362+
decompressor = zstandard.ZstdDecompressor()
363+
reader = decompressor.stream_reader(compressed)
364+
try:
365+
data = reader.read(max_size)
366+
is_oversized = bool(reader.read(1))
367+
return (None, True) if is_oversized else (data, False)
368+
finally:
369+
reader.close()
370+
371+
decompressed_body, oversized = await asyncio.to_thread(
372+
_decompress_zstd, compressed_body, max_decompressed_size
373+
)
374+
375+
if oversized:
366376
return await _send_error_response(
367377
send,
368378
status=413,
369379
message=f"Decompressed request body exceeds maximum allowed size of {max_decompressed_size} bytes",
370380
)
371-
reader.close()
372381

373382
# Strip content-encoding header and update content-length
374383
new_headers = [

src/ogx/providers/inline/files/localfs/files.py

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
download as a belt-and-suspenders measure.
2222
"""
2323

24+
import asyncio
2425
import re
2526
import time
2627
import uuid
@@ -165,8 +166,11 @@ async def openai_upload_file(
165166
content = await file.read()
166167
file_size = len(content)
167168

168-
with open(file_path, "wb") as f:
169-
f.write(content)
169+
def _write_file() -> None:
170+
with open(file_path, "wb") as f:
171+
f.write(content)
172+
173+
await asyncio.to_thread(_write_file)
170174

171175
created_at = int(time.time())
172176
expires_at = created_at + self.config.ttl_secs
@@ -255,8 +259,12 @@ async def openai_delete_file(self, request: DeleteFileRequest) -> OpenAIFileDele
255259
file_id = request.file_id
256260
# Delete physical file
257261
_, file_path = await self._lookup_file_id(file_id, action=Action.DELETE)
258-
if file_path.exists():
259-
file_path.unlink()
262+
263+
def _delete_if_exists() -> None:
264+
if file_path.exists():
265+
file_path.unlink()
266+
267+
await asyncio.to_thread(_delete_if_exists)
260268

261269
# Delete metadata from database
262270
assert self.sql_store is not None, "Files provider not initialized"
@@ -278,9 +286,11 @@ async def openai_retrieve_file_content(self, request: RetrieveFileContentRequest
278286
await self.openai_delete_file(DeleteFileRequest(file_id=file_id))
279287
raise OpenAIFileObjectNotFoundError(file_id)
280288

289+
file_content = await asyncio.to_thread(file_path.read_bytes)
290+
281291
# Return as binary response with appropriate content type
282292
return Response(
283-
content=file_path.read_bytes(),
293+
content=file_content,
284294
media_type="application/octet-stream",
285295
headers={
286296
"Content-Disposition": f'attachment; filename="{sanitize_content_disposition_filename(file_obj.filename)}"'

src/ogx/providers/inline/vector_io/sqlite_vec/sqlite_vec.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -460,8 +460,10 @@ async def query_hybrid(
460460
reranker_params = {}
461461

462462
# Get results from both search methods, passing filters to each
463-
vector_response = await self.query_vector(embedding, k, score_threshold, filters)
464-
keyword_response = await self.query_keyword(query_string, k, score_threshold, filters)
463+
vector_response, keyword_response = await asyncio.gather(
464+
self.query_vector(embedding, k, score_threshold, filters),
465+
self.query_keyword(query_string, k, score_threshold, filters),
466+
)
465467

466468
# Convert responses to score dictionaries using chunk_id (EmbeddedChunk inherits from Chunk)
467469
vector_scores = {

src/ogx/providers/remote/files/s3/files.py

Lines changed: 46 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
# This source code is licensed under the terms described in the LICENSE file in
55
# the root directory of this source tree.
66

7+
import asyncio
78
import uuid
89
from datetime import UTC, datetime
910
from typing import TYPE_CHECKING, Any, cast
@@ -73,33 +74,36 @@ def _create_s3_client(config: S3FilesImplConfig) -> "S3Client":
7374

7475

7576
async def _create_bucket_if_not_exists(client: "S3Client", config: S3FilesImplConfig) -> None:
76-
try:
77-
client.head_bucket(Bucket=config.bucket_name)
78-
except ClientError as e:
79-
error_code = e.response["Error"]["Code"]
80-
if error_code == "404":
81-
if not config.auto_create_bucket:
82-
raise RuntimeError(
83-
f"S3 bucket '{config.bucket_name}' does not exist. "
84-
f"Either create the bucket manually or set 'auto_create_bucket: true' in your configuration."
85-
) from e
86-
try:
87-
# For us-east-1, we can't specify LocationConstraint
88-
if config.region == "us-east-1":
89-
client.create_bucket(Bucket=config.bucket_name)
90-
else:
91-
client.create_bucket(
92-
Bucket=config.bucket_name,
93-
CreateBucketConfiguration=cast(Any, {"LocationConstraint": config.region}),
94-
)
95-
except ClientError as create_error:
96-
raise RuntimeError(
97-
f"Failed to create S3 bucket '{config.bucket_name}': {create_error}"
98-
) from create_error
99-
elif error_code == "403":
100-
raise RuntimeError(f"Access denied to S3 bucket '{config.bucket_name}'") from e
101-
else:
102-
raise RuntimeError(f"Failed to access S3 bucket '{config.bucket_name}': {e}") from e
77+
def _check_and_create() -> None:
78+
try:
79+
client.head_bucket(Bucket=config.bucket_name)
80+
except ClientError as e:
81+
error_code = e.response["Error"]["Code"]
82+
if error_code == "404":
83+
if not config.auto_create_bucket:
84+
raise RuntimeError(
85+
f"S3 bucket '{config.bucket_name}' does not exist. "
86+
f"Either create the bucket manually or set 'auto_create_bucket: true' in your configuration."
87+
) from e
88+
try:
89+
# For us-east-1, we can't specify LocationConstraint
90+
if config.region == "us-east-1":
91+
client.create_bucket(Bucket=config.bucket_name)
92+
else:
93+
client.create_bucket(
94+
Bucket=config.bucket_name,
95+
CreateBucketConfiguration=cast(Any, {"LocationConstraint": config.region}),
96+
)
97+
except ClientError as create_error:
98+
raise RuntimeError(
99+
f"Failed to create S3 bucket '{config.bucket_name}': {create_error}"
100+
) from create_error
101+
elif error_code == "403":
102+
raise RuntimeError(f"Access denied to S3 bucket '{config.bucket_name}'") from e
103+
else:
104+
raise RuntimeError(f"Failed to access S3 bucket '{config.bucket_name}': {e}") from e
105+
106+
await asyncio.to_thread(_check_and_create)
103107

104108

105109
def _make_file_object(
@@ -164,7 +168,8 @@ async def _get_file(
164168
async def _delete_file(self, file_id: str) -> None:
165169
"""Delete a file from S3 and the database."""
166170
try:
167-
self.client.delete_object(
171+
await asyncio.to_thread(
172+
self.client.delete_object,
168173
Bucket=self._config.bucket_name,
169174
Key=file_id,
170175
)
@@ -251,7 +256,8 @@ async def openai_upload_file(
251256
await self.sql_store.insert("openai_files", entry)
252257

253258
try:
254-
self.client.put_object(
259+
await asyncio.to_thread(
260+
self.client.put_object,
255261
Bucket=self._config.bucket_name,
256262
Key=file_id,
257263
Body=content,
@@ -319,12 +325,17 @@ async def openai_retrieve_file_content(self, request: RetrieveFileContentRequest
319325
row = await self._get_file(file_id)
320326

321327
try:
322-
response = self.client.get_object(
323-
Bucket=self._config.bucket_name,
324-
Key=row["id"],
325-
)
326-
# TODO: can we stream this instead of loading it into memory
327-
content = response["Body"].read()
328+
329+
def _download_from_s3() -> bytes:
330+
response = self.client.get_object(
331+
Bucket=self._config.bucket_name,
332+
Key=row["id"],
333+
)
334+
# TODO: can we stream this instead of loading it into memory
335+
body: bytes = response["Body"].read()
336+
return body
337+
338+
content = await asyncio.to_thread(_download_from_s3)
328339
except ClientError as e:
329340
if e.response["Error"]["Code"] == "NoSuchKey":
330341
await self._delete_file(file_id)

src/ogx/providers/remote/inference/vertexai/vertexai.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
from __future__ import annotations
88

9+
import asyncio
910
import base64
1011
import struct
1112
import time
@@ -742,7 +743,7 @@ async def openai_chat_completion(
742743
self._warn_unsupported_chat_params(params)
743744
tools, tool_choice = self._resolve_deprecated_tools(params)
744745

745-
messages = [await self._localize_image_url(message) for message in params.messages]
746+
messages = list(await asyncio.gather(*[self._localize_image_url(message) for message in params.messages]))
746747
system_instruction, contents = converters.convert_openai_messages_to_gemini(messages)
747748
tools_input = converters.convert_openai_tools_to_gemini(tools)
748749
config = self._build_generation_config(

src/ogx/providers/utils/inference/openai_mixin.py

Lines changed: 28 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
# This source code is licensed under the terms described in the LICENSE file in
55
# the root directory of this source tree.
66

7+
import asyncio
78
import base64
89
import ssl
910
import uuid
@@ -14,7 +15,7 @@
1415
import httpx
1516
from openai import AsyncOpenAI, DefaultAsyncHttpxClient
1617
from openai.types.chat import ChatCompletionChunk
17-
from pydantic import BaseModel, ConfigDict, Field
18+
from pydantic import BaseModel, ConfigDict, Field, PrivateAttr
1819

1920
from ogx.core.request_headers import NeedsRequestProviderData
2021
from ogx.log import get_logger
@@ -106,9 +107,7 @@ class OpenAIMixin(NeedsRequestProviderData, ABC, BaseModel):
106107
# Format: {"model_id": {"embedding_dimension": 1536, "context_length": 8192}}
107108
embedding_model_metadata: dict[str, dict[str, int]] = {}
108109

109-
# Cache of available models keyed by model ID
110-
# This is set in list_models() and used in check_model_availability()
111-
_model_cache: dict[str, Model] = {}
110+
_model_cache: dict[str, Model] = PrivateAttr(default_factory=dict)
112111

113112
# Optional field name in provider data to look for API key, which takes precedence
114113
provider_data_api_key_field: str | None = None
@@ -118,6 +117,9 @@ class OpenAIMixin(NeedsRequestProviderData, ABC, BaseModel):
118117
# Trade-off: SSL context changes require server restart
119118
shared_ssl_context: ssl.SSLContext | bool = Field(default_factory=ssl.create_default_context, exclude=True)
120119

120+
_cached_client: AsyncOpenAI | None = PrivateAttr(default=None)
121+
_cached_client_key: tuple[str, str] | None = PrivateAttr(default=None)
122+
121123
def get_api_key(self) -> str | None:
122124
"""
123125
Get the API key.
@@ -199,10 +201,7 @@ async def list_provider_model_ids(self) -> Iterable[str]:
199201
200202
:return: An iterable of model IDs or None if not implemented
201203
"""
202-
client = self.client
203-
async with client:
204-
model_ids = [m.id async for m in client.models.list()]
205-
return model_ids
204+
return [m.id async for m in self.client.models.list()]
206205

207206
async def initialize(self) -> None:
208207
"""
@@ -215,26 +214,20 @@ async def initialize(self) -> None:
215214
pass
216215

217216
async def shutdown(self) -> None:
218-
"""
219-
Shutdown the OpenAI mixin.
220-
221-
This method provides a default implementation that does nothing.
222-
Subclasses can override this method to perform cleanup tasks
223-
such as closing connections, releasing resources, etc.
224-
"""
225-
pass
217+
"""Shutdown the OpenAI mixin, closing the cached HTTP client."""
218+
if self._cached_client is not None:
219+
await self._cached_client.close()
220+
self._cached_client = None
221+
self._cached_client_key = None
226222

227223
@property
228224
def client(self) -> AsyncOpenAI:
229225
"""
230226
Get an AsyncOpenAI client instance.
231227
232-
Uses the abstract methods get_api_key() and get_base_url() which must be
233-
implemented by child classes.
234-
235-
Network configuration from config.network is automatically applied.
236-
Users can also provide the API key via the provider data header, which
237-
is used instead of any config API key.
228+
Caches the client keyed by (api_key, base_url) for connection reuse.
229+
When the key changes (e.g. per-request provider_data), a new client
230+
is created and cached in its place.
238231
"""
239232

240233
api_key = self._get_api_key_from_config_or_provider_data()
@@ -244,19 +237,15 @@ def client(self) -> AsyncOpenAI:
244237
message += f' Please provide a valid API key in the provider data header, e.g. x-ogx-provider-data: {{"{self.provider_data_api_key_field}": "<API_KEY>"}}.'
245238
raise ValueError(message)
246239

240+
base_url = self.get_base_url()
241+
cache_key = (api_key, base_url)
242+
243+
if self._cached_client is not None and self._cached_client_key == cache_key:
244+
return self._cached_client
245+
247246
extra_params = self.get_extra_client_params()
248247
network_kwargs = build_network_client_kwargs(self.config.network)
249248

250-
# Handle http_client creation/merging:
251-
# - If get_extra_client_params() provides an http_client (e.g., OCI with custom auth),
252-
# merge network config into it. The merge behavior:
253-
# * Preserves auth from get_extra_client_params() (provider-specific auth like OCI signer)
254-
# * Preserves headers from get_extra_client_params() as base
255-
# * Applies network config (TLS, proxy, timeout, headers) on top
256-
# * Network config headers take precedence over provider headers (allows override)
257-
# - Otherwise, if network config exists, create http_client from it
258-
# - Otherwise, use a cached SSL context for performance
259-
# This allows providers with custom auth to still use standard network settings
260249
if "http_client" in extra_params:
261250
if network_kwargs:
262251
extra_params["http_client"] = _merge_network_config_into_client(
@@ -267,12 +256,16 @@ def client(self) -> AsyncOpenAI:
267256
else:
268257
extra_params["http_client"] = DefaultAsyncHttpxClient(verify=self.shared_ssl_context)
269258

270-
return AsyncOpenAI(
259+
client = AsyncOpenAI(
271260
api_key=api_key,
272-
base_url=self.get_base_url(),
261+
base_url=base_url,
273262
**extra_params,
274263
)
275264

265+
self._cached_client = client
266+
self._cached_client_key = cache_key
267+
return client
268+
276269
def _get_api_key_from_config_or_provider_data(self) -> str | None:
277270
api_key = self.get_api_key()
278271

@@ -430,7 +423,7 @@ async def _localize_image_url(m: OpenAIMessageParam) -> OpenAIMessageParam:
430423
# else it's a string and we don't need to modify it
431424
return m
432425

433-
messages = [await _localize_image_url(m) for m in messages]
426+
messages = list(await asyncio.gather(*[_localize_image_url(m) for m in messages]))
434427

435428
request_params = await prepare_openai_completion_params(
436429
model=provider_model_id,

0 commit comments

Comments
 (0)