fix: improve async safety in routers and Redis KV reads - #5800
Merged
franciscojavierarceo merged 8 commits intoMay 12, 2026
Merged
Conversation
The get() method called await client.ttl(key) but discarded the result, adding an unnecessary round-trip to Redis on every read operation. Signed-off-by: Sébastien Han <seb@redhat.com>
…orage Replace bare asyncio.create_task() calls with the existing create_detached_background_task() utility which properly detaches request-scoped context (OTel traces, auth identity) and adds error logging via done callbacks. Signed-off-by: Sébastien Han <seb@redhat.com>
… checks Create a shallow copy of impls_by_provider_id before iterating to prevent RuntimeError if the routing table refreshes concurrently. Signed-off-by: Sébastien Han <seb@redhat.com>
Replace bare asyncio.create_task() calls with task + done callback so that errors in background store_chat_completion() are logged instead of silently swallowed. Keeps request context (OTel trace, auth identity) intact for observability. Signed-off-by: Sébastien Han <seb@redhat.com>
leseb
requested review from
bbrowning,
cdoern,
franciscojavierarceo,
mattf and
raghotham
as code owners
May 12, 2026 13:41
…ed-text-to-determine-the-task
Contributor
|
This pull request has merge conflicts that must be resolved before it can be merged. @leseb please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork |
…ed-text-to-determine-the-task Signed-off-by: Sébastien Han <seb@redhat.com>
…ttps://github.com/leseb/ogx into Looking-at-the-attached-text-to-determine-the-task
cdoern
approved these changes
May 12, 2026
franciscojavierarceo
approved these changes
May 12, 2026
Member
Author
|
@Mergifyio backport release-1.0.x |
Contributor
✅ Backports have been createdDetails
|
eoinfennessy
pushed a commit
to opendatahub-io/ogx
that referenced
this pull request
May 14, 2026
…-ai#5800) (ogx-ai#5823) # What does this PR do? This PR hardens async behavior in router/storage paths and removes an unnecessary Redis read round-trip. It adds done-callback error logging for fire-and-forget chat completion persistence tasks in the inference router. It snapshots provider implementation maps before health-check iteration to avoid `RuntimeError` from concurrent dict mutation. It also removes an unused `ttl` call from Redis KV `get()`. ## Test Plan `uv run pytest tests/unit/core/routers/test_inference_router.py tests/unit/core/routers/test_vector_io.py -q` Result: `12 passed in 0.47s`. `uv run pytest tests/unit/utils/kvstore -q` Result: `60 passed, 3 xfailed in 0.44s`. <hr>This is an automatic backport of pull request ogx-ai#5800 done by [Mergify](https://mergify.com). Signed-off-by: Sébastien Han <seb@redhat.com> Co-authored-by: Sébastien Han <seb@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR hardens async behavior in router/storage paths and removes an unnecessary Redis read round-trip. It adds done-callback error logging for fire-and-forget chat completion persistence tasks in the inference router. It snapshots provider implementation maps before health-check iteration to avoid
RuntimeErrorfrom concurrent dict mutation. It also removes an unusedttlcall from Redis KVget().Test Plan
uv run pytest tests/unit/core/routers/test_inference_router.py tests/unit/core/routers/test_vector_io.py -qResult:
12 passed in 0.47s.uv run pytest tests/unit/utils/kvstore -qResult:
60 passed, 3 xfailed in 0.44s.