From 3e803be3d1756207e572145388f12e115105fc6e Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 26 May 2022 14:14:31 +0100 Subject: [PATCH 1/6] Reduce DB load of /sync when using presence While the query was fast, we were calling it *a lot*. --- synapse/storage/databases/main/presence.py | 70 +++++++++++++--------- 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/synapse/storage/databases/main/presence.py b/synapse/storage/databases/main/presence.py index b47c511450b9..1a66bd60b4f6 100644 --- a/synapse/storage/databases/main/presence.py +++ b/synapse/storage/databases/main/presence.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import TYPE_CHECKING, Any, Dict, Iterable, List, Tuple, cast +from typing import TYPE_CHECKING, Any, Dict, Iterable, List, Optional, Tuple, cast from synapse.api.presence import PresenceState, UserPresenceState from synapse.replication.tcp.streams import PresenceStream @@ -22,6 +22,7 @@ LoggingDatabaseConnection, LoggingTransaction, ) +from synapse.storage.databases.main.cache import CacheInvalidationWorkerStore from synapse.storage.engines import PostgresEngine from synapse.storage.types import Connection from synapse.storage.util.id_generators import ( @@ -56,7 +57,7 @@ def __init__( ) -class PresenceStore(PresenceBackgroundUpdateStore): +class PresenceStore(PresenceBackgroundUpdateStore, CacheInvalidationWorkerStore): def __init__( self, database: DatabasePool, @@ -281,20 +282,25 @@ async def should_user_receive_full_presence_with_token( True if the user should have full presence sent to them, False otherwise. """ - def _should_user_receive_full_presence_with_token_txn( - txn: LoggingTransaction, - ) -> bool: - sql = """ - SELECT 1 FROM users_to_send_full_presence_to - WHERE user_id = ? - AND presence_stream_id >= ? - """ - txn.execute(sql, (user_id, from_token)) - return bool(txn.fetchone()) + token = await self._get_when_user_should_receive_full_presence(user_id) + if token is None: + return False - return await self.db_pool.runInteraction( - "should_user_receive_full_presence_with_token", - _should_user_receive_full_presence_with_token_txn, + return from_token <= token + + @cached() + async def _get_when_user_should_receive_full_presence( + self, user_id: str + ) -> Optional[int]: + """Return the presence stream token, if any, which should trigger the + user to receive full presence. + """ + return await self.db_pool.simple_select_one_onecol( + table="users_to_send_full_presence_to", + keyvalues={"user_id": user_id}, + retcol="presence_stream_id", + allow_none=True, + desc="_get_when_user_should_receive_full_presence", ) async def add_users_to_send_full_presence_to(self, user_ids: Iterable[str]) -> None: @@ -307,18 +313,28 @@ async def add_users_to_send_full_presence_to(self, user_ids: Iterable[str]) -> N # Add user entries to the table, updating the presence_stream_id column if the user already # exists in the table. presence_stream_id = self._presence_id_gen.get_current_token() - await self.db_pool.simple_upsert_many( - table="users_to_send_full_presence_to", - key_names=("user_id",), - key_values=[(user_id,) for user_id in user_ids], - value_names=("presence_stream_id",), - # We save the current presence stream ID token along with the user ID entry so - # that when a user /sync's, even if they syncing multiple times across separate - # devices at different times, each device will receive full presence once - when - # the presence stream ID in their sync token is less than the one in the table - # for their user ID. - value_values=[(presence_stream_id,) for _ in user_ids], - desc="add_users_to_send_full_presence_to", + + def _add_users_to_send_full_presence_to(txn: LoggingTransaction) -> None: + self.db_pool.simple_upsert_many_txn( + txn, + table="users_to_send_full_presence_to", + key_names=("user_id",), + key_values=[(user_id,) for user_id in user_ids], + value_names=("presence_stream_id",), + # We save the current presence stream ID token along with the user ID entry so + # that when a user /sync's, even if they syncing multiple times across separate + # devices at different times, each device will receive full presence once - when + # the presence stream ID in their sync token is less than the one in the table + # for their user ID. + value_values=[(presence_stream_id,) for _ in user_ids], + ) + for user_id in user_ids: + self._invalidate_cache_and_stream( + txn, self._get_when_user_should_receive_full_presence, (user_id,) + ) + + return await self.db_pool.runInteraction( + "add_users_to_send_full_presence_to", _add_users_to_send_full_presence_to ) async def get_presence_for_all_users( From ed85af0fb2a59e47c65ddc7dab1349165c4707ba Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 26 May 2022 15:03:00 +0100 Subject: [PATCH 2/6] Newsfile --- changelog.d/12885.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12885.misc diff --git a/changelog.d/12885.misc b/changelog.d/12885.misc new file mode 100644 index 000000000000..252405630714 --- /dev/null +++ b/changelog.d/12885.misc @@ -0,0 +1 @@ +Reduce database load of `/sync` when presence is enabled. From d5019b6fd4e990e9b82e24b027eff607f5d652ce Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 31 May 2022 12:50:31 +0100 Subject: [PATCH 3/6] Update synapse/storage/databases/main/presence.py Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/storage/databases/main/presence.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/presence.py b/synapse/storage/databases/main/presence.py index 1a66bd60b4f6..eafb82b323a9 100644 --- a/synapse/storage/databases/main/presence.py +++ b/synapse/storage/databases/main/presence.py @@ -292,8 +292,12 @@ async def should_user_receive_full_presence_with_token( async def _get_when_user_should_receive_full_presence( self, user_id: str ) -> Optional[int]: - """Return the presence stream token, if any, which should trigger the - user to receive full presence. + """Get the presence token corresponding to the last full presence update for this user. + + If the user presents a sync token with a presence stream token at least as old as the result, + then we need to send them a full presence update. + + If this user has never needed a full presence update, returns `None`. """ return await self.db_pool.simple_select_one_onecol( table="users_to_send_full_presence_to", From 98d69c80e61d49134a86d0c95ca6c0e5ff94c14a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 31 May 2022 12:51:23 +0100 Subject: [PATCH 4/6] Fixup comment --- synapse/storage/databases/main/presence.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/synapse/storage/databases/main/presence.py b/synapse/storage/databases/main/presence.py index eafb82b323a9..051478d53e20 100644 --- a/synapse/storage/databases/main/presence.py +++ b/synapse/storage/databases/main/presence.py @@ -292,11 +292,12 @@ async def should_user_receive_full_presence_with_token( async def _get_when_user_should_receive_full_presence( self, user_id: str ) -> Optional[int]: - """Get the presence token corresponding to the last full presence update for this user. - - If the user presents a sync token with a presence stream token at least as old as the result, - then we need to send them a full presence update. - + """Get the presence token corresponding to the last full presence update + for this user. + + If the user presents a sync token with a presence stream token at least + as old as the result, then we need to send them a full presence update. + If this user has never needed a full presence update, returns `None`. """ return await self.db_pool.simple_select_one_onecol( From 56c5e0b6d84d414e6b7bf8a62e3ab25c571729e0 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 31 May 2022 12:51:30 +0100 Subject: [PATCH 5/6] Rename func --- synapse/storage/databases/main/presence.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/storage/databases/main/presence.py b/synapse/storage/databases/main/presence.py index 051478d53e20..b76bdcba18c2 100644 --- a/synapse/storage/databases/main/presence.py +++ b/synapse/storage/databases/main/presence.py @@ -282,14 +282,14 @@ async def should_user_receive_full_presence_with_token( True if the user should have full presence sent to them, False otherwise. """ - token = await self._get_when_user_should_receive_full_presence(user_id) + token = await self._get_full_presence_stream_token_for_user(user_id) if token is None: return False return from_token <= token @cached() - async def _get_when_user_should_receive_full_presence( + async def _get_full_presence_stream_token_for_user( self, user_id: str ) -> Optional[int]: """Get the presence token corresponding to the last full presence update @@ -305,7 +305,7 @@ async def _get_when_user_should_receive_full_presence( keyvalues={"user_id": user_id}, retcol="presence_stream_id", allow_none=True, - desc="_get_when_user_should_receive_full_presence", + desc="_get_full_presence_stream_token_for_user", ) async def add_users_to_send_full_presence_to(self, user_ids: Iterable[str]) -> None: From 176a0682cbdb2bdde46b9e484894d7ed460a520a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 31 May 2022 13:14:30 +0100 Subject: [PATCH 6/6] Renake take2 --- synapse/storage/databases/main/presence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/presence.py b/synapse/storage/databases/main/presence.py index b76bdcba18c2..9769a18a9d0c 100644 --- a/synapse/storage/databases/main/presence.py +++ b/synapse/storage/databases/main/presence.py @@ -335,7 +335,7 @@ def _add_users_to_send_full_presence_to(txn: LoggingTransaction) -> None: ) for user_id in user_ids: self._invalidate_cache_and_stream( - txn, self._get_when_user_should_receive_full_presence, (user_id,) + txn, self._get_full_presence_stream_token_for_user, (user_id,) ) return await self.db_pool.runInteraction(