Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speed up room keys query by using read/write lock #17461

Merged
merged 3 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/17461.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Speed up fetching room keys from backup.
18 changes: 9 additions & 9 deletions synapse/handlers/e2e_room_keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
from synapse.logging.opentracing import log_kv, trace
from synapse.storage.databases.main.e2e_room_keys import RoomKey
from synapse.types import JsonDict
from synapse.util.async_helpers import Linearizer
from synapse.util.async_helpers import ReadWriteLock

if TYPE_CHECKING:
from synapse.server import HomeServer
Expand All @@ -58,7 +58,7 @@ def __init__(self, hs: "HomeServer"):
# clients belonging to a user will receive and try to upload a new session at
# roughly the same time. Also used to lock out uploads when the key is being
# changed.
self._upload_linearizer = Linearizer("upload_room_keys_lock")
self._upload_lock = ReadWriteLock()

@trace
async def get_room_keys(
Expand Down Expand Up @@ -89,7 +89,7 @@ async def get_room_keys(

# we deliberately take the lock to get keys so that changing the version
# works atomically
async with self._upload_linearizer.queue(user_id):
async with self._upload_lock.read(user_id):
# make sure the backup version exists
try:
await self.store.get_e2e_room_keys_version_info(user_id, version)
Expand Down Expand Up @@ -132,7 +132,7 @@ async def delete_room_keys(
"""

# lock for consistency with uploading
async with self._upload_linearizer.queue(user_id):
async with self._upload_lock.write(user_id):
# make sure the backup version exists
try:
version_info = await self.store.get_e2e_room_keys_version_info(
Expand Down Expand Up @@ -193,7 +193,7 @@ async def upload_room_keys(
# TODO: Validate the JSON to make sure it has the right keys.

# XXX: perhaps we should use a finer grained lock here?
Copy link
Member

Choose a reason for hiding this comment

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

Is this gripe still valid? How can we go more finer-grained than the user_id here?

If not, let's delete this comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is referring to locking by version or maybe device? Not sure TBH

async with self._upload_linearizer.queue(user_id):
async with self._upload_lock.write(user_id):
# Check that the version we're trying to upload is the current version
try:
version_info = await self.store.get_e2e_room_keys_version_info(user_id)
Expand Down Expand Up @@ -355,7 +355,7 @@ async def create_version(self, user_id: str, version_info: JsonDict) -> str:
# TODO: Validate the JSON to make sure it has the right keys.

# lock everyone out until we've switched version
async with self._upload_linearizer.queue(user_id):
async with self._upload_lock.write(user_id):
new_version = await self.store.create_e2e_room_keys_version(
user_id, version_info
)
Expand All @@ -382,7 +382,7 @@ async def get_version_info(
}
"""

async with self._upload_linearizer.queue(user_id):
async with self._upload_lock.read(user_id):
try:
res = await self.store.get_e2e_room_keys_version_info(user_id, version)
except StoreError as e:
Expand All @@ -407,7 +407,7 @@ async def delete_version(self, user_id: str, version: Optional[str] = None) -> N
NotFoundError: if this backup version doesn't exist
"""

async with self._upload_linearizer.queue(user_id):
async with self._upload_lock.write(user_id):
try:
await self.store.delete_e2e_room_keys_version(user_id, version)
except StoreError as e:
Expand Down Expand Up @@ -437,7 +437,7 @@ async def update_version(
raise SynapseError(
400, "Version in body does not match", Codes.INVALID_PARAM
)
async with self._upload_linearizer.queue(user_id):
async with self._upload_lock.write(user_id):
try:
old_info = await self.store.get_e2e_room_keys_version_info(
user_id, version
Expand Down
Loading