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

Put a cache on /state_ids #7931

Merged
merged 1 commit into from
Jul 23, 2020
Merged

Put a cache on /state_ids #7931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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/7931.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Cache responses to `/_matrix/federation/v1/state_ids` to reduce duplicated work.
13 changes: 11 additions & 2 deletions synapse/federation/federation_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ def __init__(self, hs):
# We cache responses to state queries, as they take a while and often
# come in waves.
self._state_resp_cache = ResponseCache(hs, "state_resp", timeout_ms=30000)
self._state_ids_resp_cache = ResponseCache(
hs, "state_ids_resp", timeout_ms=30000
)

async def on_backfill_request(
self, origin: str, room_id: str, versions: List[str], limit: int
Expand Down Expand Up @@ -376,10 +379,16 @@ async def on_state_ids_request(
if not in_room:
raise AuthError(403, "Host not in room.")

resp = await self._state_ids_resp_cache.wrap(
(room_id, event_id), self._on_state_ids_request_compute, room_id, event_id,
)
Comment on lines +382 to +384
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The call above to self._state_resp_cache gets wrapped by dict(), which we don't do here. It doesn't seem necessary, but curious about the difference.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the dict there was added in https://github.com/matrix-org/synapse/pull/6176/files. It looks like it's so that we can add a room_version property after it comes out of the cache, without accidentally modifying the cached response.

It is unclear to me why that implementation was chosen instead of adding the room_id in _on_context_state_request_compute.

In any case, I think it's unnecessary in the new code.


return 200, resp

async def _on_state_ids_request_compute(self, room_id, event_id):
state_ids = await self.handler.get_state_ids_for_pdu(room_id, event_id)
auth_chain_ids = await self.store.get_auth_chain_ids(state_ids)

return 200, {"pdu_ids": state_ids, "auth_chain_ids": auth_chain_ids}
return {"pdu_ids": state_ids, "auth_chain_ids": auth_chain_ids}

async def _on_context_state_request_compute(
self, room_id: str, event_id: str
Expand Down