diff --git a/docs/store_caching.rst b/docs/store_caching.rst index c129ef8..8ca6b3f 100644 --- a/docs/store_caching.rst +++ b/docs/store_caching.rst @@ -165,14 +165,22 @@ Limitations Statistics ---------- -``Store.stats`` includes cache counters: +``Store.stats`` includes counters for the primary backend: - ``backend_load_volume`` - ``backend_store_volume`` - ``backend_load_calls`` - ``backend_store_calls`` - ``backend_delete_calls`` -- ``cache_disabled`` + +It also includes cache information: + +- ``cache_enabled`` (``True`` if a cache backend is configured and was not + disabled at runtime) + +If a cache backend is configured, these keys are also present: + +- ``cache_disabled`` (``True`` if the cache backend failed to open at runtime) - ``cache_hits`` - ``cache_misses`` - ``cache_hit_ratio`` @@ -182,3 +190,6 @@ Statistics - ``cache_load_calls`` - ``cache_store_calls`` - ``cache_delete_calls`` + +Without a cache backend, ``cache_enabled`` is ``False`` and none of the other +``cache_*`` keys are present. \ No newline at end of file diff --git a/src/borgstore/store.py b/src/borgstore/store.py index 9e704d7..af573ba 100644 --- a/src/borgstore/store.py +++ b/src/borgstore/store.py @@ -485,17 +485,22 @@ def stats(self): st["backend_load_volume"] = st.get("backend_load_volume", 0) st["backend_gather_volume"] = st.get("backend_gather_volume", 0) st["backend_store_volume"] = st.get("backend_store_volume", 0) - st["cache_disabled"] = self._cache_disabled - st["cache_hits"] = st.get("cache_hits", 0) - st["cache_misses"] = st.get("cache_misses", 0) - cache_total = st["cache_hits"] + st["cache_misses"] - st["cache_hit_ratio"] = st["cache_hits"] / cache_total if cache_total else 0 - st["cache_errors"] = st.get("cache_errors", 0) - st["cache_load_calls"] = st.get("cache_load_calls", 0) - st["cache_store_calls"] = st.get("cache_store_calls", 0) - st["cache_delete_calls"] = st.get("cache_delete_calls", 0) - st["cache_load_volume"] = st.get("cache_load_volume", 0) - st["cache_store_volume"] = st.get("cache_store_volume", 0) + # True only if a cache backend is configured and it is not disabled at runtime. + st["cache_enabled"] = self.cache_backend is not None and not self._cache_disabled + if self.cache_backend is not None: + # the other cache stats only make sense if a cache is configured at all. + # cache_disabled is True if the cache backend failed to open at runtime. + st["cache_disabled"] = self._cache_disabled + st["cache_hits"] = st.get("cache_hits", 0) + st["cache_misses"] = st.get("cache_misses", 0) + cache_total = st["cache_hits"] + st["cache_misses"] + st["cache_hit_ratio"] = st["cache_hits"] / cache_total if cache_total else 0 + st["cache_errors"] = st.get("cache_errors", 0) + st["cache_load_calls"] = st.get("cache_load_calls", 0) + st["cache_store_calls"] = st.get("cache_store_calls", 0) + st["cache_delete_calls"] = st.get("cache_delete_calls", 0) + st["cache_load_volume"] = st.get("cache_load_volume", 0) + st["cache_store_volume"] = st.get("cache_store_volume", 0) return st def _get_levels(self, name): diff --git a/tests/test_cache.py b/tests/test_cache.py index 88395fa..4b613a3 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -1342,3 +1342,51 @@ def test_cache_load_negative_offset(tmp_path, mode): assert store.gather([("00000000", -3, 3), ("00000000", 0, 1)], namespace="data") == b"7890" finally: store.destroy() + + +def test_stats_without_cache_backend(tmp_path): + """No cache configured: cache_enabled is False and there are no other cache stats.""" + store, _ = make_store(tmp_path, with_cache_backend=False) + store.create() + try: + with store: + stats = store.stats + assert stats["cache_enabled"] is False + assert {key for key in stats if key.startswith("cache_")} == {"cache_enabled"} + finally: + store.destroy() + + +def test_stats_with_working_cache(tmp_path): + """Cache configured and working: cache_enabled is True, the cache counters are present.""" + store, _ = make_store(tmp_path, config=make_config({"data/": {"cache": CacheMode.C_WRITETHROUGH}})) + store.create() + try: + with store: + stats = store.stats + assert stats["cache_enabled"] is True + assert stats["cache_disabled"] is False + assert stats["cache_hits"] == 0 + assert stats["cache_load_calls"] == 0 + finally: + store.destroy() + + +def test_stats_with_cache_that_fails_to_open(tmp_path): + """Cache configured, but opening it failed: cache_enabled is False, cache_disabled is True.""" + store, _ = make_store(tmp_path, config=make_config({"data/": {"cache": CacheMode.C_WRITETHROUGH}})) + store.create() + + def failing_open(): + raise RuntimeError("boom") + + store.cache_backend.open = failing_open + try: + with store: + stats = store.stats + assert stats["cache_enabled"] is False + assert stats["cache_disabled"] is True + assert stats["cache_hits"] == 0 + assert stats["cache_errors"] == 0 + finally: + store.destroy() \ No newline at end of file diff --git a/tests/test_store.py b/tests/test_store.py index 323483e..78ea205 100644 --- a/tests/test_store.py +++ b/tests/test_store.py @@ -544,13 +544,10 @@ def test_stats(posixfs_store_created): store.load(key) assert store._stats["load_volume"] == 200 - # Assert default values for cache stats when cache is disabled + # Without a cache backend, cache_enabled is False and there are no other cache stats stats = store.stats - assert stats["cache_load_calls"] == 0 - assert stats["cache_store_calls"] == 0 - assert stats["cache_delete_calls"] == 0 - assert stats["cache_load_volume"] == 0 - assert stats["cache_store_volume"] == 0 + assert stats["cache_enabled"] is False + assert {k for k in stats if k.startswith("cache_")} == {"cache_enabled"} # Assert primary backend stats are tracked accurately assert stats["backend_store_calls"] == 4