mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-09 21:49:37 +00:00
fix(skills): refresh user storage after config reload (#4972)
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
431892e16a
commit
013dca6352
@ -29,7 +29,7 @@ _MAX_USER_SCOPED_STORAGES = 64
|
||||
# OrderedDict so that LRU eviction can remove the least-recently-used entry
|
||||
# via ``move_to_end`` + ``popitem(last=False)`` when the cache exceeds
|
||||
# ``_MAX_USER_SCOPED_STORAGES``.
|
||||
_user_scoped_storages: OrderedDict[str, UserScopedSkillStorage] = OrderedDict()
|
||||
_user_scoped_storages: OrderedDict[str, tuple[object, UserScopedSkillStorage]] = OrderedDict()
|
||||
_user_scoped_storage_lock = threading.Lock()
|
||||
|
||||
|
||||
@ -113,34 +113,40 @@ def get_or_new_user_skill_storage(user_id: str, **kwargs) -> SkillStorage:
|
||||
are safely bucketed before reaching :class:`UserScopedSkillStorage`, which
|
||||
calls :func:`_validate_user_id` internally.
|
||||
|
||||
Instances are cached by the *normalised* ``user_id`` with double-check
|
||||
locking to prevent concurrent creation races. When the cache exceeds
|
||||
``_MAX_USER_SCOPED_STORAGES``, the least-recently-accessed entry is
|
||||
evicted (true LRU, not FIFO).
|
||||
Instances are cached by the *normalised* ``user_id`` and current
|
||||
``AppConfig`` identity with double-check locking to prevent concurrent
|
||||
creation races. When the cache exceeds ``_MAX_USER_SCOPED_STORAGES``, the
|
||||
least-recently-accessed entry is evicted (true LRU, not FIFO).
|
||||
"""
|
||||
from deerflow.config import get_app_config
|
||||
from deerflow.config.paths import make_safe_user_id
|
||||
|
||||
safe_id = make_safe_user_id(user_id)
|
||||
app_config = kwargs.get("app_config")
|
||||
if app_config is None:
|
||||
app_config = get_app_config()
|
||||
kwargs["app_config"] = app_config
|
||||
|
||||
# Always acquire lock so move_to_end is safe — makes this a true LRU
|
||||
# cache instead of FIFO. The overhead is negligible since dict ops are
|
||||
# fast and this function is called once per agent-creation cycle.
|
||||
with _user_scoped_storage_lock:
|
||||
cached = _user_scoped_storages.get(safe_id)
|
||||
if cached is not None:
|
||||
if cached is not None and cached[0] is app_config:
|
||||
_user_scoped_storages.move_to_end(safe_id)
|
||||
return cached
|
||||
return cached[1]
|
||||
|
||||
cached = UserScopedSkillStorage(safe_id, **kwargs)
|
||||
_user_scoped_storages[safe_id] = cached
|
||||
storage = UserScopedSkillStorage(safe_id, **kwargs)
|
||||
_user_scoped_storages[safe_id] = (app_config, storage)
|
||||
_user_scoped_storages.move_to_end(safe_id)
|
||||
# Evict least-recently-used entry if cache exceeds the ceiling.
|
||||
# Since we just moved the current user_id to the end, popitem(last=False)
|
||||
# will evict the oldest/least-recently-accessed entry (never the
|
||||
# one we just created).
|
||||
while len(_user_scoped_storages) > _MAX_USER_SCOPED_STORAGES:
|
||||
evicted_key, evicted_val = _user_scoped_storages.popitem(last=False)
|
||||
evicted_key, _ = _user_scoped_storages.popitem(last=False)
|
||||
logger.info("Evicted user-scoped skill storage for safe_id=%s (cache ceiling %d)", evicted_key, _MAX_USER_SCOPED_STORAGES)
|
||||
return cached
|
||||
return storage
|
||||
|
||||
|
||||
def user_should_see_legacy_skills(user_id: str, **kwargs) -> bool:
|
||||
|
||||
@ -284,6 +284,25 @@ def test_different_users_get_different_storages(monkeypatch):
|
||||
skill_storage.reset_skill_storage()
|
||||
|
||||
|
||||
def test_user_storage_is_rebuilt_when_app_config_changes(monkeypatch):
|
||||
"""A hot-reloaded config must not reuse storage bound to the old paths."""
|
||||
skill_storage.reset_skill_storage()
|
||||
SlowUserSkillStorage.instances_created = 0
|
||||
_patch_user_storage_resolution(monkeypatch)
|
||||
|
||||
first_config = _AppConfig()
|
||||
second_config = _AppConfig()
|
||||
first = skill_storage.get_or_new_user_skill_storage("alice", app_config=first_config)
|
||||
second = skill_storage.get_or_new_user_skill_storage("alice", app_config=second_config)
|
||||
|
||||
try:
|
||||
assert second is not first
|
||||
assert skill_storage.get_or_new_user_skill_storage("alice", app_config=second_config) is second
|
||||
assert SlowUserSkillStorage.instances_created == 2
|
||||
finally:
|
||||
skill_storage.reset_skill_storage()
|
||||
|
||||
|
||||
def test_reset_user_skill_storage_only_clears_target_user(monkeypatch):
|
||||
"""Resetting alice's storage must not invalidate bob's."""
|
||||
skill_storage.reset_skill_storage()
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user