Skip to content

Commit

Permalink
Add a cache on get_rooms_for_local_user_where_membership_is (#17460)
Browse files Browse the repository at this point in the history
As it gets used in sliding sync.

We basically invalidate it in all the same places as
`get_rooms_for_user`. Most of the changes are due to needing the
arguments you pass in to be hashable (which lists aren't)
  • Loading branch information
erikjohnston authored Jul 19, 2024
1 parent 43c865f commit d3f9afd
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 4 deletions.
1 change: 1 addition & 0 deletions changelog.d/17460.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add cache to `get_rooms_for_local_user_where_membership_is` to speed up sliding sync.
2 changes: 1 addition & 1 deletion synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class Membership:
KNOCK: Final = "knock"
LEAVE: Final = "leave"
BAN: Final = "ban"
LIST: Final = {INVITE, JOIN, KNOCK, LEAVE, BAN}
LIST: Final = frozenset((INVITE, JOIN, KNOCK, LEAVE, BAN))


class PresenceState:
Expand Down
6 changes: 6 additions & 0 deletions synapse/storage/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ def _invalidate_state_caches(
"get_user_in_room_with_profile", (room_id, user_id)
)
self._attempt_to_invalidate_cache("get_rooms_for_user", (user_id,))
self._attempt_to_invalidate_cache(
"_get_rooms_for_local_user_where_membership_is_inner", (user_id,)
)

# Purge other caches based on room state.
self._attempt_to_invalidate_cache("get_room_summary", (room_id,))
Expand All @@ -146,6 +149,9 @@ def _invalidate_state_caches_all(self, room_id: str) -> None:
self._attempt_to_invalidate_cache("does_pair_of_users_share_a_room", None)
self._attempt_to_invalidate_cache("get_user_in_room_with_profile", None)
self._attempt_to_invalidate_cache("get_rooms_for_user", None)
self._attempt_to_invalidate_cache(
"_get_rooms_for_local_user_where_membership_is_inner", None
)
self._attempt_to_invalidate_cache("get_room_summary", (room_id,))

def _attempt_to_invalidate_cache(
Expand Down
6 changes: 6 additions & 0 deletions synapse/storage/databases/main/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,9 @@ def _invalidate_caches_for_event(
"get_invited_rooms_for_local_user", (state_key,)
)
self._attempt_to_invalidate_cache("get_rooms_for_user", (state_key,))
self._attempt_to_invalidate_cache(
"_get_rooms_for_local_user_where_membership_is_inner", (state_key,)
)

self._attempt_to_invalidate_cache(
"did_forget",
Expand Down Expand Up @@ -393,6 +396,9 @@ def _invalidate_caches_for_room_events(self, room_id: str) -> None:
self._attempt_to_invalidate_cache("get_thread_id_for_receipts", None)
self._attempt_to_invalidate_cache("get_invited_rooms_for_local_user", None)
self._attempt_to_invalidate_cache("get_rooms_for_user", None)
self._attempt_to_invalidate_cache(
"_get_rooms_for_local_user_where_membership_is_inner", None
)
self._attempt_to_invalidate_cache("did_forget", None)
self._attempt_to_invalidate_cache("get_forgotten_rooms_for_user", None)
self._attempt_to_invalidate_cache("get_references_for_event", None)
Expand Down
26 changes: 23 additions & 3 deletions synapse/storage/databases/main/roommember.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,9 +445,11 @@ async def get_rooms_for_local_user_where_membership_is(
if not membership_list:
return []

rooms = await self.db_pool.runInteraction(
"get_rooms_for_local_user_where_membership_is",
self._get_rooms_for_local_user_where_membership_is_txn,
# Convert membership list to frozen set as a) it needs to be hashable,
# and b) we don't care about the order.
membership_list = frozenset(membership_list)

rooms = await self._get_rooms_for_local_user_where_membership_is_inner(
user_id,
membership_list,
)
Expand All @@ -466,6 +468,24 @@ async def get_rooms_for_local_user_where_membership_is(

return [room for room in rooms if room.room_id not in rooms_to_exclude]

@cached(max_entries=1000, tree=True)
async def _get_rooms_for_local_user_where_membership_is_inner(
self,
user_id: str,
membership_list: Collection[str],
) -> Sequence[RoomsForUser]:
if not membership_list:
return []

rooms = await self.db_pool.runInteraction(
"get_rooms_for_local_user_where_membership_is",
self._get_rooms_for_local_user_where_membership_is_txn,
user_id,
membership_list,
)

return rooms

def _get_rooms_for_local_user_where_membership_is_txn(
self,
txn: LoggingTransaction,
Expand Down
1 change: 1 addition & 0 deletions tests/handlers/test_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ def test_unknown_room_version(self) -> None:

# Blow away caches (supported room versions can only change due to a restart).
self.store.get_rooms_for_user.invalidate_all()
self.store._get_rooms_for_local_user_where_membership_is_inner.invalidate_all()
self.store._get_event_cache.clear()
self.store._event_ref.clear()

Expand Down

0 comments on commit d3f9afd

Please sign in to comment.