Sliding Sync: Make sure we get up-to-date information from `get_sliding_sync_rooms_for_user(...)` #17692

MadLittleMods · 2024-09-10T22:12:07Z

Make sure we get up-to-date information from get_sliding_sync_rooms_for_user(...). We need to bust the get_sliding_sync_rooms_for_user cache when the room encryption is updated and any other field that is used in the query.

Follow-up to #17630

TODO

Bust cache for membership change (cross-reference get_rooms_for_user)
Bust cache for room encryption (cross-reference get_room_encryption)
Bust cache for forgotten (cross-reference did_forget/get_forgotten_rooms_for_user)

Pull Request Checklist

Pull request is based on the develop branch
Pull request includes a changelog file. The entry should:
- Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
- Use markdown where necessary, mostly for code blocks.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
Code style is correct
(run the linters)

Make sure we get up-to-date information from `get_sliding_sync_rooms_for_user(...)`

MadLittleMods · 2024-09-10T22:23:21Z

synapse/storage/databases/main/cache.py

@@ -483,6 +488,7 @@ def _invalidate_caches_for_room(self, room_id: str) -> None:
        self._attempt_to_invalidate_cache("get_room_version_id", (room_id,))
        self._attempt_to_invalidate_cache("get_room_type", (room_id,))
        self._attempt_to_invalidate_cache("get_room_encryption", (room_id,))
+        self._attempt_to_invalidate_cache("get_sliding_sync_rooms_for_user", None)


get_sliding_sync_rooms_for_user(...) doesn't cache very well because we have to bust it whenever state in any room changes. Is it even worth caching it?

The reason I discovered this in the first place is because I tried adding COALESCE(j.event_stream_ordering, m.event_stream_ordering) for sorting and discovered that get_sliding_sync_rooms_for_user(...) was returning a bunch of cached values. And busting the cache for any event sent on the homeserver just doesn't make sense to do.

But busting the cache when any state across the homeserver seems just as bad.

MadLittleMods · 2024-09-10T22:25:08Z

synapse/storage/databases/main/cache.py

@@ -275,6 +275,9 @@ def _process_event_stream_row(self, token: int, row: EventsStreamRow) -> None:
                self._attempt_to_invalidate_cache(
                    "get_room_encryption", (data.room_id,)
                )
+                self._attempt_to_invalidate_cache(
+                    "get_sliding_sync_rooms_for_user", None
+                )


Busting the cache wherever we bust it for get_room_encryption already

MadLittleMods · 2024-09-10T22:27:52Z

synapse/storage/databases/main/roommember.py

@@ -1348,6 +1348,9 @@ def f(txn: LoggingTransaction) -> None:
            self._invalidate_cache_and_stream(
                txn, self.get_forgotten_rooms_for_user, (user_id,)
            )
+            self._invalidate_cache_and_stream(
+                txn, self.get_sliding_sync_rooms_for_user, (user_id,)
+            )


Busting the cache whenever we bust the cache for did_forget/get_forgotten_rooms_for_user

…ooms_for_user(...)`

MadLittleMods · 2024-09-10T23:17:20Z

tests/rest/client/sliding_sync/test_sliding_sync.py

+                },
+            }
+        }
+        response_body, from_token = self.do_sync(sync_body, tok=user1_tok)


Unrelated, just updating to use the do_sync helper

MadLittleMods · 2024-09-10T23:17:25Z

tests/rest/client/sliding_sync/test_sliding_sync.py

@@ -831,6 +820,98 @@ def test_filter_regardless_of_membership_server_left_room(self) -> None:
            ],
        )

+    def test_filter_is_encrypted_up_to_date(self) -> None:


MadLittleMods · 2024-09-11T17:14:17Z

Thanks for the review @erikjohnston 🐕

MadLittleMods added 4 commits September 10, 2024 16:40

Bust the cache when the room encryption is updated

52797dc

Make sure we get up-to-date information from `get_sliding_sync_rooms_for_user(...)`

Add other tests

a384147

Use rest-layer test

e0fc7ec

Fix lints

f0f06ea

MadLittleMods added the A-Sync label Sep 10, 2024

Add changelog

3152d5c

MadLittleMods commented Sep 10, 2024

View reviewed changes

Bust cache when we forget

62b1745

MadLittleMods commented Sep 10, 2024

View reviewed changes

MadLittleMods changed the title ~~Make sure we get up-to-date information from get_sliding_sync_rooms_for_user(...)~~ Sliding Sync: Make sure we get up-to-date information from get_sliding_sync_rooms_for_user(...) Sep 10, 2024

MadLittleMods added 2 commits September 10, 2024 17:57

Make sure we bust cache wherever we already bust the cache for `get_r…

cb17f39

…ooms_for_user(...)`

Fix lints

260974c

MadLittleMods commented Sep 10, 2024

View reviewed changes

MadLittleMods marked this pull request as ready for review September 10, 2024 23:17

MadLittleMods requested a review from a team as a code owner September 10, 2024 23:17

erikjohnston approved these changes Sep 11, 2024

View reviewed changes

MadLittleMods merged commit e4a1f27 into develop Sep 11, 2024
39 checks passed

MadLittleMods deleted the madlittlemods/get_sliding_sync_rooms_for_user-bust-cache branch September 11, 2024 17:13

MadLittleMods mentioned this pull request Sep 11, 2024

Sliding sync: remove the list ops from the response #17671

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sliding Sync: Make sure we get up-to-date information from `get_sliding_sync_rooms_for_user(...)` #17692

Sliding Sync: Make sure we get up-to-date information from `get_sliding_sync_rooms_for_user(...)` #17692

MadLittleMods commented Sep 10, 2024 •

edited

Loading

MadLittleMods Sep 10, 2024 •

edited

Loading

MadLittleMods Sep 10, 2024

MadLittleMods Sep 10, 2024

MadLittleMods Sep 10, 2024 •

edited

Loading

MadLittleMods Sep 10, 2024

MadLittleMods Sep 10, 2024

MadLittleMods commented Sep 11, 2024

Sliding Sync: Make sure we get up-to-date information from get_sliding_sync_rooms_for_user(...) #17692

Sliding Sync: Make sure we get up-to-date information from get_sliding_sync_rooms_for_user(...) #17692

Conversation

MadLittleMods commented Sep 10, 2024 • edited Loading

TODO

Pull Request Checklist

MadLittleMods Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

MadLittleMods Sep 10, 2024

Choose a reason for hiding this comment

MadLittleMods Sep 10, 2024

Choose a reason for hiding this comment

MadLittleMods Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

MadLittleMods Sep 10, 2024

Choose a reason for hiding this comment

MadLittleMods Sep 10, 2024

Choose a reason for hiding this comment

MadLittleMods commented Sep 11, 2024

Sliding Sync: Make sure we get up-to-date information from `get_sliding_sync_rooms_for_user(...)` #17692

Sliding Sync: Make sure we get up-to-date information from `get_sliding_sync_rooms_for_user(...)` #17692

MadLittleMods commented Sep 10, 2024 •

edited

Loading

MadLittleMods Sep 10, 2024 •

edited

Loading

MadLittleMods Sep 10, 2024 •

edited

Loading