[Feat] Add is_user_level property and cache_salt param to EvictionPolicy by royyhuang · Pull Request #3032 · LMCache/LMCache

royyhuang · 2026-04-14T22:56:39Z

Summary

Add is_user_level property to EvictionPolicy base class (default False)
Add cache_salt: str | None = None parameter to get_eviction_actions()
LRUEvictionPolicy and NoOpEvictionPolicy accept and ignore cache_salt

Why

The eviction controller needs a way to distinguish aggregate-level policies (LRU) from per-user-level policies (UserLRU, coming in a follow-up PR) without isinstance checks. is_user_level is the extension point — future policies override it to True and use the cache_salt parameter for scoped eviction.

Changes

File	Change
`eviction.py`	`is_user_level` property (default `False`); `cache_salt` param on `get_eviction_actions()`
`lru.py`	Accept (ignore) `cache_salt`
`noop.py`	Accept (ignore) `cache_salt`

Test plan

441 distributed tests pass
pre-commit clean
No behavioral change (all defaults are None, existing callers unaffected)

Context

PR4 of the per-user L2 quota feature. See design doc: docs/design/l2_adapters/l2_per_user_quota.md

Note

Low Risk
Low risk: this is a backward-compatible interface extension (new property + optional parameter) with no behavior change in existing policies, which simply accept/ignore cache_salt.

Overview
Adds an isolation-capability extension point to EvictionPolicy via a new support_isolation property (default False) and an optional cache_salt argument on get_eviction_actions() for scoping evictions.

Updates LRUEvictionPolicy and NoOpEvictionPolicy to accept the new cache_salt parameter (documented as ignored), keeping current eviction behavior unchanged while enabling future per-salt/user eviction policies.

^{Reviewed by Cursor Bugbot for commit 762966e. Bugbot is set up for automated code reviews on this repo. Configure here.}

Add is_user_level property to EvictionPolicy base class (default False). The eviction controller uses this to decide whether to check per-user quotas or aggregate usage — no isinstance checks needed. Add cache_salt optional parameter to get_eviction_actions(). When set, user-level policies scope eviction to keys belonging to that user. Non-user-level policies (LRU, NoOp) accept and ignore the parameter. This is an interface extension only — no new policies or behavioral changes. UserLRUEvictionPolicy (follow-up PR) will override is_user_level to return True and use cache_salt for scoped eviction. Signed-off-by: royyhuang <royyhuang@gmail.com> Signed-off-by: royyhuang <roy.y.huang@gmail.com>

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 5a8a031. Configure here.}

cursor · 2026-04-14T22:58:55Z

        self,
        expected_ratio: float,
        key_eligible_filter: Callable[[ObjectKey], bool] | None = None,
+        cache_salt: str | None = None,


Modified public method missing required docstring

Medium Severity

The get_eviction_actions method in NoOpEvictionPolicy is modified (new cache_salt parameter added) but has no docstring. Per AGENTS.md, all new/modified public functions require complete docstrings covering what the function does, arguments, return values, and exceptions. The LRUEvictionPolicy counterpart has a full docstring, so this is inconsistent.

^{Triggered by project rule: LMCache Code Review Style Guide}

^{Reviewed by Cursor Bugbot for commit 5a8a031. Configure here.}

cursor · 2026-04-14T22:58:55Z

+        Default is False. Subclasses that support per-user eviction
+        (e.g., ``UserLRUEvictionPolicy``) should override to return True.
+        """
+        return False


New features lack any corresponding tests

Medium Severity

The new is_user_level property and cache_salt parameter have zero test coverage. No tests verify that LRUEvictionPolicy and NoOpEvictionPolicy return False for is_user_level, nor that they accept and correctly ignore cache_salt. Per the project rules, new features require corresponding tests — this is classified as an error-level issue.

Additional Locations (1)

lmcache/v1/distributed/eviction.py#L89-L90

^{Triggered by project rule: LMCache Code Review Style Guide}

^{Reviewed by Cursor Bugbot for commit 5a8a031. Configure here.}

gemini-code-assist

Code Review

This pull request introduces infrastructure for per-user eviction by adding an is_user_level property to the EvictionPolicy base class and extending the get_eviction_actions method signature to include a cache_salt parameter. These changes are applied to the base class and the LRU and Noop policy implementations. Feedback highlights that the documentation in eviction.py references a cache_salt attribute on ObjectKey that does not currently exist, which should be corrected to ensure consistency and prevent potential errors.

gemini-code-assist · 2026-04-14T22:59:54Z

                skipped. This is useful for skipping locked keys that
                cannot be deleted.
+            cache_salt: When set, scope eviction to keys belonging to this
+                salt only (identified by ``ObjectKey.cache_salt``). When


The docstring refers to ObjectKey.cache_salt, but the ObjectKey class (defined in lmcache/v1/distributed/api.py) does not currently have a cache_salt attribute. This will lead to confusion or AttributeError when implementing policies that attempt to access this field. Please ensure ObjectKey is updated or correct the documentation reference.

ApostaC

Otherwise LGTM!

ApostaC · 2026-04-15T00:22:05Z

    """

+    @property
+    def is_user_level(self) -> bool:


nit: change to "support_isolation" maybe?

Signed-off-by: royyhuang <roy.y.huang@gmail.com>

sammshen

LGTM!

…icy (LMCache#3032) * [Feat] Add is_user_level property and cache_salt param to EvictionPolicy Add is_user_level property to EvictionPolicy base class (default False). The eviction controller uses this to decide whether to check per-user quotas or aggregate usage — no isinstance checks needed. Add cache_salt optional parameter to get_eviction_actions(). When set, user-level policies scope eviction to keys belonging to that user. Non-user-level policies (LRU, NoOp) accept and ignore the parameter. This is an interface extension only — no new policies or behavioral changes. UserLRUEvictionPolicy (follow-up PR) will override is_user_level to return True and use cache_salt for scoped eviction. Signed-off-by: royyhuang <royyhuang@gmail.com> Signed-off-by: royyhuang <roy.y.huang@gmail.com> * address comments Signed-off-by: royyhuang <roy.y.huang@gmail.com> --------- Signed-off-by: royyhuang <royyhuang@gmail.com> Signed-off-by: royyhuang <roy.y.huang@gmail.com>

royyhuang requested a review from YaoJiayi as a code owner April 14, 2026 22:56

royyhuang requested review from ApostaC and sammshen April 14, 2026 22:58

cursor Bot reviewed Apr 14, 2026

View reviewed changes

gemini-code-assist Bot reviewed Apr 14, 2026

View reviewed changes

ApostaC approved these changes Apr 15, 2026

View reviewed changes

royyhuang added 2 commits April 15, 2026 00:54

address comments

cf2cedc

Signed-off-by: royyhuang <roy.y.huang@gmail.com>

Merge branch 'dev' into feat/eviction-policy-interface

762966e

royyhuang enabled auto-merge (squash) April 15, 2026 00:55

github-actions Bot added the full Run comprehensive tests on this PR label Apr 15, 2026

sammshen approved these changes Apr 15, 2026

View reviewed changes

royyhuang merged commit 0177179 into LMCache:dev Apr 15, 2026
37 of 38 checks passed

royyhuang mentioned this pull request Apr 15, 2026

[MP][Feat] Add cache_salt to ObjectKey for cache isolation #3042

Merged

4 tasks

ruizhang0101 mentioned this pull request Apr 20, 2026

[MP][Feat] Tag MP metrics and spans with cache_salt for per-tenant attribution #3089

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feat] Add is_user_level property and cache_salt param to EvictionPolicy#3032

[Feat] Add is_user_level property and cache_salt param to EvictionPolicy#3032
royyhuang merged 3 commits intoLMCache:devfrom
royyhuang:feat/eviction-policy-interface

royyhuang commented Apr 14, 2026 •

edited by cursor Bot

Loading

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Apr 14, 2026

Uh oh!

cursor Bot Apr 14, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Apr 14, 2026

Uh oh!

ApostaC left a comment

Uh oh!

ApostaC Apr 15, 2026

Uh oh!

sammshen left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

royyhuang commented Apr 14, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Changes

Test plan

Context

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Apr 14, 2026

Choose a reason for hiding this comment

Modified public method missing required docstring

Uh oh!

cursor Bot Apr 14, 2026

Choose a reason for hiding this comment

New features lack any corresponding tests

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

ApostaC left a comment

Choose a reason for hiding this comment

Uh oh!

ApostaC Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

sammshen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

royyhuang commented Apr 14, 2026 •

edited by cursor Bot

Loading