chore(caching): isolate semantic cache by tenant scope - #26992
Conversation
Both ``RedisSemanticCache`` and ``QdrantSemanticCache`` ignored the proxy-injected tenant metadata when storing or retrieving cache entries. Two callers from different teams could read each other's cached LLM responses by sending semantically similar prompts — embedding-only retrieval bypassed the model/team isolation that the standard cache backends get implicitly via the cache key. New ``litellm.caching._tenant_scope.get_tenant_scope`` helper extracts a stable scope identifier from ``user_api_key_team_id``, ``user_api_key_user_id``, ``user_api_key_org_id`` (joined with ``|`` in canonical order). Both semantic backends route storage and retrieval through this scope. - Qdrant: ``tenant_scope`` is added to the point payload on store and enforced via a ``query_filter`` on retrieve. Sentinel ``""`` for callers without proxy metadata so direct-SDK use shares its own legacy pool. - Redis: each tenant scope gets its own ``SemanticCache`` instance (lazy-created, hashed index name) so the two backends share no schema, no keys, no vector neighborhood. Direct-SDK callers continue to use the existing default index — no schema migration, no data loss for upgrading deployments. 15 mock-only unit tests cover the helper + both backends across sync/async set/get for tenant-scoped, no-tenant, and cross-tenant cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR adds tenant-scope isolation to
Confidence Score: 3/5Not safe to merge — two P1 defects: Qdrant upgrade silently invalidates the full existing cache, and Redis _tenant_caches grows unboundedly. Two distinct P1 findings on separate files bring the score below the P1 ceiling of 4. The Qdrant backward-compat break is a silent, immediate regression for all upgrading deployments; the Redis unbounded connection pool is a latent resource exhaustion. Neither is gated by a feature flag. litellm/caching/qdrant_semantic_cache.py (sentinel filter breaks existing entries) and litellm/caching/redis_semantic_cache.py (_tenant_caches growth + on-path SemanticCache construction).
|
| Filename | Overview |
|---|---|
| litellm/caching/_tenant_scope.py | New helper that extracts a stable tenant scope string from proxy-injected metadata; logic is clean, handles edge cases (non-dict metadata, empty strings, absent fields), no issues found. |
| litellm/caching/qdrant_semantic_cache.py | Adds tenant_scope to payload on writes and a must-match filter on reads; P1 issue — the sentinel "" filter silently drops all pre-existing points (which have no tenant_scope field) on upgrade, invalidating the entire existing Qdrant cache for no-tenant callers. |
| litellm/caching/redis_semantic_cache.py | Adds per-tenant lazy SemanticCache instances via _get_cache_for_scope; P1 issue — _tenant_caches is unbounded (one Redis connection pool per unique tenant combination, forever) and the first-use index creation happens synchronously on the critical request path. |
| tests/test_litellm/caching/test_semantic_cache_tenant_isolation.py | 15 mock-only unit tests covering helper, Qdrant, and Redis isolation paths; all mock-based (no real network calls, consistent with repo rules), but no test covers the Qdrant backward-compatibility gap (existing points without tenant_scope). |
Reviews (1): Last reviewed commit: "fix(caching): isolate semantic cache by ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| "filter": _qdrant_tenant_filter( | ||
| get_tenant_scope(kwargs) or _NO_TENANT_SCOPE | ||
| ), |
There was a problem hiding this comment.
All points stored before this PR have no tenant_scope field in their Qdrant payload. After upgrading, every get_cache call (even with no tenant) applies {"must": [{"key": "tenant_scope", "match": {"value": ""}}]}. Qdrant's match.value filter only returns points where the field exists and has that exact value — points missing the field entirely are excluded. The result is 100% cache miss for all pre-existing no-tenant callers, silently discarding every entry in the collection. The PR description claims "no data loss for upgrading deployments" but this only holds for the Redis backend; there is no analogous fallback path here. A migration step (e.g. bulk-update existing points with tenant_scope: "" via the Qdrant update-vectors API, or a conditional filter that also accepts the field being absent) is needed before the filter can safely be applied.
Rule Used: What: avoid backwards-incompatible changes without... (source)
| def _get_cache_for_scope(self, tenant_scope: Optional[str]) -> Any: | ||
| """Return the ``SemanticCache`` instance for a tenant scope. | ||
|
|
||
| ``None`` returns the default index (BC for callers without proxy | ||
| metadata). Any non-None scope gets its own RedisVL index, lazily | ||
| created on first use so two tenants share no keys, no schema, | ||
| and no vector neighborhood. | ||
| """ | ||
| if tenant_scope is None: | ||
| return self.llmcache | ||
| cached = self._tenant_caches.get(tenant_scope) | ||
| if cached is not None: | ||
| return cached | ||
| from hashlib import sha256 | ||
|
|
||
| from redisvl.extensions.llmcache import SemanticCache | ||
|
|
||
| # Hash the scope so the Redis index name is always a valid | ||
| # identifier (team_ids and user_ids can contain characters that | ||
| # RedisVL rejects in index names). | ||
| scope_hash = sha256(tenant_scope.encode("utf-8")).hexdigest()[:16] | ||
| scoped_name = f"{self._index_name_base}:tenant:{scope_hash}" | ||
| scoped = SemanticCache( | ||
| name=scoped_name, | ||
| redis_url=self._redis_url, | ||
| vectorizer=self._cache_vectorizer, | ||
| distance_threshold=self.distance_threshold, | ||
| overwrite=False, | ||
| ) | ||
| self._tenant_caches[tenant_scope] = scoped | ||
| return scoped |
There was a problem hiding this comment.
_tenant_caches dict can exhaust Redis connections
_tenant_caches grows indefinitely — one entry per unique (team_id, user_id, org_id) combination ever seen in the process lifetime. Each SemanticCache instance holds its own RedisVL connection pool. In a multi-tenant proxy with many teams or per-user scoping, this can exhaust Redis max-connections and OOM the proxy process. Additionally, the first request from any new tenant creates the SemanticCache synchronously inside the hot request path (index creation involves a Redis round-trip with overwrite=False), which can introduce latency spikes. An LRU-bounded cache or a fixed-capacity dict would cap resource usage.
Rule Used: What: Avoid creating new database requests or Rout... (source)
| cached = self._tenant_caches.get(tenant_scope) | ||
| if cached is not None: | ||
| return cached | ||
| from hashlib import sha256 | ||
|
|
||
| from redisvl.extensions.llmcache import SemanticCache | ||
|
|
||
| # Hash the scope so the Redis index name is always a valid | ||
| # identifier (team_ids and user_ids can contain characters that | ||
| # RedisVL rejects in index names). | ||
| scope_hash = sha256(tenant_scope.encode("utf-8")).hexdigest()[:16] | ||
| scoped_name = f"{self._index_name_base}:tenant:{scope_hash}" | ||
| scoped = SemanticCache( | ||
| name=scoped_name, | ||
| redis_url=self._redis_url, | ||
| vectorizer=self._cache_vectorizer, | ||
| distance_threshold=self.distance_threshold, | ||
| overwrite=False, | ||
| ) | ||
| self._tenant_caches[tenant_scope] = scoped |
There was a problem hiding this comment.
Two concurrent requests from the same new tenant can both find self._tenant_caches.get(tenant_scope) returning None, both call SemanticCache(...), and both write to self._tenant_caches[tenant_scope]. The last write wins and neither raises an error, but two separate Redis connection pools are momentarily created per tenant instead of one. Under high concurrency this doubles (or more) the connection spike on the first request for every tenant. A simple lock keyed on tenant_scope, or a check-then-cache pattern, would prevent this.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Three Greptile findings on the previous commit: 1. (P1, Qdrant) Pre-existing points have no ``tenant_scope`` field, so a strict ``match.value=""`` filter silently dropped every entry cached before this fix landed — a full cache wipe on upgrade for no-tenant callers. Switched the no-tenant filter to ``should`` with both an explicit ``""`` match AND ``is_empty`` so legacy points remain retrievable. 2. (P1, Redis) ``_tenant_caches`` was an unbounded dict — one ``SemanticCache`` instance per unique ``(team, user, org)`` ever seen, each holding its own RedisVL connection pool. Replaced with an ``OrderedDict``-backed LRU capped at 256 entries; eviction is FIFO by last access, so hot tenants stay warm. 3. (P2, Redis) Two concurrent first-touches for the same tenant could both construct a ``SemanticCache`` and race the dict write. Added double-checked locking — the slow path runs the constructor outside the lock (so a hot tenant's cache hit isn't blocked by a cold tenant's index check), then re-checks inside the lock and discards the loser. Adds three regression tests: - Qdrant no-tenant filter includes ``is_empty`` clause for legacy points - Redis LRU evicts oldest, keeps recently-used - Redis concurrent first-touch produces one instance, not two Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Closing as superseded by #26990, which targets litellm_internal_staging and links VERIA-54 to the active fix PR. |
1 participant
Relevant issues
n/a — operational hardening.
Pre-Submission checklist
Type
🐛 Bug Fix
Changes
Both `RedisSemanticCache` and `QdrantSemanticCache` ignored the proxy-injected tenant metadata when storing and retrieving cache entries. The standard cache backends (Redis exact-match, in-memory, S3) get isolation implicitly because team/user/org IDs are part of the request parameters that get hashed into the cache key. The semantic caches retrieve based on prompt embedding similarity, so two callers from different teams could retrieve each other's cached LLM responses by sending semantically similar prompts.
New helper
`litellm/caching/_tenant_scope.py:get_tenant_scope(kwargs) -> Optional[str]` extracts a stable scope identifier from `metadata.user_api_key_team_id`, `user_api_key_user_id`, `user_api_key_org_id` (joined with `|` in canonical order). Returns `None` when no scope is present (master key, direct SDK use) so callers can fall back to the legacy shared pool.
Qdrant — filter-based isolation
Redis — index-per-tenant isolation
Tests
15 mock-only unit tests in `tests/test_litellm/caching/test_semantic_cache_tenant_isolation.py`:
🤖 Generated with Claude Code