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

Batch up replication requests to request the resyncing of remote users's devices. #14716

Merged
merged 15 commits into from
Jan 10, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
13 changes: 11 additions & 2 deletions synapse/handlers/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -1212,9 +1212,18 @@ async def multi_user_device_resync(
raise InvalidAPICallError(f"Only one origin permitted, got {origins!r}")

result = {}
failed = set()
# TODO(Perf): Actually batch these up
for user_id in user_ids:
result[user_id] = await self.user_device_resync(user_id)
user_result, user_failed = await self._user_device_resync_returning_failed(
user_id
)
result[user_id] = user_result
if user_failed:
failed.add(user_id)

if mark_failed_as_stale:
await self.store.mark_remote_users_device_caches_as_stale(failed)

return result

Expand All @@ -1226,7 +1235,7 @@ async def user_device_resync(
if failed and mark_failed_as_stale:
# Mark the remote user's device list as stale so we know we need to retry
# it later.
await self.store.mark_remote_user_device_cache_as_stale(user_id)
await self.store.mark_remote_users_device_caches_as_stale((user_id,))

return result

Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/devicemessage.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ async def _check_for_unknown_devices(
sender_user_id,
unknown_devices,
)
await self.store.mark_remote_user_device_cache_as_stale(sender_user_id)
await self.store.mark_remote_users_device_caches_as_stale((sender_user_id,))

# Immediately attempt a resync in the background
run_in_background(self._user_device_resync, user_id=sender_user_id)
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/federation_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -1423,7 +1423,7 @@ async def _resync_device(self, sender: str) -> None:
"""

try:
await self._store.mark_remote_user_device_cache_as_stale(sender)
await self._store.mark_remote_users_device_caches_as_stale((sender,))

# Immediately attempt a resync in the background
if self._config.worker.worker_app:
Expand Down
30 changes: 22 additions & 8 deletions synapse/storage/databases/main/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
AbstractStreamIdTracker,
StreamIdGenerator,
)
from synapse.types import JsonDict, get_verify_key_from_cross_signing_key
from synapse.types import JsonDict, StrCollection, get_verify_key_from_cross_signing_key
from synapse.util import json_decoder, json_encoder
from synapse.util.caches.descriptors import cached, cachedList
from synapse.util.caches.lrucache import LruCache
Expand Down Expand Up @@ -1062,16 +1062,30 @@ async def get_user_ids_requiring_device_list_resync(

return {row["user_id"] for row in rows}

async def mark_remote_user_device_cache_as_stale(self, user_id: str) -> None:
async def mark_remote_users_device_caches_as_stale(
self, user_ids: StrCollection
) -> None:
"""Records that the server has reason to believe the cache of the devices
for the remote users is out of date.
"""
await self.db_pool.simple_upsert(
table="device_lists_remote_resync",
keyvalues={"user_id": user_id},
values={},
insertion_values={"added_ts": self._clock.time_msec()},
desc="mark_remote_user_device_cache_as_stale",

def _mark_remote_users_device_caches_as_stale_txn(
txn: LoggingTransaction,
) -> None:
# TODO add insertion_values support to simple_upsert_many and use
# that!
Comment on lines +1075 to +1076
Copy link
Member

Choose a reason for hiding this comment

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

Could likely replace this with raw SQL if we wanted.

Copy link
Member

Choose a reason for hiding this comment

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

(I think I wanted to use this somewhere in the receipts or event push actions code, but can't seem to find where.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm on the fence about raw SQL vs just leaving it like this.
It'd be interesting to have support for pipelined statements (c.f. https://www.psycopg.org/psycopg3/docs/advanced/pipeline.html) in the future. I do generally like the readability of our helpers...

for user_id in user_ids:
self.db_pool.simple_upsert_txn(
txn,
table="device_lists_remote_resync",
keyvalues={"user_id": user_id},
values={},
insertion_values={"added_ts": self._clock.time_msec()},
)

await self.db_pool.runInteraction(
"mark_remote_users_device_caches_as_stale",
_mark_remote_users_device_caches_as_stale_txn,
)

async def mark_remote_user_device_cache_as_valid(self, user_id: str) -> None:
Expand Down
4 changes: 4 additions & 0 deletions synapse/types/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@
# A JSON-serialisable object.
JsonSerializable = object

# Collection[str] that does not include str itself; str being a Sequence[str]
# is very misleading and results in bugs.
StrCollection = Union[Tuple[str, ...], List[str], Set[str]]
reivilibre marked this conversation as resolved.
Show resolved Hide resolved


# Note that this seems to require inheriting *directly* from Interface in order
# for mypy-zope to realize it is an interface.
Expand Down