fix: use real LRU eviction in DirCache

fsspec/dircache.py

@@ -1,6 +1,6 @@
 import time
+from collections import OrderedDict
 from collections.abc import MutableMapping
-from functools import lru_cache
 
 
 class DirCache(MutableMapping):
@@ -48,7 +48,7 @@ def __init__(
         self._cache = {}
         self._times = {}
         if max_paths:
-            self._q = lru_cache(max_paths + 1)(lambda key: self._cache.pop(key, None))
+            self._q = OrderedDict()
         self.use_listings_cache = use_listings_cache
         self.listings_expiry_time = listings_expiry_time
         self.max_paths = max_paths
@@ -58,11 +58,15 @@ def __getitem__(self, item):
             if self._times.get(item, 0) - time.time() < -self.listings_expiry_time:
                 del self._cache[item]
         if self.max_paths:
-            self._q(item)
+            # Refresh the recency order; the item may already be evicted.
+            self._q.pop(item, None)
+            self._q[item] = None
         return self._cache[item]  # maybe raises KeyError
 
     def clear(self):
         self._cache.clear()
+        if self.max_paths:
+            self._q.clear()
 
     def __len__(self):
         return len(self._cache)
@@ -78,13 +82,20 @@ def __setitem__(self, key, value):
         if not self.use_listings_cache:
             return
         if self.max_paths:
-            self._q(key)
+            self._q.pop(key, None)
+            self._q[key] = None
+            while len(self._q) > self.max_paths:
+                oldest, _ = self._q.popitem(last=False)
+                self._cache.pop(oldest, None)
+                self._times.pop(oldest, None)
         self._cache[key] = value
         if self.listings_expiry_time is not None:
             self._times[key] = time.time()
 
     def __delitem__(self, key):
         del self._cache[key]
+        if self.max_paths:
+            self._q.pop(key, None)
 
     def __iter__(self):
         entries = list(self._cache)

fsspec/tests/test_dircache.py

@@ -0,0 +1,64 @@
+import time
+
+from fsspec.dircache import DirCache
+
+
+def test_max_paths_evicts_oldest_on_write():
+    dc = DirCache(max_paths=2)
+    dc["a"] = 1
+    dc["b"] = 2
+    dc["c"] = 3
+    dc["d"] = 4
+    # Only the two most recent entries are retained.
+    assert len(dc) == 2
+    assert dict(dc._cache) == {"c": 3, "d": 4}
+
+
+def test_max_paths_iteration_does_not_destroy_cache():
+    dc = DirCache(max_paths=2)
+    dc["a"] = 1
+    dc["b"] = 2
+    assert sorted(dc) == ["a", "b"]
+    # Iteration must not evict entries (regression: __iter__ used __getitem__,
+    # which popped entries out of the cache).
+    assert sorted(dc) == ["a", "b"]
+    assert len(dc) == 2
+
+
+def test_max_paths_access_refreshes_recency():
+    dc = DirCache(max_paths=2)
+    dc["a"] = 1
+    dc["b"] = 2
+    assert dc["a"] == 1  # refresh 'a'
+    dc["c"] = 3
+    # 'b' was the least recently used and should be evicted, not 'a'.
+    assert dict(dc._cache) == {"a": 1, "c": 3}
+
+
+def test_no_max_paths_keeps_all():
+    dc = DirCache()
+    for i in range(10):
+        dc[f"p{i}"] = i
+    assert len(dc) == 10
+    assert sorted(dc) == [f"p{i}" for i in range(10)]
+
+
+def test_deleted_path_removed_from_recency_tracking():
+    dc = DirCache(max_paths=2)
+    dc["a"] = 1
+    dc["b"] = 2
+    del dc["a"]
+    dc["c"] = 3
+    assert dict(dc._cache) == {"b": 2, "c": 3}
+
+
+def test_expiry_still_applies():
+    dc = DirCache(listings_expiry_time=0.1)
+    dc["a"] = 1
+    time.sleep(0.2)
+    try:
+        dc["a"]
+    except KeyError:
+        pass
+    else:
+        raise AssertionError("expected expired entry to raise KeyError")