From 69018acbd2d1f331d6a52335b4938c3753b16de6 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Thu, 22 Apr 2021 16:53:24 +0100 Subject: [PATCH] Clear the resync bit after resyncing device lists (#9867) Fixes #9866. --- changelog.d/9867.bugfix | 1 + synapse/handlers/device.py | 7 +++++++ synapse/storage/databases/main/devices.py | 19 +++++++++---------- 3 files changed, 17 insertions(+), 10 deletions(-) create mode 100644 changelog.d/9867.bugfix diff --git a/changelog.d/9867.bugfix b/changelog.d/9867.bugfix new file mode 100644 index 0000000000..f236de247d --- /dev/null +++ b/changelog.d/9867.bugfix @@ -0,0 +1 @@ +Fix a bug which could cause Synapse to get stuck in a loop of resyncing device lists. diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 34d39e3b44..95bdc5902a 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -925,6 +925,10 @@ class DeviceListUpdater: else: cached_devices = await self.store.get_cached_devices_for_user(user_id) if cached_devices == {d["device_id"]: d for d in devices}: + logging.info( + "Skipping device list resync for %s, as our cache matches already", + user_id, + ) devices = [] ignore_devices = True @@ -940,6 +944,9 @@ class DeviceListUpdater: await self.store.update_remote_device_list_cache( user_id, devices, stream_id ) + # mark the cache as valid, whether or not we actually processed any device + # list updates. + await self.store.mark_remote_user_device_cache_as_valid(user_id) device_ids = [device["device_id"] for device in devices] # Handle cross-signing keys. diff --git a/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py index 9be713399f..c9346de316 100644 --- a/synapse/storage/databases/main/devices.py +++ b/synapse/storage/databases/main/devices.py @@ -717,7 +717,15 @@ class DeviceWorkerStore(SQLBaseStore): keyvalues={"user_id": user_id}, values={}, insertion_values={"added_ts": self._clock.time_msec()}, - desc="make_remote_user_device_cache_as_stale", + desc="mark_remote_user_device_cache_as_stale", + ) + + async def mark_remote_user_device_cache_as_valid(self, user_id: str) -> None: + # Remove the database entry that says we need to resync devices, after a resync + await self.db_pool.simple_delete( + table="device_lists_remote_resync", + keyvalues={"user_id": user_id}, + desc="mark_remote_user_device_cache_as_valid", ) async def mark_remote_user_device_list_as_unsubscribed(self, user_id: str) -> None: @@ -1289,15 +1297,6 @@ class DeviceStore(DeviceWorkerStore, DeviceBackgroundUpdateStore): lock=False, ) - # If we're replacing the remote user's device list cache presumably - # we've done a full resync, so we remove the entry that says we need - # to resync - self.db_pool.simple_delete_txn( - txn, - table="device_lists_remote_resync", - keyvalues={"user_id": user_id}, - ) - async def add_device_change_to_streams( self, user_id: str, device_ids: Collection[str], hosts: List[str] ):