From c2ad19cf025fa93670c1fa9f8ababd62d2026a37 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 22 Apr 2021 15:35:51 +0100 Subject: [PATCH 1/3] Clear the resync bit after resyncing device lists --- changelog.d/9867.bugfix | 1 + synapse/handlers/device.py | 6 ++++++ synapse/storage/databases/main/devices.py | 16 +++++++--------- 3 files changed, 14 insertions(+), 9 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 000000000000..f236de247d8b --- /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 c1d780098153..178ea6f8d94a 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -926,6 +926,9 @@ async def user_device_resync( 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." + ) devices = [] ignore_devices = True @@ -941,6 +944,9 @@ async def user_device_resync( 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 b20487558060..98872d2a2518 100644 --- a/synapse/storage/databases/main/devices.py +++ b/synapse/storage/databases/main/devices.py @@ -720,6 +720,13 @@ async def mark_remote_user_device_cache_as_stale(self, user_id: str) -> None: desc="make_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}, + ) + async def mark_remote_user_device_list_as_unsubscribed(self, user_id: str) -> None: """Mark that we no longer track device lists for remote user.""" @@ -1289,15 +1296,6 @@ def _update_remote_device_list_cache_txn( 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] ): From 8fdfb5214e5726f6c0079b79a7de69e7fdc8f9b4 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 22 Apr 2021 15:48:06 +0100 Subject: [PATCH 2/3] fix logging --- synapse/handlers/device.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 178ea6f8d94a..065d4735c79f 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -927,7 +927,8 @@ async def user_device_resync( 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." + "Skipping device list resync for %s, as our cache matches already", + user_id, ) devices = [] ignore_devices = True From 8b29f108c874f03912c08b84db4a1846c154e76d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 22 Apr 2021 15:51:14 +0100 Subject: [PATCH 3/3] add missing description --- synapse/storage/databases/main/devices.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py index 98872d2a2518..9d2677c7cadd 100644 --- a/synapse/storage/databases/main/devices.py +++ b/synapse/storage/databases/main/devices.py @@ -717,7 +717,7 @@ async def mark_remote_user_device_cache_as_stale(self, user_id: str) -> None: 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: @@ -725,6 +725,7 @@ async def mark_remote_user_device_cache_as_valid(self, user_id: str) -> None: 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: