Skip to content

fix: force aiohttp's ThreadedResolver to stop the c-ares channel leak#12985

Merged
fregataa merged 2 commits into
mainfrom
fix/force-threaded-dns-resolver
Jul 21, 2026
Merged

fix: force aiohttp's ThreadedResolver to stop the c-ares channel leak#12985
fregataa merged 2 commits into
mainfrom
fix/force-threaded-dns-resolver

Conversation

@Yaminyam

Copy link
Copy Markdown
Member

Summary

Backportable half of the c-ares leak fix (the dependency removal itself lands separately on main in #12979 — split per review discussion so release branches don't need lockfile surgery).

Since aiodns 3.2, aiohttp picks the aiodns/pycares AsyncResolver as its default resolver whenever the package is importable. Every ephemeral ClientSession/TCPConnector then creates a c-ares channel that is never destroyed. On long-running services this accumulates:

  • native heap memory — ~0.5 MB/h on an idle production agent (eBPF memleak: ares_malloc_zero outstanding allocations strictly linear, with zero on-the-wire DNS)
  • one /dev/urandom fd per channel — three 16.5-day-old agent workers each held ~85,600 open urandom fds (~216 channels/hour, uniform across idle/busy nodes), converging on fd exhaustion

Same root-cause family as aio-libs/aiodns#191 (aiodns ≥ 3.3 additionally exhausts inotify watches, so upgrading aiodns worsens it).

Why a code change at all

Our own code barely touches aiodns — aiohttp picks it up implicitly just because it is installed. aiohttp has no configuration knob for the resolver choice, and injecting resolver= at every connector creation site is easy to miss. The only lockfile-free way to stop the selection is overriding the module-level defaults once at startup.

Changes

  • ai.backend.common.networking.force_threaded_dns_resolver() — overrides aiohttp.resolver.DefaultResolver and aiohttp.connector.DefaultResolver with ThreadedResolver, with a docstring explaining why
  • Called first thing in each service entrypoint (manager, agent, storage-proxy, webserver, account-manager, app-proxy coordinator/worker) — before any connector exists; the override survives the aiotools worker fork
  • Regression tests: the module defaults are overridden and a freshly created TCPConnector actually receives a ThreadedResolver

Verification

  • Production A/B: on one agent, this exact override eliminated both symptoms — zero c-ares allocations over a 30-minute eBPF window and a single urandom fd (vs. 85k on control nodes)
  • pants test green for the new tests; pants fmt/lint/check green across all touched files
  • No requirements/lockfile changes → cherry-picks cleanly onto release branches

🤖 Generated with Claude Code

Since aiodns 3.2, aiohttp selects the aiodns/pycares AsyncResolver as its
default resolver whenever the package is importable. Every ephemeral
ClientSession/TCPConnector then creates a c-ares channel that is never
destroyed, which on long-running services accumulates native heap memory
(~0.5 MB/h observed on an idle production agent via eBPF memleak) and one
leaked /dev/urandom fd per channel (~85,600 open fds on 16.5-day-old agent
workers, converging on fd exhaustion). Same root-cause family as
aio-libs/aiodns#191.

aiohttp offers no configuration knob for the resolver choice and passing
resolver= at every connector creation site is easy to miss, so add
force_threaded_dns_resolver() to ai.backend.common.networking and call it
once at each service entrypoint before any connector is created (the
override survives the aiotools worker fork). An A/B test on a production
agent confirmed this eliminates both symptoms completely.

This intentionally leaves the aiodns/pycares packages installed so the
change is backportable without touching release-branch lockfiles; the
dependency removal itself lands separately on main (#12979).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Yaminyam
Yaminyam requested a review from a team as a code owner July 21, 2026 05:17
Copilot AI review requested due to automatic review settings July 21, 2026 05:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added size:M 30~100 LoC comp:manager Related to Manager component comp:agent Related to Agent component comp:common Related to Common component comp:webserver Related to Web Server component comp:storage-proxy Related to Storage proxy component comp:app-proxy Related to App Proxy component labels Jul 21, 2026
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@fregataa
fregataa merged commit b166f88 into main Jul 21, 2026
33 checks passed
@fregataa
fregataa deleted the fix/force-threaded-dns-resolver branch July 21, 2026 23:32
fregataa pushed a commit that referenced this pull request Jul 22, 2026
…#12985) [26.4 backport] (#13018)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:agent Related to Agent component comp:app-proxy Related to App Proxy component comp:common Related to Common component comp:manager Related to Manager component comp:storage-proxy Related to Storage proxy component comp:webserver Related to Web Server component size:M 30~100 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants