From 7ae25a853fb8b8c84be2846fe1b2130f2a87d7ad Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 May 2023 13:43:52 -0400 Subject: [PATCH 1/7] Remove obsolete docs. --- docs/replication.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/docs/replication.md b/docs/replication.md index 108da9a065d8..25145daaf5e5 100644 --- a/docs/replication.md +++ b/docs/replication.md @@ -30,12 +30,6 @@ minimal. See [the TCP replication documentation](tcp_replication.md). -### The Slaved DataStore - -There are read-only version of the synapse storage layer in -`synapse/replication/slave/storage` that use the response of the -replication API to invalidate their caches. - ### The TCP Replication Module Information about how the tcp replication module is structured, including how the classes interact, can be found in From e7042f11788d7fcb60cb287d5e98a16f00ffa594 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 May 2023 13:45:41 -0400 Subject: [PATCH 2/7] Rename datastores. --- synapse/app/admin_cmd.py | 4 ++-- synapse/app/generic_worker.py | 4 ++-- synapse/module_api/__init__.py | 6 ++---- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/synapse/app/admin_cmd.py b/synapse/app/admin_cmd.py index b05fe2c589c4..f9aada269a0a 100644 --- a/synapse/app/admin_cmd.py +++ b/synapse/app/admin_cmd.py @@ -64,7 +64,7 @@ logger = logging.getLogger("synapse.app.admin_cmd") -class AdminCmdSlavedStore( +class AdminCmdStore( FilteringWorkerStore, ClientIpWorkerStore, DeviceWorkerStore, @@ -103,7 +103,7 @@ def __init__( class AdminCmdServer(HomeServer): - DATASTORE_CLASS = AdminCmdSlavedStore # type: ignore + DATASTORE_CLASS = AdminCmdStore # type: ignore async def export_data_command(hs: HomeServer, args: argparse.Namespace) -> None: diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index e17ce35b8e29..909ebccf78cb 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -102,7 +102,7 @@ logger = logging.getLogger("synapse.app.generic_worker") -class GenericWorkerSlavedStore( +class GenericWorkerStore( # FIXME(#3714): We need to add UserDirectoryStore as we write directly # rather than going via the correct worker. UserDirectoryStore, @@ -154,7 +154,7 @@ class GenericWorkerSlavedStore( class GenericWorkerServer(HomeServer): - DATASTORE_CLASS = GenericWorkerSlavedStore # type: ignore + DATASTORE_CLASS = GenericWorkerStore # type: ignore def _listen_http(self, listener_config: ListenerConfig) -> None: assert listener_config.http_options is not None diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 2c9d181acf36..0e9f366cba62 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -134,7 +134,7 @@ from synapse.util.frozenutils import freeze if TYPE_CHECKING: - from synapse.app.generic_worker import GenericWorkerSlavedStore + from synapse.app.generic_worker import GenericWorkerStore from synapse.server import HomeServer @@ -237,9 +237,7 @@ def __init__(self, hs: "HomeServer", auth_handler: AuthHandler) -> None: # TODO: Fix this type hint once the types for the data stores have been ironed # out. - self._store: Union[ - DataStore, "GenericWorkerSlavedStore" - ] = hs.get_datastores().main + self._store: Union[DataStore, "GenericWorkerStore"] = hs.get_datastores().main self._storage_controllers = hs.get_storage_controllers() self._auth = hs.get_auth() self._auth_handler = auth_handler From 49a00958c673171a2f699c31dadeaa631101a3af Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 May 2023 13:48:21 -0400 Subject: [PATCH 3/7] Update obsolete comments. --- synapse/storage/databases/main/account_data.py | 7 ++----- synapse/storage/databases/main/devices.py | 2 -- synapse/storage/databases/main/events_worker.py | 7 ++----- synapse/storage/databases/main/receipts.py | 7 ++----- 4 files changed, 6 insertions(+), 17 deletions(-) diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index a9843f6e174a..8f7bdbc61a7b 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -85,13 +85,10 @@ def __init__( writers=hs.config.worker.writers.account_data, ) else: + # Multiple writers are not supported for SQLite. + # # We shouldn't be running in worker mode with SQLite, but its useful # to support it for unit tests. - # - # If this process is the writer than we need to use - # `StreamIdGenerator`, otherwise we use `SlavedIdTracker` which gets - # updated over replication. (Multiple writers are not supported for - # SQLite). self._account_data_id_gen = StreamIdGenerator( db_conn, hs.get_replication_notifier(), diff --git a/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py index 5503621ad613..a67fdb3c22ce 100644 --- a/synapse/storage/databases/main/devices.py +++ b/synapse/storage/databases/main/devices.py @@ -105,8 +105,6 @@ def __init__( is_writer=hs.config.worker.worker_app is None, ) - # Type-ignore: _device_list_id_gen is mixed in from either DataStore (as a - # StreamIdGenerator) or SlavedDataStore (as a SlavedIdTracker). device_list_max = self._device_list_id_gen.get_current_token() device_list_prefill, min_device_list_id = self.db_pool.get_cache_dict( db_conn, diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index 0ff3fc73693e..53aa5933d56f 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -213,13 +213,10 @@ def __init__( writers=hs.config.worker.writers.events, ) else: + # Multiple writers are not supported for SQLite. + # # We shouldn't be running in worker mode with SQLite, but its useful # to support it for unit tests. - # - # If this process is the writer than we need to use - # `StreamIdGenerator`, otherwise we use `SlavedIdTracker` which gets - # updated over replication. (Multiple writers are not supported for - # SQLite). self._stream_id_gen = StreamIdGenerator( db_conn, hs.get_replication_notifier(), diff --git a/synapse/storage/databases/main/receipts.py b/synapse/storage/databases/main/receipts.py index 074942b16785..5ee5c7ad9f14 100644 --- a/synapse/storage/databases/main/receipts.py +++ b/synapse/storage/databases/main/receipts.py @@ -85,13 +85,10 @@ def __init__( else: self._can_write_to_receipts = True + # Multiple writers are not supported for SQLite. + # # We shouldn't be running in worker mode with SQLite, but its useful # to support it for unit tests. - # - # If this process is the writer than we need to use - # `StreamIdGenerator`, otherwise we use `SlavedIdTracker` which gets - # updated over replication. (Multiple writers are not supported for - # SQLite). self._receipts_id_gen = StreamIdGenerator( db_conn, hs.get_replication_notifier(), From c1d65a273b76057188f59609c4d298f143e64913 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 May 2023 13:50:26 -0400 Subject: [PATCH 4/7] Update additional comments. --- synapse/storage/databases/main/cache.py | 14 +++++++------- .../storage/schema/main/delta/34/cache_stream.py | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/synapse/storage/databases/main/cache.py b/synapse/storage/databases/main/cache.py index bd07d2017143..46fa0a73f9e4 100644 --- a/synapse/storage/databases/main/cache.py +++ b/synapse/storage/databases/main/cache.py @@ -274,11 +274,11 @@ def _invalidate_caches_for_event( async def invalidate_cache_and_stream( self, cache_name: str, keys: Tuple[Any, ...] ) -> None: - """Invalidates the cache and adds it to the cache stream so slaves + """Invalidates the cache and adds it to the cache stream so other workers will know to invalidate their caches. - This should only be used to invalidate caches where slaves won't - otherwise know from other replication streams that the cache should + This should only be used to invalidate caches where other workers won't + otherwise have known from other replication streams that the cache should be invalidated. """ cache_func = getattr(self, cache_name, None) @@ -297,11 +297,11 @@ def _invalidate_cache_and_stream( cache_func: CachedFunction, keys: Tuple[Any, ...], ) -> None: - """Invalidates the cache and adds it to the cache stream so slaves + """Invalidates the cache and adds it to the cache stream so other workers will know to invalidate their caches. - This should only be used to invalidate caches where slaves won't - otherwise know from other replication streams that the cache should + This should only be used to invalidate caches where other workers won't + otherwise have known from other replication streams that the cache should be invalidated. """ txn.call_after(cache_func.invalidate, keys) @@ -310,7 +310,7 @@ def _invalidate_cache_and_stream( def _invalidate_all_cache_and_stream( self, txn: LoggingTransaction, cache_func: CachedFunction ) -> None: - """Invalidates the entire cache and adds it to the cache stream so slaves + """Invalidates the entire cache and adds it to the cache stream so other workers will know to invalidate their caches. """ diff --git a/synapse/storage/schema/main/delta/34/cache_stream.py b/synapse/storage/schema/main/delta/34/cache_stream.py index 682c86da1abd..882f9b893b05 100644 --- a/synapse/storage/schema/main/delta/34/cache_stream.py +++ b/synapse/storage/schema/main/delta/34/cache_stream.py @@ -21,7 +21,7 @@ logger = logging.getLogger(__name__) -# This stream is used to notify replication slaves that some caches have +# This stream is used to notify workers over replication that some caches have # been invalidated that they cannot infer from the other streams. CREATE_TABLE = """ CREATE TABLE cache_invalidation_stream ( From 2d2abe6b8e8b453e000d58651120eac177cfe503 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 May 2023 13:53:22 -0400 Subject: [PATCH 5/7] Update more comments. --- synapse/replication/tcp/client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/replication/tcp/client.py b/synapse/replication/tcp/client.py index 200f667fdf27..139f57cf8668 100644 --- a/synapse/replication/tcp/client.py +++ b/synapse/replication/tcp/client.py @@ -60,7 +60,7 @@ class ReplicationDataHandler: """Handles incoming stream updates from replication. - This instance notifies the slave data store about updates. Can be subclassed + This instance notifies the data store about updates. Can be subclassed to handle updates in additional ways. """ @@ -91,7 +91,7 @@ async def on_rdata( ) -> None: """Called to handle a batch of replication data with a given stream token. - By default this just pokes the slave store. Can be overridden in subclasses to + By default, this just pokes the data store. Can be overridden in subclasses to handle more. Args: From 8efe91468891ffb97be58f991f5c416ff05de2c2 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 May 2023 14:00:59 -0400 Subject: [PATCH 6/7] Fix-up remaining test references. --- tests/app/test_openid_listener.py | 2 +- tests/replication/slave/storage/__init__.py | 13 ------------- .../replication/{slave => storage}/__init__.py | 0 tests/replication/{slave => }/storage/_base.py | 18 +++++++++--------- .../{slave => }/storage/test_events.py | 10 +++++----- 5 files changed, 15 insertions(+), 28 deletions(-) delete mode 100644 tests/replication/slave/storage/__init__.py rename tests/replication/{slave => storage}/__init__.py (100%) rename tests/replication/{slave => }/storage/_base.py (82%) rename tests/replication/{slave => }/storage/test_events.py (98%) diff --git a/tests/app/test_openid_listener.py b/tests/app/test_openid_listener.py index 2ee343d8a4ad..6e0413400ee3 100644 --- a/tests/app/test_openid_listener.py +++ b/tests/app/test_openid_listener.py @@ -38,7 +38,7 @@ def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: def default_config(self) -> JsonDict: conf = super().default_config() - # we're using FederationReaderServer, which uses a SlavedStore, so we + # we're using GenericWorkerServer, which uses a GenericWorkerStore, so we # have to tell the FederationHandler not to try to access stuff that is only # in the primary store. conf["worker_app"] = "yes" diff --git a/tests/replication/slave/storage/__init__.py b/tests/replication/slave/storage/__init__.py deleted file mode 100644 index f43a360a807c..000000000000 --- a/tests/replication/slave/storage/__init__.py +++ /dev/null @@ -1,13 +0,0 @@ -# Copyright 2016 OpenMarket Ltd -# -# 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. diff --git a/tests/replication/slave/__init__.py b/tests/replication/storage/__init__.py similarity index 100% rename from tests/replication/slave/__init__.py rename to tests/replication/storage/__init__.py diff --git a/tests/replication/slave/storage/_base.py b/tests/replication/storage/_base.py similarity index 82% rename from tests/replication/slave/storage/_base.py rename to tests/replication/storage/_base.py index 4c9b494344ae..de26a62ae19f 100644 --- a/tests/replication/slave/storage/_base.py +++ b/tests/replication/storage/_base.py @@ -24,7 +24,7 @@ from tests.replication._base import BaseStreamTestCase -class BaseSlavedStoreTestCase(BaseStreamTestCase): +class BaseWorkerStoreTestCase(BaseStreamTestCase): def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: return self.setup_test_homeserver(federation_client=Mock()) @@ -34,7 +34,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.reconnect() self.master_store = hs.get_datastores().main - self.slaved_store = self.worker_hs.get_datastores().main + self.worker_store = self.worker_hs.get_datastores().main persistence = hs.get_storage_controllers().persistence assert persistence is not None self.persistance = persistence @@ -50,7 +50,7 @@ def check( self, method: str, args: Iterable[Any], expected_result: Optional[Any] = None ) -> None: master_result = self.get_success(getattr(self.master_store, method)(*args)) - slaved_result = self.get_success(getattr(self.slaved_store, method)(*args)) + worker_result = self.get_success(getattr(self.worker_store, method)(*args)) if expected_result is not None: self.assertEqual( master_result, @@ -59,14 +59,14 @@ def check( % (expected_result, master_result), ) self.assertEqual( - slaved_result, + worker_result, expected_result, - "Expected slave result to be %r but was %r" - % (expected_result, slaved_result), + "Expected worker result to be %r but was %r" + % (expected_result, worker_result), ) self.assertEqual( master_result, - slaved_result, - "Slave result %r does not match master result %r" - % (slaved_result, master_result), + worker_result, + "Worker result %r does not match master result %r" + % (worker_result, master_result), ) diff --git a/tests/replication/slave/storage/test_events.py b/tests/replication/storage/test_events.py similarity index 98% rename from tests/replication/slave/storage/test_events.py rename to tests/replication/storage/test_events.py index b2125b1fea41..f7c6417a09fd 100644 --- a/tests/replication/slave/storage/test_events.py +++ b/tests/replication/storage/test_events.py @@ -36,7 +36,7 @@ from tests.server import FakeTransport -from ._base import BaseSlavedStoreTestCase +from ._base import BaseWorkerStoreTestCase USER_ID = "@feeling:test" USER_ID_2 = "@bright:test" @@ -63,7 +63,7 @@ def unpatch() -> None: return unpatch -class EventsWorkerStoreTestCase(BaseSlavedStoreTestCase): +class EventsWorkerStoreTestCase(BaseWorkerStoreTestCase): STORE_TYPE = EventsWorkerStore def setUp(self) -> None: @@ -294,7 +294,7 @@ def test_get_rooms_for_user_with_stream_ordering_with_multi_event_persist( assert j2.internal_metadata.stream_ordering is not None event_source = RoomEventSource(self.hs) - event_source.store = self.slaved_store + event_source.store = self.worker_store current_token = event_source.get_current_key() # gradually stream out the replication @@ -310,12 +310,12 @@ def test_get_rooms_for_user_with_stream_ordering_with_multi_event_persist( # # First, we get a list of the rooms we are joined to joined_rooms = self.get_success( - self.slaved_store.get_rooms_for_user_with_stream_ordering(USER_ID_2) + self.worker_store.get_rooms_for_user_with_stream_ordering(USER_ID_2) ) # Then, we get a list of the events since the last sync membership_changes = self.get_success( - self.slaved_store.get_membership_changes_for_user( + self.worker_store.get_membership_changes_for_user( USER_ID_2, prev_token, current_token ) ) From 478cf9f3316d2815b5c740879b32884b387b6c27 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 May 2023 14:09:20 -0400 Subject: [PATCH 7/7] Newsfragment --- changelog.d/15606.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/15606.misc diff --git a/changelog.d/15606.misc b/changelog.d/15606.misc new file mode 100644 index 000000000000..44265fbf02ab --- /dev/null +++ b/changelog.d/15606.misc @@ -0,0 +1 @@ +Update internal terminology for workers.