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

Fix some instances of ExpiringCache not expiring cache items #3932

Merged
merged 2 commits into from
Sep 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/3932.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix some instances of ExpiringCache not expiring cache items
1 change: 0 additions & 1 deletion synapse/app/appservice.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ def start(config_options):

def start():
ps.get_datastore().start_profiling()
ps.get_state_handler().start_caching()

reactor.callWhenRunning(start)

Expand Down
1 change: 0 additions & 1 deletion synapse/app/client_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ def start(config_options):
ss.start_listening(config.worker_listeners)

def start():
ss.get_state_handler().start_caching()
ss.get_datastore().start_profiling()

reactor.callWhenRunning(start)
Expand Down
1 change: 0 additions & 1 deletion synapse/app/event_creator.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ def start(config_options):
ss.start_listening(config.worker_listeners)

def start():
ss.get_state_handler().start_caching()
ss.get_datastore().start_profiling()

reactor.callWhenRunning(start)
Expand Down
1 change: 0 additions & 1 deletion synapse/app/federation_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ def start(config_options):
ss.start_listening(config.worker_listeners)

def start():
ss.get_state_handler().start_caching()
ss.get_datastore().start_profiling()

reactor.callWhenRunning(start)
Expand Down
1 change: 0 additions & 1 deletion synapse/app/federation_sender.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ def start(config_options):

def start():
ps.get_datastore().start_profiling()
ps.get_state_handler().start_caching()

reactor.callWhenRunning(start)
_base.start_worker_reactor("synapse-federation-sender", config)
Expand Down
1 change: 0 additions & 1 deletion synapse/app/frontend_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,6 @@ def start(config_options):
ss.start_listening(config.worker_listeners)

def start():
ss.get_state_handler().start_caching()
ss.get_datastore().start_profiling()

reactor.callWhenRunning(start)
Expand Down
1 change: 0 additions & 1 deletion synapse/app/homeserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,6 @@ def setup(config_options):

def start():
hs.get_pusherpool().start()
hs.get_state_handler().start_caching()
hs.get_datastore().start_profiling()
hs.get_datastore().start_doing_background_updates()
hs.get_federation_client().start_get_pdu_cache()
Expand Down
1 change: 0 additions & 1 deletion synapse/app/media_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ def start(config_options):
ss.start_listening(config.worker_listeners)

def start():
ss.get_state_handler().start_caching()
ss.get_datastore().start_profiling()

reactor.callWhenRunning(start)
Expand Down
1 change: 0 additions & 1 deletion synapse/app/pusher.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,6 @@ def start(config_options):
def start():
ps.get_pusherpool().start()
ps.get_datastore().start_profiling()
ps.get_state_handler().start_caching()

reactor.callWhenRunning(start)

Expand Down
1 change: 0 additions & 1 deletion synapse/app/synchrotron.py
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,6 @@ def start(config_options):

def start():
ss.get_datastore().start_profiling()
ss.get_state_handler().start_caching()

reactor.callWhenRunning(start)

Expand Down
1 change: 0 additions & 1 deletion synapse/app/user_dir.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,6 @@ def start(config_options):

def start():
ps.get_datastore().start_profiling()
ps.get_state_handler().start_caching()

reactor.callWhenRunning(start)

Expand Down
28 changes: 12 additions & 16 deletions synapse/federation/federation_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,14 @@ def __init__(self, hs):
self.state = hs.get_state_handler()
self.transport_layer = hs.get_federation_transport_client()

self._get_pdu_cache = ExpiringCache(
cache_name="get_pdu_cache",
clock=self._clock,
max_len=1000,
expiry_ms=120 * 1000,
reset_expiry_on_get=False,
)

def _clear_tried_cache(self):
"""Clear pdu_destination_tried cache"""
now = self._clock.time_msec()
Expand All @@ -82,17 +90,6 @@ def _clear_tried_cache(self):
if destination_dict:
self.pdu_destination_tried[event_id] = destination_dict

def start_get_pdu_cache(self):
self._get_pdu_cache = ExpiringCache(
cache_name="get_pdu_cache",
clock=self._clock,
max_len=1000,
expiry_ms=120 * 1000,
reset_expiry_on_get=False,
)

self._get_pdu_cache.start()

@log_function
def make_query(self, destination, query_type, args,
retry_on_dns_fail=False, ignore_backoff=False):
Expand Down Expand Up @@ -229,10 +226,9 @@ def get_pdu(self, destinations, event_id, outlier=False, timeout=None):

# TODO: Rate limit the number of times we try and get the same event.

if self._get_pdu_cache:
ev = self._get_pdu_cache.get(event_id)
if ev:
defer.returnValue(ev)
ev = self._get_pdu_cache.get(event_id)
if ev:
defer.returnValue(ev)

pdu_attempts = self.pdu_destination_tried.setdefault(event_id, {})

Expand Down Expand Up @@ -285,7 +281,7 @@ def get_pdu(self, destinations, event_id, outlier=False, timeout=None):
)
continue

if self._get_pdu_cache is not None and signed_pdu:
if signed_pdu:
self._get_pdu_cache[event_id] = signed_pdu

defer.returnValue(signed_pdu)
Expand Down
1 change: 0 additions & 1 deletion synapse/rest/media/v1/preview_url_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ def __init__(self, hs, media_repo, media_storage):
# don't spider URLs more often than once an hour
expiry_ms=60 * 60 * 1000,
)
self._cache.start()

self._cleaner_loop = self.clock.looping_call(
self._start_expire_url_cache_data, 10 * 1000,
Expand Down
9 changes: 0 additions & 9 deletions synapse/state/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,6 @@ def __init__(self, hs):
self.hs = hs
self._state_resolution_handler = hs.get_state_resolution_handler()

def start_caching(self):
# TODO: remove this shim
self._state_resolution_handler.start_caching()

@defer.inlineCallbacks
def get_current_state(self, room_id, event_type=None, state_key="",
latest_event_ids=None):
Expand Down Expand Up @@ -428,9 +424,6 @@ def __init__(self, hs):
self._state_cache = None
self.resolve_linearizer = Linearizer(name="state_resolve_lock")

def start_caching(self):
logger.debug("start_caching")

self._state_cache = ExpiringCache(
cache_name="state_cache",
clock=self.clock,
Expand All @@ -440,8 +433,6 @@ def start_caching(self):
reset_expiry_on_get=True,
)

self._state_cache.start()

@defer.inlineCallbacks
@log_function
def resolve_state_groups(
Expand Down
1 change: 0 additions & 1 deletion synapse/util/caches/expiringcache.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ def __init__(self, cache_name, clock, max_len=0, expiry_ms=0,

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

def start(self):
if not self._expiry_ms:
# Don't bother starting the loop if things never expire
return
Expand Down
1 change: 0 additions & 1 deletion tests/util/test_expiring_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ def test_iterable_eviction(self):
def test_time_eviction(self):
clock = MockClock()
cache = ExpiringCache("test", clock, expiry_ms=1000)
cache.start()

cache["key"] = 1
clock.advance_time(0.5)
Expand Down