Skip to content

fix: enforce max_workers in LLMMetadataExtractor.run_async#11248

Merged
bogdankostic merged 3 commits into
deepset-ai:mainfrom
etairl:fix/llm-metadata-extractor-async-semaphore
May 12, 2026
Merged

fix: enforce max_workers in LLMMetadataExtractor.run_async#11248
bogdankostic merged 3 commits into
deepset-ai:mainfrom
etairl:fix/llm-metadata-extractor-async-semaphore

Conversation

@etairl
Copy link
Copy Markdown
Contributor

@etairl etairl commented May 4, 2026

Summary

  • LLMMetadataExtractor.run_async acquires its asyncio.Semaphore once around the outer gather(...) instead of inside each task, so max_workers has no effect and every prompt in a batch fires its LLM call simultaneously.
  • The docstring on __init__ advertises max_workers as "the maximum number of requests that should be allowed to run concurrently when using the run_async method", so the current behavior silently breaks that contract and can blow up LLM-provider rate limits / connection pools on large batches.
  • Fix is to move the async with sem: into a per-task wrapper coroutine so the limit is actually enforced. Added a regression test that verifies peak in-flight calls stay <= max_workers.

Before

sem = Semaphore(max(1, self.max_workers))
async with sem:
    results = await gather(*[self._run_async(prompt) for prompt in all_prompts])

After

sem = Semaphore(max(1, self.max_workers))

async def _bounded_run(prompt: ChatMessage | None) -> dict[str, Any]:
    async with sem:
        return await self._run_async(prompt)

results = await gather(*[_bounded_run(prompt) for prompt in all_prompts])

Test plan

  • hatch run test:unit -k test_llm_metadata_extractor passes (includes new test_run_async_respects_max_workers).
  • CI green.

The asyncio.Semaphore intended to bound concurrent LLM calls was acquired
once around the outer gather(...) call instead of inside each task, so
max_workers had no effect in run_async and all batched LLM requests fired
simultaneously. Move the semaphore acquisition into a per-task wrapper so
the documented concurrency cap is honored.
@etairl etairl requested a review from a team as a code owner May 4, 2026 17:13
@etairl etairl requested review from bogdankostic and removed request for a team May 4, 2026 17:13
@vercel
Copy link
Copy Markdown

vercel Bot commented May 4, 2026

@etairl is attempting to deploy a commit to the deepset Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added topic:tests type:documentation Improvements on the docs labels May 4, 2026
Copy link
Copy Markdown
Contributor

@bogdankostic bogdankostic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @etairl, looks already good in general! Please just make sure to use double backticks in the release note for in-line code.

@etairl
Copy link
Copy Markdown
Contributor Author

etairl commented May 12, 2026

Thanks for reviewing. Fixed.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 12, 2026

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  haystack/components/extractors
  llm_metadata_extractor.py
Project Total  

This report was generated by python-coverage-comment-action

Copy link
Copy Markdown
Contributor

@bogdankostic bogdankostic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @etairl! :)

@bogdankostic bogdankostic merged commit 50b2141 into deepset-ai:main May 12, 2026
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:tests type:documentation Improvements on the docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants