fix(telegram): bound fallback-pool connection budget by pool count - #82860
Open
chelsealong wants to merge 1 commit into
Open
fix(telegram): bound fallback-pool connection budget by pool count#82860chelsealong wants to merge 1 commit into
chelsealong wants to merge 1 commit into
Conversation
TelegramFallbackTransport forwards whatever `limits` it is given to its primary pool and to every lazily-built fallback pool unchanged. The adapter was passing the same per-pool `httpx.Limits(max_connections=connection_pool_size)` used for a single client straight through, so the effective connection ceiling scaled with 2 PTB clients * (1 + fallback IP count) instead of staying at what the operator configured — with the default 512 pool size and even one fallback IP, up to 2048 connections. The number of fallback IPs was also unbounded, since DoH discovery returns every unique A record. Cap the fallback-IP list and derate the per-pool limit by the number of pools that will actually be constructed, so raising the fallback-IP count no longer multiplies the total connection budget. Fixes NousResearch#82678
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.
What does this PR do?
Bounds the Telegram fallback-IP connection-pool budget so it no longer scales
multiplicatively with the number of fallback IPs.
Related Issue
Fixes #82678
Type of Change
Root cause
TelegramAdapter.connect()builds twoHTTPXRequestclients (general +get_updates). On the fallback-IP path, each wraps aTelegramFallbackTransportthat owns one primary
AsyncHTTPTransportplus one lazily-built transport perfallback IP. The adapter passed the same
httpx.Limits(max_connections=connection_pool_size)(default 512,
HERMES_TELEGRAM_HTTP_POOL_SIZE) straight intoTelegramFallbackTransport, which forwards that exact object to its primarypool and to every fallback pool unchanged — that "caller limits win" behavior
is intentional and already covered by
test_caller_limits_win_over_pool_default/test_forwards_limits_to_inner_transports.The result: the effective ceiling was
2 clients × (1 primary + N fallback) × 512, i.e. 1024 connections with zero fallback IPs and 2048 with one — farbeyond a typical 256 soft
RLIMIT_NOFILE, before SQLite, logs, pipes, andother sockets are counted. The fallback-IP list itself was also unbounded,
since DNS-over-HTTPS discovery returns every unique A record it sees.
#45507 lowers the same default from 512 to 64 but (as noted on the issue)
still doesn't bound the address count or the per-pool multiplication — with
one fallback IP it's still
2 × (1+1) × 64 = 256before other descriptors arecounted, growing further with more addresses.
Changes Made
plugins/platforms/telegram/telegram_network.py: addedcap_fallback_ips()(bounds the fallback-IP list to
DEFAULT_MAX_FALLBACK_IPS = 3, logging whentruncated) and
safe_fallback_pool_limits()(derates ahttpx.Limitsby thetotal pool count —
num_ptb_clients * (1 + fallback_ip_count)— so thetotal connection budget across every pool stays close to what was
configured, rather than multiplying by pool count).
plugins/platforms/telegram/adapter.py: applycap_fallback_ips()to theresolved fallback-IP list in
connect(), and pass the derated limits (viasafe_fallback_pool_limits()) intoTelegramFallbackTransportinstead ofthe raw per-client limits object.
TelegramFallbackTransportitself isunchanged — its "caller limits win" contract stays intact.
tests/gateway/test_telegram_network.py: addedTestCapFallbackIpsandTestSafeFallbackPoolLimitscovering the cap and the derating math(including the zero-fallback-IP and near-zero-budget edge cases).
How to Test
bash scripts/run_tests.sh tests/gateway/test_telegram_network.py— newtests fail with
AttributeError: module ... has no attribute 'cap_fallback_ips'/'safe_fallback_pool_limits'against the pre-fixsource (confirmed via
git stashon just the two source files with thetest file kept), and pass after the fix.
bash scripts/run_tests.sh tests/gateway/test_telegram_network.py tests/gateway/test_telegram_fallback_pool_release_71593.py tests/gateway/test_telegram_closewait_limits_31599.py tests/gateway/test_telegram_connect.py tests/gateway/test_telegram_network_reconnect.py— all pass:bash scripts/run_tests.sh tests/gateway/(611 files):The 2 failures are both in
tests/gateway/test_wecom_callback.py(
test_build_event_extracts_text_message,test_poll_loop_dispatches_handle_message), unrelated to this change —they fail with the same
AttributeError: 'NoneType' object has no attribute 'fromstring'inplugins/platforms/wecom/callback_adapter.py:400on
HEAD~1(before this commit) as well, confirmed by checking out thepre-fix source and rerunning that file in isolation.
ruff check plugins/platforms/telegram/adapter.py plugins/platforms/telegram/telegram_network.py tests/gateway/test_telegram_network.py—All checks passed!Checklist
Code
Root cause section re: fix: stop Telegram gateway leaking sockets/FDs on reconnect storms #45507; fix(gateway): stop macOS gateway EMFILE — raise launchd fd limit + fix Telegram pool leak #39336 proposes raising the OS FD limit
instead and is orthogonal)
pre-existing, unrelated
test_wecom_callback.pyfailures reproduceidentically on
HEAD~1, see step 3 above)new tests and existing pool/transport tests are unit-level with fake
transports, no network
Documentation & Housekeeping
Note on AI assistance
Drafted with AI assistance (an autonomous coding agent), with the diff,
tests, and RED→GREEN verification reviewed before pushing.