Skip to content

chore: drop the now-unused aiodns/pycares dependencies (main only)#12979

Open
Yaminyam wants to merge 3 commits into
mainfrom
fix/remove-aiodns-cares-leak
Open

chore: drop the now-unused aiodns/pycares dependencies (main only)#12979
Yaminyam wants to merge 3 commits into
mainfrom
fix/remove-aiodns-cares-leak

Conversation

@Yaminyam

Copy link
Copy Markdown
Member

Summary

Removes aiodns / pycares from the dependency set to stop a native memory + file-descriptor leak that affects every long-running component making aiohttp client requests (most visibly the agent).

Root cause

Since aiodns 3.2 (which added getaddrinfo support), aiohttp selects the aiodns/pycares AsyncResolver as its default resolver whenever the package is importable. Every ephemeral ClientSession/TCPConnector then instantiates a pycares channel (a c-ares channel underneath) that is never destroyed:

  • Native heap growth: eBPF memleak on an idle production agent showed ares_malloc_zero outstanding allocations growing strictly linearly (~0.5 MB/h) with zero on-the-wire DNS queries — the channels' internal state simply accumulates. The growth sits in [heap] (malloc_info confirmed ~1.1 GB live, not-fragmentation, on a 10-day-old worker).
  • fd leak: c-ares opens /dev/urandom per channel for query-id randomization. Three agent workers with 16.5 days of uptime each held ~85,600 open /dev/urandom fds (~216 channels/hour, uniform across idle and busy nodes), steadily converging on fd exhaustion.

A/B verification: forcing aiohttp's ThreadedResolver on one node eliminated both symptoms completely — zero c-ares allocations over a 30-minute eBPF window and a single urandom fd (vs. 85k), while a control node kept leaking.

This is the same root-cause family as aio-libs/aiodns#191 (many short-lived ClientSessions); on aiodns >= 3.3 it additionally manifests as inotify watch exhaustion (c-ares config monitoring), so upgrading aiodns makes it worse, not better.

Changes

  • Remove aiodns==3.2 / pycares~=4.11.0 from requirements.txt (and the explicit //:reqs#pycares BUILD dependency). aiohttp falls back to its ThreadedResolver (getaddrinfo in a thread pool) — our outbound HTTP targets a small set of fixed hosts, so async DNS resolution buys us nothing.
  • Replace the only direct aiodns usage — the boot-time self-IP lookup in common/identity.py — with stdlib loop.getaddrinfo().
  • Update the identity unit tests accordingly.
  • Surgical lockfile edit: only the aiodns/pycares entries and their references are removed; every other pin is byte-identical (a full generate-lockfiles run would have floated 37 unrelated packages).

Verification

  • pants test tests/unit/common/test_identity.py green (both the resolve-success and fallback paths, now mocked via loop.getaddrinfo)
  • pants fmt/lint/check green; no aiodns/pycares references remain anywhere in sources, BUILD files, requirements, or the lock
  • Production A/B evidence above (ThreadedResolver ≡ post-removal behavior)

🤖 Generated with Claude Code

…lients

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, which on long-running agents accumulates:

- native heap memory (~0.5 MB/h observed on an idle production agent via
  eBPF memleak: `ares_malloc_zero` outstanding allocations growing
  strictly linearly), and
- one leaked `/dev/urandom` fd per channel (c-ares opens it for query-id
  randomization): three 16.5-day-old agent workers each held ~85,600 such
  fds, converging on fd exhaustion.

An A/B test forcing aiohttp's ThreadedResolver eliminated both symptoms
completely (zero c-ares allocations over a 30-minute eBPF window, 1
urandom fd). This is the same root-cause family as aio-libs/aiodns#191,
which manifests as inotify-watch exhaustion on aiodns >= 3.3.

Removing aiodns makes aiohttp fall back to its ThreadedResolver
(getaddrinfo in a thread pool). Our outbound HTTP traffic targets a small
set of fixed hosts, so the async-DNS benefit is negligible. The only
direct aiodns usage was the boot-time self-IP lookup in
common/identity.py, now served by loop.getaddrinfo(). The lockfile edit
is surgical (only the two package entries removed); all other pins are
untouched.

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 04:35
Copilot AI review requested due to automatic review settings July 21, 2026 04:35

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:common Related to Common component labels Jul 21, 2026
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Yaminyam Yaminyam changed the title fix: drop aiodns/pycares to stop the c-ares channel leak in aiohttp clients chore: drop the now-unused aiodns/pycares dependencies (main only) Jul 21, 2026
@Yaminyam Yaminyam modified the milestones: 26.4, 26.5 Jul 21, 2026
@Yaminyam

Copy link
Copy Markdown
Member Author

Per review discussion, this PR is now the main-only cleanup half: the backportable leak fix (forcing aiohttp's ThreadedResolver at service startup, no lockfile changes) moved to #12985 with milestone 26.4. This PR removes the then-unused packages (surgical lockfile edit) and should land after #12985. Milestone moved to 26.5 — no backport.

@Yaminyam

Copy link
Copy Markdown
Member Author

The test-component failures here are unrelated to this PR: the same 6 runtime-variant CRUD failures reproduce deterministically on a clean checkout of current main locally (identical pydantic model_attributes_type error). Root-caused and filed as #12986 (regression from #12970 — JSON-null default_model_definition crashes the new adapter). Will re-run CI here once that lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:common Related to Common component size:M 30~100 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants