From 8830f229591d3b597d6b7ccca79c79674618978d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 5 Jan 2021 16:29:28 -0500 Subject: [PATCH 01/13] Denormalize the ignored users from account data to a separate table. This allows for efficiently finding which users ignore a particular user. --- changelog.d/9024.feature | 1 + synapse/push/bulk_push_rule_evaluator.py | 12 +- .../storage/databases/main/account_data.py | 119 +++++++++++++----- .../main/schema/delta/58/28ignored_user.py | 67 ++++++++++ 4 files changed, 166 insertions(+), 33 deletions(-) create mode 100644 changelog.d/9024.feature create mode 100644 synapse/storage/databases/main/schema/delta/58/28ignored_user.py diff --git a/changelog.d/9024.feature b/changelog.d/9024.feature new file mode 100644 index 000000000000..073dafbf834e --- /dev/null +++ b/changelog.d/9024.feature @@ -0,0 +1 @@ +Improved performance when calculating ignored users in large rooms. diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 10f27e4378d7..9018f9e20bd2 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -203,14 +203,18 @@ async def action_for_event_by_user( condition_cache = {} # type: Dict[str, bool] + # If the event is not a state event check if any users ignore the sender. + if not event.is_state(): + ignorers = await self.store.ignored_by(event.sender) + else: + ignorers = set() + for uid, rules in rules_by_user.items(): if event.sender == uid: continue - if not event.is_state(): - is_ignored = await self.store.is_ignored_by(event.sender, uid) - if is_ignored: - continue + if uid in ignorers: + continue display_name = None profile_info = room_members.get(uid) diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index 49ee23470d61..6c43b799cea3 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -16,7 +16,7 @@ import abc import logging -from typing import Dict, List, Optional, Tuple +from typing import Dict, List, Optional, Set, Tuple from synapse.api.constants import AccountDataTypes from synapse.storage._base import SQLBaseStore, db_to_json @@ -24,7 +24,7 @@ from synapse.storage.util.id_generators import StreamIdGenerator from synapse.types import JsonDict from synapse.util import json_encoder -from synapse.util.caches.descriptors import _CacheContext, cached +from synapse.util.caches.descriptors import cached from synapse.util.caches.stream_change_cache import StreamChangeCache logger = logging.getLogger(__name__) @@ -287,23 +287,24 @@ def get_updated_account_data_for_user_txn(txn): "get_updated_account_data_for_user", get_updated_account_data_for_user_txn ) - @cached(num_args=2, cache_context=True, max_entries=5000) - async def is_ignored_by( - self, ignored_user_id: str, ignorer_user_id: str, cache_context: _CacheContext - ) -> bool: - ignored_account_data = await self.get_global_account_data_by_type_for_user( - AccountDataTypes.IGNORED_USER_LIST, - ignorer_user_id, - on_invalidate=cache_context.invalidate, - ) - if not ignored_account_data: - return False + @cached(max_entries=5000) + async def ignored_by(self, user_id: str) -> Set[str]: + """ + Get users which ignore the given user. - try: - return ignored_user_id in ignored_account_data.get("ignored_users", {}) - except TypeError: - # The type of the ignored_users field is invalid. - return False + Params: + user_id: + + Return: + The user IDs which ignore the given user. + """ + return set( + await self.db_pool.simple_select_one_onecol( + table="ignored_users", + keyvalues={"ignored_user_id": user_id}, + retcol="ignorer_user_id", + ) + ) class AccountDataStore(AccountDataWorkerStore): @@ -390,18 +391,14 @@ async def add_account_data_for_user( Returns: The maximum stream ID. """ - content_json = json_encoder.encode(content) - async with self._account_data_id_gen.get_next() as next_id: - # no need to lock here as account_data has a unique constraint on - # (user_id, account_data_type) so simple_upsert will retry if - # there is a conflict. - await self.db_pool.simple_upsert( - desc="add_user_account_data", - table="account_data", - keyvalues={"user_id": user_id, "account_data_type": account_data_type}, - values={"stream_id": next_id, "content": content_json}, - lock=False, + await self.db_pool.runInteraction( + "add_user_account_data", + self._add_account_data_for_user, + next_id, + user_id, + account_data_type, + content, ) # it's theoretically possible for the above to succeed and the @@ -424,6 +421,70 @@ async def add_account_data_for_user( return self._account_data_id_gen.get_current_token() + def _add_account_data_for_user( + self, + txn, + next_id: int, + user_id: str, + account_data_type: str, + content: JsonDict, + ) -> None: + content_json = json_encoder.encode(content) + + # no need to lock here as account_data has a unique constraint on + # (user_id, account_data_type) so simple_upsert will retry if + # there is a conflict. + self.db_pool.simple_upsert_txn( + txn, + table="account_data", + keyvalues={"user_id": user_id, "account_data_type": account_data_type}, + values={"stream_id": next_id, "content": content_json}, + lock=False, + ) + + # Ignored users get denormalized into a separate table as an optimisation. + if account_data_type == AccountDataTypes.IGNORED_USER_LIST: + # Insert / delete to sync the list of ignored users. + previously_ignored_users = set( + self.db_pool.simple_select_onecol_txn( + txn, + table="ignored_users", + keyvalues={"ignorer_user_id": user_id}, + retcol="ignored_user_id", + ) + ) + + # If the data is invalid, no one is ignored. + ignored_users_content = content.get("ignored_users", {}) + if isinstance(ignored_users_content, dict): + currently_ignored_users = set(ignored_users_content) + else: + currently_ignored_users = set() + newly_ignored_users = currently_ignored_users - previously_ignored_users + + # Delete entries which are no longer ignored. + self.db_pool.simple_delete_many_txn( + txn, + table="ignored_users", + column="ignored_user_id", + iterable=previously_ignored_users - currently_ignored_users, + keyvalues={"ignorer_user_id": user_id}, + ) + + # Add entries which are newly ignored. + self.db_pool.simple_insert_many_txn( + txn, + table="ignored_users", + values=[ + {"ignorer_user_id": user_id, "ignored_user_id": u} + for u in newly_ignored_users + ], + ) + + # Invalidate the cache for newly ignored users. + for ignored_user_id in newly_ignored_users: + self.ignored_by.invalidate((ignored_user_id,)) + async def _update_max_stream_id(self, next_id: int) -> None: """Update the max stream_id diff --git a/synapse/storage/databases/main/schema/delta/58/28ignored_user.py b/synapse/storage/databases/main/schema/delta/58/28ignored_user.py new file mode 100644 index 000000000000..ad7537bd1fdc --- /dev/null +++ b/synapse/storage/databases/main/schema/delta/58/28ignored_user.py @@ -0,0 +1,67 @@ +# Copyright 2021 The Matrix.org Foundation C.I.C. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + +""" +This migration denormalises the account_data table into an ignored users table. +""" + +import logging +from io import StringIO + +from synapse.storage._base import db_to_json +from synapse.storage.engines import BaseDatabaseEngine +from synapse.storage.prepare_database import execute_statements_from_stream +from synapse.storage.types import Cursor + +logger = logging.getLogger(__name__) + + +def run_upgrade(cur: Cursor, database_engine: BaseDatabaseEngine, *args, **kwargs): + insert_sql = """ + INSERT INTO ignored_users (ignorer_user_id, ignored_user_id) VALUES (?, ?) + """ + + logger.info("Converting existing ignore lists") + cur.execute( + "SELECT user_id, content FROM account_data WHERE account_data_type = 'm.ignored_user_list'" + ) + for user_id, content_json in cur.fetchall(): + content = db_to_json(content_json) + + # The content should be the form of a dictionary with a key + # "ignored_users" pointing to a dictionary with keys of ignored users. + # + # { "ignored_users": "@someone:example.org": {} } + ignored_users = content.get("ignored_users", {}) + if isinstance(ignored_users, dict) and ignored_users: + cur.executemany(insert_sql, [(user_id, u) for u in ignored_users]) + + +def run_create(cur: Cursor, database_engine: BaseDatabaseEngine, *args, **kwargs): + logger.info("Creating ignored_users table") + execute_statements_from_stream(cur, StringIO(_create_commands)) + + +# there might be duplicates, so the easiest way to achieve this is to create a new +# table with the right data, and renaming it into place + +_create_commands = """ +-- Users which are ignored when calculating push notifications. This data is +-- denormalized from account data. +CREATE TABLE IF NOT EXISTS ignored_users( + ignorer_user_id TEXT NOT NULL, -- The user ID of the user who is ignoring another user. (This is a local user.) + ignored_user_id TEXT NOT NULL, -- The user ID of the user who is being ignored. (This is a local or remote user.) + UNIQUE (ignorer_user_id, ignored_user_id) +); +""" From 1e680b2e50eab5045d4b32f8c1f7143420fa6ceb Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 5 Jan 2021 16:52:40 -0500 Subject: [PATCH 02/13] Fixup a comment. --- synapse/storage/databases/main/account_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index 6c43b799cea3..65797f365871 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -293,7 +293,7 @@ async def ignored_by(self, user_id: str) -> Set[str]: Get users which ignore the given user. Params: - user_id: + user_id: The user ID which might be ignored. Return: The user IDs which ignore the given user. From c2d7087a13888fa09cddbce5fdf3925d4c513136 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 6 Jan 2021 07:52:31 -0500 Subject: [PATCH 03/13] Return all ignorers, not just the first. --- synapse/storage/databases/main/account_data.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index 65797f365871..fc3e48f0fd4f 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -299,10 +299,11 @@ async def ignored_by(self, user_id: str) -> Set[str]: The user IDs which ignore the given user. """ return set( - await self.db_pool.simple_select_one_onecol( + await self.db_pool.simple_select_onecol( table="ignored_users", keyvalues={"ignored_user_id": user_id}, retcol="ignorer_user_id", + desc="ignored_by", ) ) From 181db66e6b3759218a42fb03dbe7262bd1ceda9e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 6 Jan 2021 08:04:18 -0500 Subject: [PATCH 04/13] Fix invaliating caches. --- synapse/storage/databases/main/account_data.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index fc3e48f0fd4f..51f955e174c7 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -461,7 +461,6 @@ def _add_account_data_for_user( currently_ignored_users = set(ignored_users_content) else: currently_ignored_users = set() - newly_ignored_users = currently_ignored_users - previously_ignored_users # Delete entries which are no longer ignored. self.db_pool.simple_delete_many_txn( @@ -478,13 +477,15 @@ def _add_account_data_for_user( table="ignored_users", values=[ {"ignorer_user_id": user_id, "ignored_user_id": u} - for u in newly_ignored_users + for u in currently_ignored_users - previously_ignored_users ], ) - # Invalidate the cache for newly ignored users. - for ignored_user_id in newly_ignored_users: - self.ignored_by.invalidate((ignored_user_id,)) + # Invalidate the cache for any ignored users which were added or removed. + for ignored_user_id in previously_ignored_users ^ currently_ignored_users: + self._invalidate_cache_and_stream( + txn, self.ignored_by, (ignored_user_id,) + ) async def _update_max_stream_id(self, next_id: int) -> None: """Update the max stream_id From 6577e646cb7527ca292b2284c83f661e4532c8f5 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 6 Jan 2021 08:22:48 -0500 Subject: [PATCH 05/13] Add tests for ignoring users. --- tests/storage/test_account_data.py | 120 +++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 tests/storage/test_account_data.py diff --git a/tests/storage/test_account_data.py b/tests/storage/test_account_data.py new file mode 100644 index 000000000000..673e1fe3e339 --- /dev/null +++ b/tests/storage/test_account_data.py @@ -0,0 +1,120 @@ +# -*- coding: utf-8 -*- +# Copyright 2021 The Matrix.org Foundation C.I.C. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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 Iterable, Set + +from synapse.api.constants import AccountDataTypes + +from tests import unittest + + +class IgnoredUsersTestCase(unittest.HomeserverTestCase): + def prepare(self, hs, reactor, clock): + self.store = self.hs.get_datastore() + self.user = "@user:test" + + def _update_ignore_list( + self, *ignored_user_ids: Iterable[str], ignorer_user_id: str = None + ) -> None: + """Update the account data to block the given users.""" + if ignorer_user_id is None: + ignorer_user_id = self.user + + self.get_success( + self.store.add_account_data_for_user( + ignorer_user_id, + AccountDataTypes.IGNORED_USER_LIST, + {"ignored_users": {u: {} for u in ignored_user_ids}}, + ) + ) + + def assert_ignorers( + self, ignored_user_id: str, expected_ignorer_user_ids: Set[str] + ) -> None: + self.assertEqual( + self.get_success(self.store.ignored_by(ignored_user_id)), + expected_ignorer_user_ids, + ) + + def test_ignoring_users(self): + """Basic adding/removing of users from the ignore list.""" + self._update_ignore_list("@other:test", "@another:remote") + + # Check a user which no one ignores. + self.assert_ignorers("@user:test", set()) + + # Check a local user which is ignored. + self.assert_ignorers("@other:test", {self.user}) + + # Check a remote user which is ignored. + self.assert_ignorers("@another:remote", {self.user}) + + # Add one user, remove one user, and leave one user. + self._update_ignore_list("@foo:test", "@another:remote") + + # Check the removed user. + self.assert_ignorers("@other:test", set()) + + # Check the added user. + self.assert_ignorers("@foo:test", {self.user}) + + # Check the removed user. + self.assert_ignorers("@another:remote", {self.user}) + + def test_caching(self): + """Ensure that caching works properly between different users.""" + # The first user ignores a user. + self._update_ignore_list("@other:test") + self.assert_ignorers("@other:test", {self.user}) + + # The second user ignores them. + self._update_ignore_list("@other:test", ignorer_user_id="@second:test") + self.assert_ignorers("@other:test", {self.user, "@second:test"}) + + # The first user un-ignores them. + self._update_ignore_list() + self.assert_ignorers("@other:test", {"@second:test"}) + + def test_invalid_data(self): + """Invalid data ends up clearing out the ignored users list.""" + # Add some data and ensure it is there. + self._update_ignore_list("@other:test") + self.assert_ignorers("@other:test", {self.user}) + + # No ignored_users key. + self.get_success( + self.store.add_account_data_for_user( + self.user, AccountDataTypes.IGNORED_USER_LIST, {}, + ) + ) + + # No one ignores the user now. + self.assert_ignorers("@other:test", set()) + + # Add some data and ensure it is there. + self._update_ignore_list("@other:test") + self.assert_ignorers("@other:test", {self.user}) + + # Invalid data. + self.get_success( + self.store.add_account_data_for_user( + self.user, + AccountDataTypes.IGNORED_USER_LIST, + {"ignored_users": "unexpected"}, + ) + ) + + # No one ignores the user now. + self.assert_ignorers("@other:test", set()) From 179f37dad0136e8923bbc0728c536c07e2276ba9 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 6 Jan 2021 08:25:39 -0500 Subject: [PATCH 06/13] Return early for non-ignored users data. --- .../storage/databases/main/account_data.py | 72 ++++++++++--------- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index 51f955e174c7..1b0f3013b484 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -444,48 +444,50 @@ def _add_account_data_for_user( ) # Ignored users get denormalized into a separate table as an optimisation. - if account_data_type == AccountDataTypes.IGNORED_USER_LIST: - # Insert / delete to sync the list of ignored users. - previously_ignored_users = set( - self.db_pool.simple_select_onecol_txn( - txn, - table="ignored_users", - keyvalues={"ignorer_user_id": user_id}, - retcol="ignored_user_id", - ) - ) - - # If the data is invalid, no one is ignored. - ignored_users_content = content.get("ignored_users", {}) - if isinstance(ignored_users_content, dict): - currently_ignored_users = set(ignored_users_content) - else: - currently_ignored_users = set() + if account_data_type != AccountDataTypes.IGNORED_USER_LIST: + return - # Delete entries which are no longer ignored. - self.db_pool.simple_delete_many_txn( + # Insert / delete to sync the list of ignored users. + previously_ignored_users = set( + self.db_pool.simple_select_onecol_txn( txn, table="ignored_users", - column="ignored_user_id", - iterable=previously_ignored_users - currently_ignored_users, keyvalues={"ignorer_user_id": user_id}, + retcol="ignored_user_id", ) + ) - # Add entries which are newly ignored. - self.db_pool.simple_insert_many_txn( - txn, - table="ignored_users", - values=[ - {"ignorer_user_id": user_id, "ignored_user_id": u} - for u in currently_ignored_users - previously_ignored_users - ], - ) + # If the data is invalid, no one is ignored. + ignored_users_content = content.get("ignored_users", {}) + if isinstance(ignored_users_content, dict): + currently_ignored_users = set(ignored_users_content) + else: + currently_ignored_users = set() - # Invalidate the cache for any ignored users which were added or removed. - for ignored_user_id in previously_ignored_users ^ currently_ignored_users: - self._invalidate_cache_and_stream( - txn, self.ignored_by, (ignored_user_id,) - ) + # Delete entries which are no longer ignored. + self.db_pool.simple_delete_many_txn( + txn, + table="ignored_users", + column="ignored_user_id", + iterable=previously_ignored_users - currently_ignored_users, + keyvalues={"ignorer_user_id": user_id}, + ) + + # Add entries which are newly ignored. + self.db_pool.simple_insert_many_txn( + txn, + table="ignored_users", + values=[ + {"ignorer_user_id": user_id, "ignored_user_id": u} + for u in currently_ignored_users - previously_ignored_users + ], + ) + + # Invalidate the cache for any ignored users which were added or removed. + for ignored_user_id in previously_ignored_users ^ currently_ignored_users: + self._invalidate_cache_and_stream( + txn, self.ignored_by, (ignored_user_id,) + ) async def _update_max_stream_id(self, next_id: int) -> None: """Update the max stream_id From 2e7a8e0d3df1a33d361f39d81f76e4b14592d269 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 6 Jan 2021 08:33:25 -0500 Subject: [PATCH 07/13] Add indexes after inserting data. --- .../main/schema/delta/58/28ignored_user.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/schema/delta/58/28ignored_user.py b/synapse/storage/databases/main/schema/delta/58/28ignored_user.py index ad7537bd1fdc..05f2008a60fa 100644 --- a/synapse/storage/databases/main/schema/delta/58/28ignored_user.py +++ b/synapse/storage/databases/main/schema/delta/58/28ignored_user.py @@ -47,6 +47,11 @@ def run_upgrade(cur: Cursor, database_engine: BaseDatabaseEngine, *args, **kwarg if isinstance(ignored_users, dict) and ignored_users: cur.executemany(insert_sql, [(user_id, u) for u in ignored_users]) + # Add indexes after inserting data for efficiency. + logger.info("Adding constraints to ignored_users table") + execute_statements_from_stream(cur, StringIO(_constraints_commands)) + + def run_create(cur: Cursor, database_engine: BaseDatabaseEngine, *args, **kwargs): logger.info("Creating ignored_users table") @@ -61,7 +66,14 @@ def run_create(cur: Cursor, database_engine: BaseDatabaseEngine, *args, **kwargs -- denormalized from account data. CREATE TABLE IF NOT EXISTS ignored_users( ignorer_user_id TEXT NOT NULL, -- The user ID of the user who is ignoring another user. (This is a local user.) - ignored_user_id TEXT NOT NULL, -- The user ID of the user who is being ignored. (This is a local or remote user.) - UNIQUE (ignorer_user_id, ignored_user_id) + ignored_user_id TEXT NOT NULL -- The user ID of the user who is being ignored. (This is a local or remote user.) ); """ + +_constraints_commands = """ +ALTER TABLE ONLY ignored_users + ADD CONSTRAINT ignored_users_uniqueness UNIQUE (ignorer_user_id, ignored_user_id); + +-- Add an index on ignored_users since look-ups are done to get all ignorers of an ignored user. +CREATE INDEX ignored_users_ignored_user_id ON ignored_users (ignored_user_id); +""" From 5abfb22108e2235d49f99d466614c82d6be4d5a8 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 6 Jan 2021 08:45:31 -0500 Subject: [PATCH 08/13] Mark the cache as iterable. --- synapse/storage/databases/main/account_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index 1b0f3013b484..2a241ef3654e 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -287,7 +287,7 @@ def get_updated_account_data_for_user_txn(txn): "get_updated_account_data_for_user", get_updated_account_data_for_user_txn ) - @cached(max_entries=5000) + @cached(max_entries=5000, iterable=True) async def ignored_by(self, user_id: str) -> Set[str]: """ Get users which ignore the given user. From 35d0743fcfbdfb1e20ed18e4c15478a2537e79a1 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 6 Jan 2021 08:51:13 -0500 Subject: [PATCH 09/13] Lint. --- synapse/storage/databases/main/account_data.py | 4 +--- .../storage/databases/main/schema/delta/58/28ignored_user.py | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index 2a241ef3654e..bff51e92b9d1 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -485,9 +485,7 @@ def _add_account_data_for_user( # Invalidate the cache for any ignored users which were added or removed. for ignored_user_id in previously_ignored_users ^ currently_ignored_users: - self._invalidate_cache_and_stream( - txn, self.ignored_by, (ignored_user_id,) - ) + self._invalidate_cache_and_stream(txn, self.ignored_by, (ignored_user_id,)) async def _update_max_stream_id(self, next_id: int) -> None: """Update the max stream_id diff --git a/synapse/storage/databases/main/schema/delta/58/28ignored_user.py b/synapse/storage/databases/main/schema/delta/58/28ignored_user.py index 05f2008a60fa..3d29cc686b35 100644 --- a/synapse/storage/databases/main/schema/delta/58/28ignored_user.py +++ b/synapse/storage/databases/main/schema/delta/58/28ignored_user.py @@ -52,7 +52,6 @@ def run_upgrade(cur: Cursor, database_engine: BaseDatabaseEngine, *args, **kwarg execute_statements_from_stream(cur, StringIO(_constraints_commands)) - def run_create(cur: Cursor, database_engine: BaseDatabaseEngine, *args, **kwargs): logger.info("Creating ignored_users table") execute_statements_from_stream(cur, StringIO(_create_commands)) From 0778a42604d2dd789491d6b6a5730d958b450081 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 6 Jan 2021 09:11:40 -0500 Subject: [PATCH 10/13] Fix uniqueness constraint. --- .../storage/databases/main/schema/delta/58/28ignored_user.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/schema/delta/58/28ignored_user.py b/synapse/storage/databases/main/schema/delta/58/28ignored_user.py index 3d29cc686b35..f3b253582806 100644 --- a/synapse/storage/databases/main/schema/delta/58/28ignored_user.py +++ b/synapse/storage/databases/main/schema/delta/58/28ignored_user.py @@ -70,8 +70,7 @@ def run_create(cur: Cursor, database_engine: BaseDatabaseEngine, *args, **kwargs """ _constraints_commands = """ -ALTER TABLE ONLY ignored_users - ADD CONSTRAINT ignored_users_uniqueness UNIQUE (ignorer_user_id, ignored_user_id); +CREATE UNIQUE INDEX ignored_users_uniqueness ON ignored_users (ignorer_user_id, ignored_user_id); -- Add an index on ignored_users since look-ups are done to get all ignorers of an ignored user. CREATE INDEX ignored_users_ignored_user_id ON ignored_users (ignored_user_id); From de576054e8e1b3b42c5a8e982de94b93766e443a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 6 Jan 2021 11:13:05 -0500 Subject: [PATCH 11/13] Bump schema version to 59. --- .../schema/delta/{58/28ignored_user.py => 59/1ignored_user.py} | 0 synapse/storage/prepare_database.py | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename synapse/storage/databases/main/schema/delta/{58/28ignored_user.py => 59/1ignored_user.py} (100%) diff --git a/synapse/storage/databases/main/schema/delta/58/28ignored_user.py b/synapse/storage/databases/main/schema/delta/59/1ignored_user.py similarity index 100% rename from synapse/storage/databases/main/schema/delta/58/28ignored_user.py rename to synapse/storage/databases/main/schema/delta/59/1ignored_user.py diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index f91a2eae7aa4..a0f0ff6c9dfa 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -38,7 +38,7 @@ # XXX: If you're about to bump this to 59 (or higher) please create an update # that drops the unused `cache_invalidation_stream` table, as per #7436! # XXX: Also add an update to drop `account_data_max_stream_id` as per #7656! -SCHEMA_VERSION = 58 +SCHEMA_VERSION = 59 dir_path = os.path.abspath(os.path.dirname(__file__)) From f7ea81918d7fe5b0048cdfc87fe45314ceb31aa6 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 7 Jan 2021 11:34:12 +0000 Subject: [PATCH 12/13] Move schema file --- .../main/schema/delta/59/{1ignored_user.py => 01ignored_user.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename synapse/storage/databases/main/schema/delta/59/{1ignored_user.py => 01ignored_user.py} (100%) diff --git a/synapse/storage/databases/main/schema/delta/59/1ignored_user.py b/synapse/storage/databases/main/schema/delta/59/01ignored_user.py similarity index 100% rename from synapse/storage/databases/main/schema/delta/59/1ignored_user.py rename to synapse/storage/databases/main/schema/delta/59/01ignored_user.py From f08f688c267c0d3d5b4f52a055318a3f44c3047d Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 7 Jan 2021 12:05:08 +0000 Subject: [PATCH 13/13] Fix bug where we didn't add constraints --- .../main/schema/delta/59/01ignored_user.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/synapse/storage/databases/main/schema/delta/59/01ignored_user.py b/synapse/storage/databases/main/schema/delta/59/01ignored_user.py index f3b253582806..f35c70b69908 100644 --- a/synapse/storage/databases/main/schema/delta/59/01ignored_user.py +++ b/synapse/storage/databases/main/schema/delta/59/01ignored_user.py @@ -28,6 +28,16 @@ def run_upgrade(cur: Cursor, database_engine: BaseDatabaseEngine, *args, **kwargs): + pass + + +def run_create(cur: Cursor, database_engine: BaseDatabaseEngine, *args, **kwargs): + logger.info("Creating ignored_users table") + execute_statements_from_stream(cur, StringIO(_create_commands)) + + # We now upgrade existing data, if any. We don't do this in `run_upgrade` as + # we a) want to run these before adding constraints and b) `run_upgrade` is + # not run on empty databases. insert_sql = """ INSERT INTO ignored_users (ignorer_user_id, ignored_user_id) VALUES (?, ?) """ @@ -52,11 +62,6 @@ def run_upgrade(cur: Cursor, database_engine: BaseDatabaseEngine, *args, **kwarg execute_statements_from_stream(cur, StringIO(_constraints_commands)) -def run_create(cur: Cursor, database_engine: BaseDatabaseEngine, *args, **kwargs): - logger.info("Creating ignored_users table") - execute_statements_from_stream(cur, StringIO(_create_commands)) - - # there might be duplicates, so the easiest way to achieve this is to create a new # table with the right data, and renaming it into place