Skip to content

Blocking embedder and LLM calls run on the event loop while holding a pooled connection #10

Description

@royalpinto007

Problem

POST /ask in app/main.py is an async def handler that makes two blocking calls on the event loop, and holds a pooled database connection across both.

async with pool.connection() as conn:
    ...
    vec = app.state.embedder.embed([req.question])[0]   # blocking
    hits = await search(conn, principal, req.question, vec, limit=req.limit)
    answer = generate(app.state.llm, req.question, hits)  # blocking network call
  • LocalEmbedder.embed in app/embeddings.py runs SentenceTransformer.encode, CPU-bound synchronous inference.
  • generate calls LLM.complete, and GroqLLM.complete in app/generate.py is a synchronous HTTP round trip to the Groq API.

Neither is awaited or offloaded, so both stall the entire ASGI event loop. With one worker the server handles exactly one /ask at a time, and every other request, including /health, waits behind the LLM. On top of that, the connection from the psycopg pool is checked out for the whole duration, so a slow LLM exhausts the pool as well as the loop.

GroqLLM.complete also constructs a fresh Groq(api_key=...) client on every call and passes no timeout, so a hung upstream hangs the request, the loop, and a pooled connection indefinitely.

Suggested approach

  1. Do the database work in short, well defined transactions, and release the connection before generation. The audit insert has to happen first (the comment in ask explains why, correctly), but nothing requires holding that connection through the LLM call.
  2. Offload the blocking calls. The smallest correct change is await anyio.to_thread.run_sync(...) around embedder.embed and generate. The better change is to make LLM an async protocol with an httpx-based Groq implementation, since the whole point of that call is IO.
  3. Build the Groq client once at startup, next to where _make_llm already runs in lifespan, and give it an explicit timeout plus a bounded retry.
  4. Add a test that two concurrent /ask calls against a deliberately slow FakeLLM overlap in time rather than serializing. That is the regression guard.

Done when

  • No blocking call runs directly on the event loop in a request handler.
  • A pooled connection is not held across an LLM call.
  • The LLM call has a timeout, and exceeding it produces a clean 504 with an audit row already written.
  • Concurrency is demonstrated by a test, not asserted in a comment.

If you want to take this on, comment on the issue to claim it and it will be assigned. Please keep to a maximum of 2 open claims per person at a time so other contributors get a chance.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requesthelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions