fix(redis): cache GCP IAM token to prevent async event loop blocking - #26317
fix(redis): cache GCP IAM token to prevent async event loop blocking#26317MaximeBOUDIER wants to merge 1 commit into
Conversation
## Problem GCPIAMCredentialProvider.get_credentials() calls _generate_gcp_iam_access_token on every Redis connection establishment. This function performs synchronous HTTP and gRPC calls (google-auth + google-cloud-iam) which block Python's asyncio event loop while running. Under concurrent load (e.g. connection pool warm-up, parallel health checks), multiple connections are established simultaneously, each triggering an independent blocking IAM token refresh. These refreshes serialise behind each other inside the single-threaded event loop, causing individual Redis spans to take 20-25 seconds instead of milliseconds. Observed in production via Datadog APM: a single INCRBYFLOAT Redis span took 25.6 seconds (90% of a 28.4s trace), with GCP metadata + GenerateAccessToken gRPC calls visible inside the span. This cascaded into aiohttp SocketTimeoutError on upstream LLM API calls — not because the upstream was slow, but because the event loop was frozen and the 30-second sock_read timer fired on a connection that was never given CPU time. ## Fix Add a module-level token cache (dict keyed by service account, value is (token, expiry_monotonic)). _get_cached_gcp_iam_token() returns the cached token on cache hit (no I/O), and refreshes only when expired using double-checked locking so only one thread performs the network round-trip. GCP IAM tokens are valid for 1 hour; the cache TTL is set to 55 minutes (_GCP_IAM_TOKEN_TTL_SECONDS = 3300) to refresh safely before expiry. The cache is shared across all GCPIAMCredentialProvider instances for the same service account, so N concurrent Redis connections on the same pod share a single token and avoid N concurrent blocking refreshes. get_credentials_async() already used asyncio.to_thread (non-blocking), and is updated to call _get_cached_gcp_iam_token so it also benefits from caching. ## Tests - Updated existing test that expected a fresh token on every call to reflect the new caching behaviour. - Added tests for: cache hit (no redundant I/O), cache expiry and refresh, and cache sharing across multiple provider instances. - Added autouse fixture to clear the module-level cache between tests. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
|
Low: No security issues foundThis PR adds a thread-safe module-level cache for GCP IAM tokens used in Redis authentication. The caching uses proper double-checked locking, the cache key is a server-side configuration value (not user input), and the TTL provides adequate margin before token expiry. No new attack surface introduced. Status: 0 open Posted by Veria AI · 2026-04-23T09:04:00.513Z |
Greptile SummaryThis PR fixes event-loop blocking in Confidence Score: 5/5Safe to merge — the caching logic is correct and well-tested; only minor style issues remain. All findings are P2 (unused import, stale docstring). The double-checked locking pattern is correct, thread-safe under CPython's GIL, and tests adequately cover the new caching contract including expiry and cross-instance sharing. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/_redis_credential_provider.py | Adds module-level token cache with double-checked locking for GCP IAM tokens; logic is correct with two minor style issues (unused Optional import, stale docstring). |
| tests/test_litellm/test_redis.py | Updates and adds tests for the new caching behavior (cache hit, expiry, cross-instance sharing) with an autouse fixture to isolate module-level state; looks good. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[get_credentials / get_credentials_async] --> B{Cache hit?\n_token_cache.get}
B -- Yes, not expired --> C[Return cached token\nzero I/O]
B -- No / expired --> D[Acquire _token_cache_lock]
D --> E{Double-check:\nstill expired?}
E -- No, another thread refreshed --> F[Return fresh token\nfrom cache]
E -- Yes --> G[_generate_gcp_iam_access_token\nHTTP + gRPC to GCP]
G --> H[Store token + expiry\nmonotonic + 3300s]
H --> I[Release lock\nReturn token]
Reviews (1): Last reviewed commit: "fix(redis): cache GCP IAM token to preve..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| from typing import Tuple | ||
| import threading | ||
| import time | ||
| from typing import Dict, Optional, Tuple |
There was a problem hiding this comment.
Optional import
Optional is imported on line 4 but never referenced anywhere in the file.
| from typing import Dict, Optional, Tuple | |
| from typing import Dict, Tuple |
| """ | ||
| redis.credentials.CredentialProvider implementation that generates a fresh GCP IAM | ||
| token on every new connection. This fixes the 1-hour token expiry issue for async | ||
| Redis cluster clients, which previously generated the token once at startup and | ||
| cached it as a static password. | ||
|
|
There was a problem hiding this comment.
The first two sentences still say "generates a fresh GCP IAM token on every new connection," which directly contradicts the caching logic introduced by this PR. The class-level paragraph added below partially corrects this but leaves the opening claim misleading for future readers.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| from typing import Tuple | ||
| import threading | ||
| import time | ||
| from typing import Dict, Optional, Tuple |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
3 participants
Description
GCPIAMCredentialProvider.get_credentials()calls_generate_gcp_iam_access_tokenon every Redis connection establishment with no caching. This function performs synchronous HTTP (GCP metadata server) and gRPC (IAMCredentials/GenerateAccessToken) calls which block Python's asyncio event loop.Under concurrent load — connection pool warm-up, parallel health checks, rate limit tracking — multiple connections are established simultaneously, each triggering an independent blocking IAM token refresh. These refreshes serialise inside the single-threaded event loop, causing individual Redis spans to take 20-25 seconds instead of milliseconds.
Observed in production via Datadog APM: a single
INCRBYFLOATRedis span took 25.6s (90% of a 28.4s trace), with GCP metadata +GenerateAccessTokengRPC calls visible inside the span. This cascaded intoaiohttp.SocketTimeoutErroron upstream LLM API calls — not because the upstream was slow, but because the event loop was frozen and the 30ssock_readtimer fired on a connection that was never given CPU time.Note: PR #24426 correctly introduced
get_credentials_async()withasyncio.to_thread, butget_credentials()(called by redis-py on connection establishment) still has no caching and blocks on every call.Fix
Add a module-level token cache (dict keyed by service account →
(token, expiry_monotonic))._get_cached_gcp_iam_token()returns the cached token on cache hit (zero I/O), and refreshes only when expired using double-checked locking so only one thread performs the network round-trip at expiry time.GCPIAMCredentialProviderinstances for the same service account — N concurrent Redis connections on the same pod share one token and avoid N concurrent blocking refreshesget_credentials_async()is updated to call_get_cached_gcp_iam_tokenviaasyncio.to_threadso it also benefits from cachingTests
autousefixture to isolate the module-level cache between testsChecklist
tests/test_litellm/make test-unitpasses (1 pre-existing unrelated failure intest_sync_client_preserves_password_for_cluster_when_url_also_set— present onmainbefore this PR)This pull request was created with AI assistance.