From 0f0c9fa58e71e5f058eca866af05e88da649f863 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Jun 2022 09:39:03 -0400 Subject: [PATCH 1/7] Do not create a stream generator for groups. --- synapse/storage/databases/main/__init__.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/synapse/storage/databases/main/__init__.py b/synapse/storage/databases/main/__init__.py index d545a1c002c9..11d9d16c1900 100644 --- a/synapse/storage/databases/main/__init__.py +++ b/synapse/storage/databases/main/__init__.py @@ -151,10 +151,6 @@ def __init__( ], ) - self._group_updates_id_gen = StreamIdGenerator( - db_conn, "local_group_updates", "stream_id" - ) - self._cache_id_gen: Optional[MultiWriterIdGenerator] if isinstance(self.database_engine, PostgresEngine): # We set the `writers` to an empty list here as we don't care about @@ -197,20 +193,6 @@ def __init__( prefilled_cache=curr_state_delta_prefill, ) - _group_updates_prefill, min_group_updates_id = self.db_pool.get_cache_dict( - db_conn, - "local_group_updates", - entity_column="user_id", - stream_column="stream_id", - max_value=self._group_updates_id_gen.get_current_token(), - limit=1000, - ) - self._group_updates_stream_cache = StreamChangeCache( - "_group_updates_stream_cache", - min_group_updates_id, - prefilled_cache=_group_updates_prefill, - ) - self._stream_order_on_start = self.get_room_max_stream_ordering() self._min_stream_order_on_start = self.get_room_min_stream_ordering() From 4e45054a4a467bd69701f1cf1c98a28f74064242 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Jun 2022 09:44:57 -0400 Subject: [PATCH 2/7] Remove unused adding/getting from remote profile cache. --- synapse/handlers/profile.py | 24 ------------------ synapse/storage/databases/main/profile.py | 30 +---------------------- 2 files changed, 1 insertion(+), 53 deletions(-) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index 239b0aa7447a..1edfaff5be90 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -116,30 +116,6 @@ async def get_profile(self, user_id: str) -> JsonDict: raise SynapseError(502, "Failed to fetch profile") raise e.to_synapse_error() - async def get_profile_from_cache(self, user_id: str) -> JsonDict: - """Get the profile information from our local cache. If the user is - ours then the profile information will always be correct. Otherwise, - it may be out of date/missing. - """ - target_user = UserID.from_string(user_id) - if self.hs.is_mine(target_user): - try: - displayname = await self.store.get_profile_displayname( - target_user.localpart - ) - avatar_url = await self.store.get_profile_avatar_url( - target_user.localpart - ) - except StoreError as e: - if e.code == 404: - raise SynapseError(404, "Profile was not found", Codes.NOT_FOUND) - raise - - return {"displayname": displayname, "avatar_url": avatar_url} - else: - profile = await self.store.get_from_remote_profile_cache(user_id) - return profile or {} - async def get_displayname(self, target_user: UserID) -> Optional[str]: if self.hs.is_mine(target_user): try: diff --git a/synapse/storage/databases/main/profile.py b/synapse/storage/databases/main/profile.py index e197b7203e6a..2007901a941d 100644 --- a/synapse/storage/databases/main/profile.py +++ b/synapse/storage/databases/main/profile.py @@ -55,17 +55,6 @@ async def get_profile_avatar_url(self, user_localpart: str) -> Optional[str]: desc="get_profile_avatar_url", ) - async def get_from_remote_profile_cache( - self, user_id: str - ) -> Optional[Dict[str, Any]]: - return await self.db_pool.simple_select_one( - table="remote_profile_cache", - keyvalues={"user_id": user_id}, - retcols=("displayname", "avatar_url"), - allow_none=True, - desc="get_from_remote_profile_cache", - ) - async def create_profile(self, user_localpart: str) -> None: await self.db_pool.simple_insert( table="profiles", values={"user_id": user_localpart}, desc="create_profile" @@ -167,21 +156,4 @@ def _get_remote_profile_cache_entries_that_expire_txn( class ProfileStore(ProfileWorkerStore): - async def add_remote_profile_cache( - self, user_id: str, displayname: str, avatar_url: str - ) -> None: - """Ensure we are caching the remote user's profiles. - - This should only be called when `is_subscribed_remote_profile_for_user` - would return true for the user. - """ - await self.db_pool.simple_upsert( - table="remote_profile_cache", - keyvalues={"user_id": user_id}, - values={ - "displayname": displayname, - "avatar_url": avatar_url, - "last_check": self._clock.time_msec(), - }, - desc="add_remote_profile_cache", - ) + pass From 65f0ed3fb9e160eb8841934a9ff59d3fc25c411e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Jun 2022 09:54:49 -0400 Subject: [PATCH 3/7] Remove the remote_profile_cache only used for groups/communities. --- synapse/_scripts/synapse_port_db.py | 1 + synapse/handlers/profile.py | 59 +---------------- synapse/storage/databases/main/profile.py | 77 +---------------------- 3 files changed, 3 insertions(+), 134 deletions(-) diff --git a/synapse/_scripts/synapse_port_db.py b/synapse/_scripts/synapse_port_db.py index 4939573f3031..361b51d2fa2d 100755 --- a/synapse/_scripts/synapse_port_db.py +++ b/synapse/_scripts/synapse_port_db.py @@ -182,6 +182,7 @@ "groups", "local_group_membership", "local_group_updates", + "remote_profile_cache", } diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index 1edfaff5be90..6eed3826a776 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -23,14 +23,7 @@ StoreError, SynapseError, ) -from synapse.metrics.background_process_metrics import wrap_as_background_process -from synapse.types import ( - JsonDict, - Requester, - UserID, - create_requester, - get_domain_from_id, -) +from synapse.types import JsonDict, Requester, UserID, create_requester from synapse.util.caches.descriptors import cached from synapse.util.stringutils import parse_and_validate_mxc_uri @@ -50,9 +43,6 @@ class ProfileHandler: delegate to master when necessary. """ - PROFILE_UPDATE_MS = 60 * 1000 - PROFILE_UPDATE_EVERY_MS = 24 * 60 * 60 * 1000 - def __init__(self, hs: "HomeServer"): self.store = hs.get_datastores().main self.clock = hs.get_clock() @@ -73,11 +63,6 @@ def __init__(self, hs: "HomeServer"): self._third_party_rules = hs.get_third_party_event_rules() - if hs.config.worker.run_background_tasks: - self.clock.looping_call( - self._update_remote_profile_cache, self.PROFILE_UPDATE_MS - ) - async def get_profile(self, user_id: str) -> JsonDict: target_user = UserID.from_string(user_id) @@ -485,45 +470,3 @@ async def check_profile_query_allowed( # so we act as if we couldn't find the profile. raise SynapseError(403, "Profile isn't available", Codes.FORBIDDEN) raise - - @wrap_as_background_process("Update remote profile") - async def _update_remote_profile_cache(self) -> None: - """Called periodically to check profiles of remote users we haven't - checked in a while. - """ - entries = await self.store.get_remote_profile_cache_entries_that_expire( - last_checked=self.clock.time_msec() - self.PROFILE_UPDATE_EVERY_MS - ) - - for user_id, displayname, avatar_url in entries: - is_subscribed = await self.store.is_subscribed_remote_profile_for_user( - user_id - ) - if not is_subscribed: - await self.store.maybe_delete_remote_profile_cache(user_id) - continue - - try: - profile = await self.federation.make_query( - destination=get_domain_from_id(user_id), - query_type="profile", - args={"user_id": user_id}, - ignore_backoff=True, - ) - except Exception: - logger.exception("Failed to get avatar_url") - - await self.store.update_remote_profile_cache( - user_id, displayname, avatar_url - ) - continue - - new_name = profile.get("displayname") - if not isinstance(new_name, str): - new_name = None - new_avatar = profile.get("avatar_url") - if not isinstance(new_avatar, str): - new_avatar = None - - # We always hit update to update the last_check timestamp - await self.store.update_remote_profile_cache(user_id, new_name, new_avatar) diff --git a/synapse/storage/databases/main/profile.py b/synapse/storage/databases/main/profile.py index 2007901a941d..a1747f04ce72 100644 --- a/synapse/storage/databases/main/profile.py +++ b/synapse/storage/databases/main/profile.py @@ -11,11 +11,10 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from typing import Any, Dict, List, Optional +from typing import Optional from synapse.api.errors import StoreError from synapse.storage._base import SQLBaseStore -from synapse.storage.database import LoggingTransaction from synapse.storage.databases.main.roommember import ProfileInfo @@ -80,80 +79,6 @@ async def set_profile_avatar_url( desc="set_profile_avatar_url", ) - async def update_remote_profile_cache( - self, user_id: str, displayname: Optional[str], avatar_url: Optional[str] - ) -> int: - return await self.db_pool.simple_update( - table="remote_profile_cache", - keyvalues={"user_id": user_id}, - updatevalues={ - "displayname": displayname, - "avatar_url": avatar_url, - "last_check": self._clock.time_msec(), - }, - desc="update_remote_profile_cache", - ) - - async def maybe_delete_remote_profile_cache(self, user_id: str) -> None: - """Check if we still care about the remote user's profile, and if we - don't then remove their profile from the cache - """ - subscribed = await self.is_subscribed_remote_profile_for_user(user_id) - if not subscribed: - await self.db_pool.simple_delete( - table="remote_profile_cache", - keyvalues={"user_id": user_id}, - desc="delete_remote_profile_cache", - ) - - async def is_subscribed_remote_profile_for_user(self, user_id: str) -> bool: - """Check whether we are interested in a remote user's profile.""" - res: Optional[str] = await self.db_pool.simple_select_one_onecol( - table="group_users", - keyvalues={"user_id": user_id}, - retcol="user_id", - allow_none=True, - desc="should_update_remote_profile_cache_for_user", - ) - - if res: - return True - - res = await self.db_pool.simple_select_one_onecol( - table="group_invites", - keyvalues={"user_id": user_id}, - retcol="user_id", - allow_none=True, - desc="should_update_remote_profile_cache_for_user", - ) - - if res: - return True - return False - - async def get_remote_profile_cache_entries_that_expire( - self, last_checked: int - ) -> List[Dict[str, str]]: - """Get all users who haven't been checked since `last_checked`""" - - def _get_remote_profile_cache_entries_that_expire_txn( - txn: LoggingTransaction, - ) -> List[Dict[str, str]]: - sql = """ - SELECT user_id, displayname, avatar_url - FROM remote_profile_cache - WHERE last_check < ? - """ - - txn.execute(sql, (last_checked,)) - - return self.db_pool.cursor_to_dict(txn) - - return await self.db_pool.runInteraction( - "get_remote_profile_cache_entries_that_expire", - _get_remote_profile_cache_entries_that_expire_txn, - ) - class ProfileStore(ProfileWorkerStore): pass From d386c212c80576af4affa6edb39c84f1a5a43349 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Jun 2022 09:55:37 -0400 Subject: [PATCH 4/7] Remove groups tables from list of rooms to purge. --- synapse/storage/databases/main/purge_events.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/synapse/storage/databases/main/purge_events.py b/synapse/storage/databases/main/purge_events.py index 2353c120e9cc..ba385f9fc48d 100644 --- a/synapse/storage/databases/main/purge_events.py +++ b/synapse/storage/databases/main/purge_events.py @@ -393,7 +393,6 @@ def _purge_room_txn(self, txn: LoggingTransaction, room_id: str) -> List[int]: "partial_state_events", "events", "federation_inbound_events_staging", - "group_rooms", "local_current_membership", "partial_state_rooms_servers", "partial_state_rooms", @@ -413,7 +412,6 @@ def _purge_room_txn(self, txn: LoggingTransaction, room_id: str) -> List[int]: "e2e_room_keys", "event_push_summary", "pusher_throttle", - "group_summary_rooms", "room_account_data", "room_tags", # "rooms" happens last, to keep the foreign keys in the other tables From 6cca48fa93fe0735d553c3744ea1fab86f725650 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Jun 2022 09:56:49 -0400 Subject: [PATCH 5/7] Newsfragment --- changelog.d/12966.removal | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12966.removal diff --git a/changelog.d/12966.removal b/changelog.d/12966.removal new file mode 100644 index 000000000000..41f6fae5da91 --- /dev/null +++ b/changelog.d/12966.removal @@ -0,0 +1 @@ +Remove support for the non-standard groups/communities feature from Synapse. From 4c386515e68d0fb0f4b3d36efb951ffc60c3aff2 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Jun 2022 10:01:31 -0400 Subject: [PATCH 6/7] Note in schema version. --- synapse/storage/schema/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/storage/schema/__init__.py b/synapse/storage/schema/__init__.py index 19466150d4c6..5843fae6057d 100644 --- a/synapse/storage/schema/__init__.py +++ b/synapse/storage/schema/__init__.py @@ -70,6 +70,7 @@ Changes in SCHEMA_VERSION = 71: - event_edges.room_id is no longer read from. + - Tables related to groups are no longer accessed. """ From b8e14c123341cdb1cd7801263db6b16502163c36 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Jun 2022 10:49:00 -0400 Subject: [PATCH 7/7] Update tests to match code changes. --- tests/rest/admin/test_room.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 608d3f2dc36a..ca6af9417bdc 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -2467,7 +2467,6 @@ def _block_room(self, room_id: str) -> None: "event_push_actions", "event_search", "events", - "group_rooms", "receipts_graph", "receipts_linearized", "room_aliases", @@ -2484,7 +2483,6 @@ def _block_room(self, room_id: str) -> None: "e2e_room_keys", "event_push_summary", "pusher_throttle", - "group_summary_rooms", "room_account_data", "room_tags", # "state_groups", # Current impl leaves orphaned state groups around.