Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Fix ExpiringCache.__len__ to be accurate
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
erikjohnston authored and richvdh committed Sep 26, 2018
1 parent f651636 commit 3baf6e1
Showing 1 changed file with 7 additions and 10 deletions.
17 changes: 7 additions & 10 deletions synapse/util/caches/expiringcache.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import logging
from collections import OrderedDict

from six import itervalues

from synapse.metrics.background_process_metrics import run_as_background_process
from synapse.util.caches import register_cache

Expand Down Expand Up @@ -54,8 +56,6 @@ def __init__(self, cache_name, clock, max_len=0, expiry_ms=0,

self.iterable = iterable

self._size_estimate = 0

self.metrics = register_cache("expiring", cache_name, self)

if not self._expiry_ms:
Expand All @@ -74,16 +74,11 @@ def __setitem__(self, key, value):
now = self._clock.time_msec()
self._cache[key] = _CacheEntry(now, value)

if self.iterable:
self._size_estimate += len(value)

# Evict if there are now too many items
while self._max_len and len(self) > self._max_len:
_key, value = self._cache.popitem(last=False)
if self.iterable:
removed_len = len(value.value)
self.metrics.inc_evictions(removed_len)
self._size_estimate -= removed_len
self.metrics.inc_evictions(len(value.value))
else:
self.metrics.inc_evictions()

Expand Down Expand Up @@ -134,7 +129,9 @@ def _prune_cache(self):
for k in keys_to_delete:
value = self._cache.pop(k)
if self.iterable:
self._size_estimate -= len(value.value)
self.metrics.inc_evictions(len(value.value))
else:
self.metrics.inc_evictions()

logger.debug(
"[%s] _prune_cache before: %d, after len: %d",
Expand All @@ -143,7 +140,7 @@ def _prune_cache(self):

def __len__(self):
if self.iterable:
return self._size_estimate
return sum(len(entry.value) for entry in itervalues(self._cache))
else:
return len(self._cache)

Expand Down

0 comments on commit 3baf6e1

Please sign in to comment.