Skip to content

Commit

Permalink
Only do one concurrent fetch per server in keyring (#16894)
Browse files Browse the repository at this point in the history
Otherwise if we've stacked a bunch of requests for the keys of a server,
we'll end up sending lots of concurrent requests for its keys,
needlessly.
  • Loading branch information
erikjohnston committed Feb 9, 2024
1 parent 7c805f0 commit bfa93d1
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 4 deletions.
1 change: 1 addition & 0 deletions changelog.d/16894.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Do not send multiple concurrent requests for keys for the same server.
9 changes: 5 additions & 4 deletions synapse/crypto/keyring.py
Original file line number Diff line number Diff line change
Expand Up @@ -839,11 +839,12 @@ async def _fetch_keys(
Map from server_name -> key_id -> FetchKeyResult
"""

results = {}
# We only need to do one request per server.
servers_to_fetch = {k.server_name for k in keys_to_fetch}

async def get_keys(key_to_fetch_item: _FetchKeyRequest) -> None:
server_name = key_to_fetch_item.server_name
results = {}

async def get_keys(server_name: str) -> None:
try:
keys = await self.get_server_verify_keys_v2_direct(server_name)
results[server_name] = keys
Expand All @@ -852,7 +853,7 @@ async def get_keys(key_to_fetch_item: _FetchKeyRequest) -> None:
except Exception:
logger.exception("Error getting keys from %s", server_name)

await yieldable_gather_results(get_keys, keys_to_fetch)
await yieldable_gather_results(get_keys, servers_to_fetch)
return results

async def get_server_verify_keys_v2_direct(
Expand Down

0 comments on commit bfa93d1

Please sign in to comment.