From 47e9912c00ef94011552b09d72f5ad7ea21e09ab Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 11 Nov 2020 16:07:21 +0000 Subject: [PATCH 1/8] Make this line debug (it's noisy) --- synapse/handlers/appservice.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/appservice.py b/synapse/handlers/appservice.py index 9fc844422862..5c6458eb52fc 100644 --- a/synapse/handlers/appservice.py +++ b/synapse/handlers/appservice.py @@ -226,7 +226,7 @@ async def _notify_interested_services_ephemeral( new_token: Optional[int], users: Collection[Union[str, UserID]], ): - logger.info("Checking interested services for %s" % (stream_key)) + logger.debug("Checking interested services for %s" % (stream_key)) with Measure(self.clock, "notify_interested_services_ephemeral"): for service in services: # Only handle typing if we have the latest token From 437c92731f2706ade35297cb0631981b371e4eb3 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 11 Nov 2020 16:07:33 +0000 Subject: [PATCH 2/8] Don't include from_key for presence if we are at 0 --- synapse/handlers/appservice.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/synapse/handlers/appservice.py b/synapse/handlers/appservice.py index 5c6458eb52fc..ea41b3330fa5 100644 --- a/synapse/handlers/appservice.py +++ b/synapse/handlers/appservice.py @@ -282,6 +282,12 @@ async def _handle_presence( from_key = await self.store.get_type_stream_id_for_appservice( service, "presence" ) + + if from_key == 0: + # We don't want to fetch the changes if this is the first time the appservice + # has checked for presence, because we will see all changes for all users + from_key = None + for user in users: if isinstance(user, str): user = UserID.from_string(user) From 51cab3b2945003e4062d89a22fd8391b53582a6b Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 11 Nov 2020 16:07:54 +0000 Subject: [PATCH 3/8] Limit read receipts for all rooms to 100 --- synapse/storage/databases/main/receipts.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/synapse/storage/databases/main/receipts.py b/synapse/storage/databases/main/receipts.py index ca7917c9895b..9ee7e46c22e3 100644 --- a/synapse/storage/databases/main/receipts.py +++ b/synapse/storage/databases/main/receipts.py @@ -294,12 +294,16 @@ def f(txn): sql = """ SELECT * FROM receipts_linearized WHERE stream_id > ? AND stream_id <= ? + ORDER BY stream_id DESC + LIMIT 100 """ txn.execute(sql, [from_key, to_key]) else: sql = """ SELECT * FROM receipts_linearized WHERE stream_id <= ? + ORDER BY stream_id DESC + LIMIT 100 """ txn.execute(sql, [to_key]) From 9a34eca1d16403cfa5e9f00edf5ee5c2cf049bee Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 11 Nov 2020 16:17:10 +0000 Subject: [PATCH 4/8] changelog.d/8744.bugfix --- changelog.d/8744.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/8744.bugfix diff --git a/changelog.d/8744.bugfix b/changelog.d/8744.bugfix new file mode 100644 index 000000000000..6b02441f9a54 --- /dev/null +++ b/changelog.d/8744.bugfix @@ -0,0 +1 @@ +Fix a bug where appservices may be sent an excessive amount of read receipts and presence. \ No newline at end of file From 96e16529c900df07be96da115dcb95b1762f1fe8 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 11 Nov 2020 16:57:43 +0000 Subject: [PATCH 5/8] Allow from_key to be None --- synapse/handlers/appservice.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/appservice.py b/synapse/handlers/appservice.py index ea41b3330fa5..89241aa66311 100644 --- a/synapse/handlers/appservice.py +++ b/synapse/handlers/appservice.py @@ -281,7 +281,7 @@ async def _handle_presence( presence_source = self.event_sources.sources["presence"] from_key = await self.store.get_type_stream_id_for_appservice( service, "presence" - ) + ) # type: Optional[int] if from_key == 0: # We don't want to fetch the changes if this is the first time the appservice From 7b399994e87fe1608c8f69563322b1a8345d03a4 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 18 Nov 2020 12:23:22 +0000 Subject: [PATCH 6/8] Update 8744.bugfix --- changelog.d/8744.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/8744.bugfix b/changelog.d/8744.bugfix index 6b02441f9a54..f8f9630bd6af 100644 --- a/changelog.d/8744.bugfix +++ b/changelog.d/8744.bugfix @@ -1 +1 @@ -Fix a bug where appservices may be sent an excessive amount of read receipts and presence. \ No newline at end of file +Fix a bug where appservices may be sent an excessive amount of read receipts and presence. Broke in v1.22.0. From ef4acf2a50c6d6fe19fee09ad220207724bbd4f0 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 18 Nov 2020 12:25:45 +0000 Subject: [PATCH 7/8] The from_key is superflous --- synapse/handlers/appservice.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/synapse/handlers/appservice.py b/synapse/handlers/appservice.py index 89241aa66311..5c6458eb52fc 100644 --- a/synapse/handlers/appservice.py +++ b/synapse/handlers/appservice.py @@ -281,13 +281,7 @@ async def _handle_presence( presence_source = self.event_sources.sources["presence"] from_key = await self.store.get_type_stream_id_for_appservice( service, "presence" - ) # type: Optional[int] - - if from_key == 0: - # We don't want to fetch the changes if this is the first time the appservice - # has checked for presence, because we will see all changes for all users - from_key = None - + ) for user in users: if isinstance(user, str): user = UserID.from_string(user) From 72c6165693d9c635bc8b158a7dc3592a3bd92789 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 18 Nov 2020 12:48:06 +0000 Subject: [PATCH 8/8] Update comment --- synapse/handlers/receipts.py | 3 ++- synapse/storage/databases/main/receipts.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/receipts.py b/synapse/handlers/receipts.py index c242c409cf26..153cbae7b912 100644 --- a/synapse/handlers/receipts.py +++ b/synapse/handlers/receipts.py @@ -158,7 +158,8 @@ async def get_new_events_as( if from_key == to_key: return [], to_key - # We first need to fetch all new receipts + # Fetch all read receipts for all rooms, up to a limit of 100. This is ordered + # by most recent. rooms_to_events = await self.store.get_linearized_receipts_for_all_rooms( from_key=from_key, to_key=to_key ) diff --git a/synapse/storage/databases/main/receipts.py b/synapse/storage/databases/main/receipts.py index 9ee7e46c22e3..1e7949a3233a 100644 --- a/synapse/storage/databases/main/receipts.py +++ b/synapse/storage/databases/main/receipts.py @@ -278,7 +278,8 @@ def f(txn): async def get_linearized_receipts_for_all_rooms( self, to_key: int, from_key: Optional[int] = None ) -> Dict[str, JsonDict]: - """Get receipts for all rooms between two stream_ids. + """Get receipts for all rooms between two stream_ids, up + to a limit of the latest 100 read receipts. Args: to_key: Max stream id to fetch receipts upto.