element-hq / element-hq/synapse

Split out mutable event content from event cache into new caches that are keyed by room ID

Open
#13,916 0 comments 0 reactions 0 assignees View on GitHub
A-Database A-Memory-Usage T-Other Z-Time-Tracked
Dominant language
Python
Stars
4.6k
Forks
600
Avg merge
5d 22h
Merged PRs (30d)
51

Description

This issue has been migrated from [#13916](https://github.com/matrix-org/synapse/issues/13916).

---

In the hopes of fixing https://github.com/matrix-org/synapse/issues/11521 and paving the way towards an immutable external event cache (https://github.com/matrix-org/synapse/issues/2123), a new architecture for the event cache is proposed.

### The current state

Currently there are a couple separate data structures related to caching event contents in memory in Synapse (see code for fetching an event from cache/db [here](https://github.com/matrix-org/synapse/blob/d3d9ca156e323fe194b1bcb1af1628f65a2f3c1c/synapse/storage/databases/main/events_worker.py#L439)):

* `EventsWorkerStore._get_event_cache` - An instance of [AsyncLruCache](https://github.com/matrix-org/synapse/blob/41320a0554716aaf7cec6172da98e002c48344c5/synapse/util/caches/lrucache.py#L821) implemented as a map of `EventID` -> (`EventBase`, redacted `EventBase` | None).
* This cache is populated after fetching an event from the database.
* Entries in this cache are invalidated when an event is deleted from the database (in most cases, c.f. #11521), redacted or marked as rejected.
* Entries in this cache are invalidated when the size limit of the LruCache are reached.
* `EventsWorkerStore._event_ref` - A [WeakValueDictionary](https://docs.python.org/3/library/weakref.html#weakref.WeakValueDictionary) which serves as a single point of reference for EventBase's in memory, ensuring that we don't end up with multiple, unnecessary copies of a single EventBase in memory.
* This data structure is populated after fetched an event from the database.
* Because this is a WeakValueDictionary, entries in this cache are invalidated when all other references to the EventBase in an entry are gone.
* Entries in this cache are invalidated when an event is deleted from the database (in most cases, c.f. #11521), redacted or marked as rejected.
* Entries in this cache are **not** invalidated when an entry is evicted from `EventsWorkerStore._get_event_cache`, as something else may still be processing the event, even if it's been removed from that cache.

### What's the problem?

See https://github.com/matrix-org/synapse/issues/11521; because each of these caches are keyed by EventID alone, it becomes tricky to invalidate them when all you have is a RoomID (i.e. when purging a room completely). We could query all known events for a room from the database, but that may result in millions of events. Ideally we'd have some map of RoomID -> EventID which only covers the events that are actually currently held in memory. We could then use that to invalidate all three of these caches.

Additionally, as `get_event_cache` contains mutable `EventCacheEntry`s (comprised of `EventBase, redacted EventBase | None`), invalidating them is necessary when an event is both redacted or marked as rejected. These can differ per-homeserver, so removing this component from the cache entries opens up avenues for multiple homeservers sharing the same, immutable event cache.

### Proposal

After speaking with @erikjohnston we've (mostly Erik :) came up with the following idea:

* `EventsWorkerStore._get_event_cache` would simply become a map of `EventID` -> `EventBase`.
* We add a separate cache which is a nested map of `RoomID` -> `EventID` -> `{rejected_status: bool, redacted_event_content: Optional[EventBase]}`.
* Entries are added to this map when an event is pulled from the database. We know the RoomID at this point.
* Entries are **not** invalidated from this map when an entry is `EventsWorkerStore._get_event_cache` is invalidated due to hitting the cache size.
* This does mean that we'll need to know the RoomID when querying for rejected/redacted status though... But we can get that from the event cache?

The beauty of this is that we no longer need to invalidate the `_get_event_cache` at all (unless the size limit is hit)! Even in the room purge use case! How? Here are some examples of using this system:

#### Fetch EventID A which is not in-memory

1. Some calling function asks for EventID A.
2. This does not exist in `_get_event_cache` (nor other caches) so we query from the database. The event and related metadata is fetched from the DB (`event_json`, `redactions`, `rejections`) and both the `_get_event_cache` and event metadata cache are populated.
3. Return information from the database.

#### Fetch EventID A which *is* in-memory

1. Some calling function asks for EventID A.
2. This already exists in `_get_event_cache`, and presumably the metadata cache. We take the RoomID from the EventBase in the `_get_event_cache` and query the event metadata cache.
4. Return information from both caches.

#### EventID A is not in-memory but the event has been purged

1. Some calling function asks for EventID A.
2. This already exists in `_get_event_cache`, and presumably the metadata cache. We take the RoomID from the EventBase in the cache and query the event metadata cache - but uh oh, there's no matching entry in the metadata cache! The event must have been purged.
3. We invalidate the entry in the event cache as well and return None.

Thus when purging a room, we only need to purge entries in the metadata cache (which we can easily do by RoomID due to the metadata cache's structure). Entries in the `get_event_cache` and `event_ref` will be invalidated as they are fetched.

I'm curious for thoughts on whether this sounds reasonable from other members of the Synapse team + cc @Fizzadar.

Contributor guide

Open the contributing guide

Research direction

Start in synapse/storage/databases/main/events_worker.py, especially EventsWorkerStore._get_event_cache and _event_ref, and review the linked cache implementations in synapse/util/caches/lrucache.py. Trace event fetching, redaction, rejection, and room purge paths. Done means the proposed room-keyed metadata cache separates mutable content from the event cache and preserves correct invalidation behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.