element-hq / element-hq/synapse

Sync race with get rooms for user cache invalidation over replication

Open
#14,154 0 comments 0 reactions 0 assignees View on GitHub
A-Sync A-Workers O-Occasional S-Major T-Defect
Dominant language
Python
Stars
4.6k
Forks
600
Avg merge
5d 22h
Merged PRs (30d)
51

Description

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

---

Over the last few weeks we have started seeing syncs with missing just-joined rooms. This led me to dive deep into how sync works and ended up with identifying a few cache invalidation race conditions, my understanding of things is as follows:

- [sync calls `notifier.wait_for_events`](https://github.com/matrix-org/synapse/blob/8074430d257c27e5a4d0f6ff38a908e3a7ee91ef/synapse/handlers/sync.py#L368)
- the notifier waits for events relevant to the users rooms (it gets the room list, this maybe cached!)
- [this is handled as events come in over replication](https://github.com/matrix-org/synapse/blob/efd108b45d1706526416bc9a6f89463b5ff4506a/synapse/replication/tcp/client.py#L195-L237)
- [before that handler, `process_replication_rows` is called](https://github.com/matrix-org/synapse/blob/efd108b45d1706526416bc9a6f89463b5ff4506a/synapse/replication/tcp/client.py#L147)
- which the cache database [processes here](https://github.com/matrix-org/synapse/blob/efd108b45d1706526416bc9a6f89463b5ff4506a/synapse/storage/databases/main/cache.py#L151-L153) then for [the event here](https://github.com/matrix-org/synapse/blob/efd108b45d1706526416bc9a6f89463b5ff4506a/synapse/storage/databases/main/cache.py#L188-L199)
- the `_invalidate_caches_for_event` call does NOT invalidate rooms for user, that is left to the [state invalidations over replication](https://github.com/matrix-org/synapse/blob/efd108b45d1706526416bc9a6f89463b5ff4506a/synapse/storage/databases/main/cache.py#L200-L207)
- but these are:
1. sent after the event over replication
2. nothing to do with the sync handling/notifier process, which is just the events
- so thus this means between event replication and state, there is a window when a sync may get notified about events whilst the get rooms for user cache remains invalid

I then confirmed my suspicious by adding a log line: https://github.com/beeper/synapse/commit/1346af1360e50d5db1a33b8faee7a26ba64fa621 which successfully identified the occurrences of this. I will now submit two different PRs to address this specific issue:

+ https://github.com/matrix-org/synapse/pull/14155
+ https://github.com/matrix-org/synapse/pull/14156

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.