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

Fix #8518 (sync requests being cached wrongly on timeout) #9358

Merged
merged 7 commits into from
Feb 24, 2021
Merged
Show file tree
Hide file tree
Changes from 5 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/9358.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added a fix that invalidates cache for empty timed-out sync responses.
ShadowJonathan marked this conversation as resolved.
Show resolved Hide resolved
7 changes: 6 additions & 1 deletion synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
from synapse.util.async_helpers import concurrently_execute
from synapse.util.caches.expiringcache import ExpiringCache
from synapse.util.caches.lrucache import LruCache
from synapse.util.caches.response_cache import ResponseCache
from synapse.util.caches.response_cache import NoTimedCache, ResponseCache
from synapse.util.metrics import Measure, measure_func
from synapse.visibility import filter_events_for_client

Expand Down Expand Up @@ -331,6 +331,11 @@ def current_sync_callback(before_token, after_token):
lazy_loaded = "false"
non_empty_sync_counter.labels(sync_type, lazy_loaded).inc()

# fixme hack: sanity check to invalidate cache, prevent a self-referral loop
# replace with contextvars approach once it gets working on twisted (twisted/twisted#1262)
if since_token == result.next_batch:
result = NoTimedCache(result) # type: ignore # because it is unwrapped in ResponseCache.set.<remove>()

return result

async def current_sync_for_user(
Expand Down
11 changes: 9 additions & 2 deletions synapse/util/caches/response_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import logging
from typing import TYPE_CHECKING, Any, Callable, Dict, Generic, Optional, TypeVar

import attr

from twisted.internet import defer

from synapse.logging.context import make_deferred_yieldable, run_in_background
Expand All @@ -29,6 +31,11 @@
T = TypeVar("T")


@attr.s(slots=True, frozen=True)
class NoTimedCache:
obj = attr.ib()


class ResponseCache(Generic[T]):
"""
This caches a deferred response. Until the deferred completes it will be
Expand Down Expand Up @@ -101,13 +108,13 @@ def set(self, key: T, deferred: defer.Deferred) -> defer.Deferred:
self.pending_result_cache[key] = result

def remove(r):
if self.timeout_sec:
if self.timeout_sec and not isinstance(r, NoTimedCache):
self.clock.call_later(
self.timeout_sec, self.pending_result_cache.pop, key, None
)
else:
self.pending_result_cache.pop(key, None)
return r
return r.obj if isinstance(r, NoTimedCache) else r

result.addBoth(remove)
return result.observe()
Expand Down