chore: drop the now-unused aiodns/pycares dependencies (main only)#12979
Open
Yaminyam wants to merge 3 commits into
Open
chore: drop the now-unused aiodns/pycares dependencies (main only)#12979Yaminyam wants to merge 3 commits into
Yaminyam wants to merge 3 commits into
Conversation
…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>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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. |
Member
Author
|
The |
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.
Summary
Removes
aiodns/pycaresfrom 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
getaddrinfosupport), aiohttp selects the aiodns/pycares AsyncResolver as its default resolver whenever the package is importable. Every ephemeralClientSession/TCPConnectorthen instantiates a pycares channel (a c-ares channel underneath) that is never destroyed:memleakon an idle production agent showedares_malloc_zerooutstanding 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_infoconfirmed ~1.1 GB live, not-fragmentation, on a 10-day-old worker)./dev/urandomper channel for query-id randomization. Three agent workers with 16.5 days of uptime each held ~85,600 open/dev/urandomfds (~216 channels/hour, uniform across idle and busy nodes), steadily converging on fd exhaustion.A/B verification: forcing aiohttp's
ThreadedResolveron 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
aiodns==3.2/pycares~=4.11.0fromrequirements.txt(and the explicit//:reqs#pycaresBUILD dependency). aiohttp falls back to itsThreadedResolver(getaddrinfo in a thread pool) — our outbound HTTP targets a small set of fixed hosts, so async DNS resolution buys us nothing.aiodnsusage — the boot-time self-IP lookup incommon/identity.py— with stdlibloop.getaddrinfo().aiodns/pycaresentries and their references are removed; every other pin is byte-identical (a fullgenerate-lockfilesrun would have floated 37 unrelated packages).Verification
pants test tests/unit/common/test_identity.pygreen (both the resolve-success and fallback paths, now mocked vialoop.getaddrinfo)pants fmt/lint/checkgreen; noaiodns/pycaresreferences remain anywhere in sources, BUILD files, requirements, or the lock🤖 Generated with Claude Code