merge master into provider-aware model selection
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
commit
c5ff4f5f3f
2 files changed
+106
-1
No files matched your search
+17
-1
@@ -63,7 +63,16 @@ _CLI_SESSIONS_CACHE_STREAMING_TTL_SECONDS = 45.0
|
||||
_CLI_SESSIONS_CACHE_LOCK = threading.Lock()
|
||||
_CLI_SESSIONS_CACHE_INFLIGHT: "dict[tuple, threading.Event]" = {}
|
||||
_CLI_SESSIONS_CACHE_INVALIDATION_VERSION = 0
|
||||
_CLI_SESSIONS_CACHE = {}
|
||||
# LRU-bounded (drop-oldest) so a long-lived process under churn — where the
|
||||
# state.db fingerprint advances on every streamed message and the structural
|
||||
# clear-on-mutation listener doesn't fire for every fingerprint advance — can't
|
||||
# accumulate orphaned heavy deepcopies. Each value is a copy.deepcopy() of the
|
||||
# full CLI/cron session list (the expensive projection behind #4842/#4672), so
|
||||
# the cap is deliberately small. TTL is still the primary freshness control;
|
||||
# the cap is the backstop that the plain dict previously lacked. Mirrors the
|
||||
# _CLAUDE_CODE_PARSE_CACHE / _SIDECAR_METADATA_CACHE LRU pattern.
|
||||
_CLI_SESSIONS_CACHE: "collections.OrderedDict[tuple, tuple]" = collections.OrderedDict()
|
||||
_CLI_SESSIONS_CACHE_MAX_ENTRIES = 8
|
||||
_CLI_SESSIONS_CACHE_WAIT_SECONDS = 0.25
|
||||
# Event waits that keep stale rows visible while a rebuild is in flight.
|
||||
_CLI_SESSIONS_CACHE_STALE_WAIT_SECONDS = 0.10
|
||||
@@ -6405,6 +6414,9 @@ def _cache_cli_sessions_if_current(
|
||||
invalidation_stamp,
|
||||
_copy_cli_sessions(sessions),
|
||||
)
|
||||
_CLI_SESSIONS_CACHE.move_to_end(cache_key)
|
||||
while len(_CLI_SESSIONS_CACHE) > _CLI_SESSIONS_CACHE_MAX_ENTRIES:
|
||||
_CLI_SESSIONS_CACHE.popitem(last=False)
|
||||
return True
|
||||
|
||||
|
||||
@@ -6423,6 +6435,8 @@ def _copy_fresh_cli_sessions_cache_entry(cache_key: tuple):
|
||||
return None
|
||||
if cached_expires_at <= time.monotonic():
|
||||
return None
|
||||
# LRU: a fresh hit is the most-recently-used entry.
|
||||
_CLI_SESSIONS_CACHE.move_to_end(cache_key)
|
||||
return _copy_cli_sessions(cached_sessions)
|
||||
|
||||
|
||||
@@ -7272,6 +7286,8 @@ def get_cli_sessions(
|
||||
if cached_stamp != _CLI_SESSIONS_CACHE_INVALIDATION_VERSION:
|
||||
_CLI_SESSIONS_CACHE.pop(cache_key, None)
|
||||
elif cached_expires_at > now:
|
||||
# LRU: a fresh hit is the most-recently-used entry.
|
||||
_CLI_SESSIONS_CACHE.move_to_end(cache_key)
|
||||
return _copy_cli_sessions(cached_sessions)
|
||||
else:
|
||||
stale_sessions = _copy_cli_sessions(cached_sessions)
|
||||
|
||||
@@ -0,0 +1,89 @@
|
||||
"""Tests: ``_CLI_SESSIONS_CACHE`` is LRU-bounded (drop-oldest).
|
||||
|
||||
Regression: the CLI/cron session projection cache was a plain ``dict`` with TTL
|
||||
storage but NO size cap and only lazy, same-key eviction. Each value is a
|
||||
``copy.deepcopy()`` of the full CLI/cron session list (the expensive projection
|
||||
behind #4842/#4672). The cache key folds in a state.db content fingerprint that
|
||||
advances on every streamed message, so the next poll builds a NEW key; the
|
||||
previous key's heavy deepcopy was never looked up again, so its lazy-expiry path
|
||||
never ran. A long-lived process under churn could accumulate orphaned heavy
|
||||
deepcopies until the next structural ``clear_cli_sessions_cache()``.
|
||||
|
||||
The cache is now an ``OrderedDict`` capped at ``_CLI_SESSIONS_CACHE_MAX_ENTRIES``
|
||||
with drop-oldest on write (mirroring ``_CLAUDE_CODE_PARSE_CACHE``) and
|
||||
``move_to_end`` on fresh hits. TTL remains the primary freshness control; the
|
||||
cap is the backstop the plain dict lacked.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import api.models as models
|
||||
from api.models import (
|
||||
_CLI_SESSIONS_CACHE_MAX_ENTRIES,
|
||||
_cache_cli_sessions_if_current,
|
||||
_cli_sessions_cache_invalidation_stamp,
|
||||
_copy_fresh_cli_sessions_cache_entry,
|
||||
)
|
||||
|
||||
|
||||
def _reset_cache():
|
||||
with models._CLI_SESSIONS_CACHE_LOCK:
|
||||
models._CLI_SESSIONS_CACHE.clear()
|
||||
|
||||
|
||||
def test_cache_is_bounded_drops_oldest_on_write():
|
||||
"""Inserting more than MAX_ENTRIES distinct keys keeps len == MAX and evicts
|
||||
the OLDEST (least-recently-used) entry."""
|
||||
_reset_cache()
|
||||
stamp = _cli_sessions_cache_invalidation_stamp()
|
||||
cap = _CLI_SESSIONS_CACHE_MAX_ENTRIES
|
||||
assert cap >= 1
|
||||
|
||||
# Insert cap + 5 distinct keys, each a distinct payload so we can track them.
|
||||
for i in range(cap + 5):
|
||||
key = (f"profile-{i}",)
|
||||
ok = _cache_cli_sessions_if_current(
|
||||
key, ttl=60.0, invalidation_stamp=stamp, sessions=[{"id": i}]
|
||||
)
|
||||
assert ok
|
||||
|
||||
with models._CLI_SESSIONS_CACHE_LOCK:
|
||||
assert len(models._CLI_SESSIONS_CACHE) == cap
|
||||
keys = list(models._CLI_SESSIONS_CACHE.keys())
|
||||
# The oldest 5 (profile-0..4) were evicted; the newest `cap` remain.
|
||||
assert keys[0] == ("profile-5",)
|
||||
assert keys[-1] == (f"profile-{cap + 5 - 1}",)
|
||||
assert ("profile-0",) not in keys
|
||||
|
||||
|
||||
def test_fresh_hit_marks_entry_most_recently_used():
|
||||
"""A fresh cache hit moves the entry to the end (most-recently-used), so a
|
||||
frequently-read entry is NOT evicted over colder ones."""
|
||||
_reset_cache()
|
||||
stamp = _cli_sessions_cache_invalidation_stamp()
|
||||
cap = _CLI_SESSIONS_CACHE_MAX_ENTRIES
|
||||
|
||||
# Fill the cache exactly.
|
||||
for i in range(cap):
|
||||
_cache_cli_sessions_if_current(
|
||||
(f"k{i}",), ttl=60.0, invalidation_stamp=stamp, sessions=[{"id": i}]
|
||||
)
|
||||
# Touch k0 (the oldest) so it becomes most-recently-used.
|
||||
served = _copy_fresh_cli_sessions_cache_entry(("k0",))
|
||||
assert served == [{"id": 0}]
|
||||
# Now insert one more; the eviction victim should be k1, NOT k0.
|
||||
_cache_cli_sessions_if_current(
|
||||
("k_new",), ttl=60.0, invalidation_stamp=stamp, sessions=[{"id": 999}]
|
||||
)
|
||||
with models._CLI_SESSIONS_CACHE_LOCK:
|
||||
keys = list(models._CLI_SESSIONS_CACHE.keys())
|
||||
assert ("k0",) in keys # survived because it was just used
|
||||
assert ("k1",) not in keys # evicted as the new oldest
|
||||
assert ("k_new",) in keys
|
||||
|
||||
|
||||
def test_cache_is_ordered_dict_not_plain_dict():
|
||||
"""The regression: the cache used to be a plain dict with no ordering. It
|
||||
must now be an OrderedDict to support move_to_end / popitem(last=False)."""
|
||||
import collections
|
||||
|
||||
assert isinstance(models._CLI_SESSIONS_CACHE, collections.OrderedDict)
|
||||
Reference in new issue
Block a user