fix: set explicit max_pool_connections on boto3 S3 client (closes #6)#7
Merged
Conversation
boto3's default max_pool_connections=10 is too low for concurrent Thumbor image loads (30 thumbnails per listing page times active visitors saturates immediately). The resulting urllib3 pool-full warnings cause connection churn — every overflow request does a fresh TCP+TLS handshake — which on aaf-6 prod correlates with intermittent Thumbor 400s. Pass a botocore.Config with max_pool_connections=50 by default, overridable via PGTHUMBOR_S3_MAX_POOL_CONNECTIONS. 50 covers asyncio.to_thread's default executor (min(32, cpu+4)) plus headroom. Closes #6. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
botocore.Configwithmax_pool_connections=50(env-var overridable) when constructing the boto3 S3 client.Why this change
boto3's default
max_pool_connectionsis10. Under typical Thumbor load (30 thumbnails per listing page × active visitors), the urllib3 pool saturates immediately and overflow requests get connection-discard-then-fresh-handshake churn. On aaf-6 prod this produced the observedurllib3.connectionpool: Connection pool is full, discarding connectionwarnings and correlates with intermittent Thumbor 400s from the downstream handshake failures.50 is a reasonable default: covers
asyncio.to_thread's default executor (min(32, cpu+4)— i.e. 8–32 threads depending on host) plus headroom for reopen churn. Env-var override (PGTHUMBOR_S3_MAX_POOL_CONNECTIONS) keeps it tunable per deployment.Behaviour matrix
max_pool_connections=50PGTHUMBOR_S3_MAX_POOL_CONNECTIONS=128max_pool_connections=128Scope limitations (out of scope, candidates for a follow-up)
Configdoes not overrideconnect_timeout/read_timeout; boto3 defaults of 60s/60s remain. A hanging S3 connection still blocks a worker for up to 60s. If the pool fix alone doesn't eliminate the residual 400s, tightening to ~5s/30s via the sameConfigis the next lever._get_s3_clientcaches by(bucket, region, endpoint). The env var is read once per client construction (first call wins); not worth including in the cache key since it won't vary per process.Test plan
test_default_max_pool_connections: mocksboto3.client, assertskwargs["config"].max_pool_connections == 50with the env var unset.test_env_override_max_pool_connections: env var=128→ assertsmax_pool_connections == 128.urllib3.connectionpool: Connection pool is fullwarnings should disappear; Thumbor 400 rate should drop.One small factual note
Issue #6 mentions
asyncio.to_thread's default executor has "40 threads". Python 3.8+ actually defaults tomin(32, os.cpu_count()+4)(so 8–32 in practice). Doesn't affect the recommendation — 50 still covers it with room — but the comment in the source now uses the accurate figure.🤖 Generated with Claude Code