Skip to content
Snippets Groups Projects
Commit 3baf6e16 authored by Erik Johnston's avatar Erik Johnston Committed by Richard van der Hoff
Browse files

Fix ExpiringCache.__len__ to be accurate

It used to try and produce an estimate, which was sometimes negative.
This caused metrics to be sad, so lets always just calculate it from
scratch.

(This appears to have been a longstanding bug, but one which has been made more
of a problem by #3932 and #3933).

(This was originally done by Erik as part of #3933. I'm cherry-picking it
because really it's a fix in its own right)
parent f6516362
No related branches found
No related tags found
No related merge requests found
...@@ -16,6 +16,8 @@ ...@@ -16,6 +16,8 @@
import logging import logging
from collections import OrderedDict from collections import OrderedDict
from six import itervalues
from synapse.metrics.background_process_metrics import run_as_background_process from synapse.metrics.background_process_metrics import run_as_background_process
from synapse.util.caches import register_cache from synapse.util.caches import register_cache
...@@ -54,8 +56,6 @@ class ExpiringCache(object): ...@@ -54,8 +56,6 @@ class ExpiringCache(object):
self.iterable = iterable self.iterable = iterable
self._size_estimate = 0
self.metrics = register_cache("expiring", cache_name, self) self.metrics = register_cache("expiring", cache_name, self)
if not self._expiry_ms: if not self._expiry_ms:
...@@ -74,16 +74,11 @@ class ExpiringCache(object): ...@@ -74,16 +74,11 @@ class ExpiringCache(object):
now = self._clock.time_msec() now = self._clock.time_msec()
self._cache[key] = _CacheEntry(now, value) self._cache[key] = _CacheEntry(now, value)
if self.iterable:
self._size_estimate += len(value)
# Evict if there are now too many items # Evict if there are now too many items
while self._max_len and len(self) > self._max_len: while self._max_len and len(self) > self._max_len:
_key, value = self._cache.popitem(last=False) _key, value = self._cache.popitem(last=False)
if self.iterable: if self.iterable:
removed_len = len(value.value) self.metrics.inc_evictions(len(value.value))
self.metrics.inc_evictions(removed_len)
self._size_estimate -= removed_len
else: else:
self.metrics.inc_evictions() self.metrics.inc_evictions()
...@@ -134,7 +129,9 @@ class ExpiringCache(object): ...@@ -134,7 +129,9 @@ class ExpiringCache(object):
for k in keys_to_delete: for k in keys_to_delete:
value = self._cache.pop(k) value = self._cache.pop(k)
if self.iterable: if self.iterable:
self._size_estimate -= len(value.value) self.metrics.inc_evictions(len(value.value))
else:
self.metrics.inc_evictions()
logger.debug( logger.debug(
"[%s] _prune_cache before: %d, after len: %d", "[%s] _prune_cache before: %d, after len: %d",
...@@ -143,7 +140,7 @@ class ExpiringCache(object): ...@@ -143,7 +140,7 @@ class ExpiringCache(object):
def __len__(self): def __len__(self):
if self.iterable: if self.iterable:
return self._size_estimate return sum(len(entry.value) for entry in itervalues(self._cache))
else: else:
return len(self._cache) return len(self._cache)
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment