From 0c0c0cbf19f44a0259b6e6cdfb08fca55a3b8750 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Tue, 25 Apr 2023 04:31:36 -0500 Subject: [PATCH 01/15] Add master to the instance_map as part of Complement, have ReplicationEndpoint look at instance_map for master. --- docker/configure_workers_and_start.py | 13 +++++++++++-- synapse/replication/http/_base.py | 16 ++++++++-------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/docker/configure_workers_and_start.py b/docker/configure_workers_and_start.py index 4beec3daaf8b..3e4ee096903f 100755 --- a/docker/configure_workers_and_start.py +++ b/docker/configure_workers_and_start.py @@ -69,6 +69,9 @@ from jinja2 import Environment, FileSystemLoader MAIN_PROCESS_HTTP_LISTENER_PORT = 8080 +MAIN_PROCESS_INSTANCE_NAME = "master" +MAIN_PROCESS_LOCALHOST_ADDRESS = "127.0.0.1" +MAIN_PROCESS_REPLICATION_PORT = 9093 # A simple name used as a placeholder in the WORKERS_CONFIG below. This will be replaced # during processing with the name of the worker. @@ -719,8 +722,8 @@ def generate_worker_files( # shared config file. listeners = [ { - "port": 9093, - "bind_address": "127.0.0.1", + "port": MAIN_PROCESS_REPLICATION_PORT, + "bind_address": MAIN_PROCESS_LOCALHOST_ADDRESS, "type": "http", "resources": [{"names": ["replication"]}], } @@ -870,6 +873,12 @@ def generate_worker_files( workers_in_use = len(requested_worker_types) > 0 + instance_map = shared_config.setdefault("instance_map", {}) + instance_map[MAIN_PROCESS_INSTANCE_NAME] = { + "host": MAIN_PROCESS_LOCALHOST_ADDRESS, + "port": MAIN_PROCESS_REPLICATION_PORT, + } + # Shared homeserver config convert( "/conf/shared.yaml.j2", diff --git a/synapse/replication/http/_base.py b/synapse/replication/http/_base.py index 8c2c54c07a49..b315e4b6e4cd 100644 --- a/synapse/replication/http/_base.py +++ b/synapse/replication/http/_base.py @@ -198,9 +198,9 @@ def make_client(cls, hs: "HomeServer") -> Callable: local_instance_name = hs.get_instance_name() # The value of these option should match the replication listener settings - master_host = hs.config.worker.worker_replication_host - master_port = hs.config.worker.worker_replication_http_port - master_tls = hs.config.worker.worker_replication_http_tls + # master_host = hs.config.worker.worker_replication_host + # master_port = hs.config.worker.worker_replication_http_port + # master_tls = hs.config.worker.worker_replication_http_tls instance_map = hs.config.worker.instance_map @@ -221,11 +221,11 @@ async def send_request(*, instance_name: str = "master", **kwargs: Any) -> Any: with outgoing_gauge.track_inprogress(): if instance_name == local_instance_name: raise Exception("Trying to send HTTP request to self") - if instance_name == "master": - host = master_host - port = master_port - tls = master_tls - elif instance_name in instance_map: + # if instance_name == "master": + # host = master_host + # port = master_port + # tls = master_tls + if instance_name in instance_map: host = instance_map[instance_name].host port = instance_map[instance_name].port tls = instance_map[instance_name].tls From d6071f755e436404b979d755d53d097540a6ddae Mon Sep 17 00:00:00 2001 From: Jason Little Date: Tue, 25 Apr 2023 04:31:54 -0500 Subject: [PATCH 02/15] Fix typo in drive by. --- tests/replication/_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/replication/_base.py b/tests/replication/_base.py index 0f1a8a145f6f..94da39dd9a4f 100644 --- a/tests/replication/_base.py +++ b/tests/replication/_base.py @@ -310,7 +310,7 @@ def create_test_resource(self) -> ReplicationRestResource: def make_worker_hs( self, worker_app: str, extra_config: Optional[dict] = None, **kwargs: Any ) -> HomeServer: - """Make a new worker HS instance, correctly connecting replcation + """Make a new worker HS instance, correctly connecting replication stream to the master HS. Args: From da357b05ff99d28982b7462e5ef314a7b67dd8c6 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Tue, 25 Apr 2023 05:14:23 -0500 Subject: [PATCH 03/15] Remove unnecessary worker_replication_* bits from unit tests and add master to instance_map(hopefully in the right place) --- tests/replication/_base.py | 6 ++---- tests/replication/test_auth.py | 3 --- tests/replication/test_client_reader_shard.py | 2 -- tests/replication/test_sharded_event_persister.py | 1 + 4 files changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/replication/_base.py b/tests/replication/_base.py index 94da39dd9a4f..2838f5b45799 100644 --- a/tests/replication/_base.py +++ b/tests/replication/_base.py @@ -110,8 +110,7 @@ def create_resource_dict(self) -> Dict[str, Resource]: def _get_worker_hs_config(self) -> dict: config = self.default_config() config["worker_app"] = "synapse.app.generic_worker" - config["worker_replication_host"] = "testserv" - config["worker_replication_http_port"] = "8765" + config["instance_map"] = {"master": {"host": "testserv", "port": 8765}} return config def _build_replication_data_handler(self) -> "TestReplicationDataHandler": @@ -249,6 +248,7 @@ def default_config(self) -> Dict[str, Any]: """ base = super().default_config() base["redis"] = {"enabled": True} + base["instance_map"] = {"master": {"host": "testserv", "port": 8765}} return base def setUp(self) -> None: @@ -388,8 +388,6 @@ def make_worker_hs( def _get_worker_hs_config(self) -> dict: config = self.default_config() - config["worker_replication_host"] = "testserv" - config["worker_replication_http_port"] = "8765" return config def replicate(self) -> None: diff --git a/tests/replication/test_auth.py b/tests/replication/test_auth.py index 98602371e467..f7bca0063da8 100644 --- a/tests/replication/test_auth.py +++ b/tests/replication/test_auth.py @@ -43,9 +43,6 @@ def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: def _get_worker_hs_config(self) -> dict: config = self.default_config() config["worker_app"] = "synapse.app.generic_worker" - config["worker_replication_host"] = "testserv" - config["worker_replication_http_port"] = "8765" - return config def _test_register(self) -> FakeChannel: diff --git a/tests/replication/test_client_reader_shard.py b/tests/replication/test_client_reader_shard.py index eca503376110..a18859099fac 100644 --- a/tests/replication/test_client_reader_shard.py +++ b/tests/replication/test_client_reader_shard.py @@ -29,8 +29,6 @@ class ClientReaderTestCase(BaseMultiWorkerStreamTestCase): def _get_worker_hs_config(self) -> dict: config = self.default_config() config["worker_app"] = "synapse.app.generic_worker" - config["worker_replication_host"] = "testserv" - config["worker_replication_http_port"] = "8765" return config def test_register_single_worker(self) -> None: diff --git a/tests/replication/test_sharded_event_persister.py b/tests/replication/test_sharded_event_persister.py index 7f9cc67e735a..d3d489d174a4 100644 --- a/tests/replication/test_sharded_event_persister.py +++ b/tests/replication/test_sharded_event_persister.py @@ -50,6 +50,7 @@ def default_config(self) -> dict: conf = super().default_config() conf["stream_writers"] = {"events": ["worker1", "worker2"]} conf["instance_map"] = { + "master": {"host": "testserv", "port": 8765}, "worker1": {"host": "testserv", "port": 1001}, "worker2": {"host": "testserv", "port": 1002}, } From 1d6210119247bfc9914646dabb88cd673e2f32ea Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 26 Apr 2023 02:51:05 -0500 Subject: [PATCH 04/15] Several updates: 1. Switch from master to main for naming the main process in the instance_map. Add useful constants for easier adjustment of names in the future. 2. Add backwards compatibility for worker_replication_* to allow time to transition to new style. Make sure to prioritize declaring main directly on the instance_map. 3. Clean up old comments/commented out code. 4. Adjust unit tests to match with new code. 5. Adjust Complement setup infrastructure to only add main to the instance_map if workers are used and remove now unused options from the worker.yaml template. --- docker/conf-workers/worker.yaml.j2 | 4 - docker/configure_workers_and_start.py | 14 ++-- synapse/config/workers.py | 81 +++++++++++++++---- synapse/replication/http/_base.py | 9 +-- tests/module_api/test_api.py | 1 + tests/replication/_base.py | 4 +- .../test_sharded_event_persister.py | 2 +- 7 files changed, 80 insertions(+), 35 deletions(-) diff --git a/docker/conf-workers/worker.yaml.j2 b/docker/conf-workers/worker.yaml.j2 index 42131afc9583..44c6e413cf94 100644 --- a/docker/conf-workers/worker.yaml.j2 +++ b/docker/conf-workers/worker.yaml.j2 @@ -6,10 +6,6 @@ worker_app: "{{ app }}" worker_name: "{{ name }}" -# The replication listener on the main synapse process. -worker_replication_host: 127.0.0.1 -worker_replication_http_port: 9093 - worker_listeners: - type: http port: {{ port }} diff --git a/docker/configure_workers_and_start.py b/docker/configure_workers_and_start.py index 3e4ee096903f..79b5b8739764 100755 --- a/docker/configure_workers_and_start.py +++ b/docker/configure_workers_and_start.py @@ -69,7 +69,7 @@ from jinja2 import Environment, FileSystemLoader MAIN_PROCESS_HTTP_LISTENER_PORT = 8080 -MAIN_PROCESS_INSTANCE_NAME = "master" +MAIN_PROCESS_INSTANCE_NAME = "main" MAIN_PROCESS_LOCALHOST_ADDRESS = "127.0.0.1" MAIN_PROCESS_REPLICATION_PORT = 9093 @@ -873,11 +873,13 @@ def generate_worker_files( workers_in_use = len(requested_worker_types) > 0 - instance_map = shared_config.setdefault("instance_map", {}) - instance_map[MAIN_PROCESS_INSTANCE_NAME] = { - "host": MAIN_PROCESS_LOCALHOST_ADDRESS, - "port": MAIN_PROCESS_REPLICATION_PORT, - } + # If there are workers, add the main process to the instance_map too. + if workers_in_use: + instance_map = shared_config.setdefault("instance_map", {}) + instance_map[MAIN_PROCESS_INSTANCE_NAME] = { + "host": MAIN_PROCESS_LOCALHOST_ADDRESS, + "port": MAIN_PROCESS_REPLICATION_PORT, + } # Shared homeserver config convert( diff --git a/synapse/config/workers.py b/synapse/config/workers.py index 95b4047f1d3f..99fd42396595 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -39,6 +39,19 @@ Synapse version. Please use ``%s: name_of_worker`` instead. """ +_MISSING_MAIN_PROCESS_INSTANCE_MAP_DATA = """ +Missing data for a worker to connect to main process. Please include '%s' in the +`instance_map` declared in your shared yaml configuration, or optionally(as a deprecated +solution) in every worker's yaml as various `worker_replication_*` settings as defined +in workers documentation here: +`https://matrix-org.github.io/synapse/latest/workers.html#worker-configuration` +""" +# This allows for a handy knob when it's time to change from 'master' to +# something with less 'history' +MAIN_PROCESS_INSTANCE_NAME = "master" +# Use this to adjust what the main process is known as in the yaml instance_map +MAIN_PROCESS_INSTANCE_MAP_NAME = "main" + logger = logging.getLogger(__name__) @@ -161,27 +174,15 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: raise ConfigError("worker_log_config must be a string") self.worker_log_config = worker_log_config - # The host used to connect to the main synapse - self.worker_replication_host = config.get("worker_replication_host", None) - # The port on the main synapse for TCP replication if "worker_replication_port" in config: raise ConfigError(DIRECT_TCP_ERROR, ("worker_replication_port",)) - # The port on the main synapse for HTTP replication endpoint - self.worker_replication_http_port = config.get("worker_replication_http_port") - - # The tls mode on the main synapse for HTTP replication endpoint. - # For backward compatibility this defaults to False. - self.worker_replication_http_tls = config.get( - "worker_replication_http_tls", False - ) - # The shared secret used for authentication when connecting to the main synapse. self.worker_replication_secret = config.get("worker_replication_secret", None) self.worker_name = config.get("worker_name", self.worker_app) - self.instance_name = self.worker_name or "master" + self.instance_name = self.worker_name or MAIN_PROCESS_INSTANCE_NAME # FIXME: Remove this check after a suitable amount of time. self.worker_main_http_uri = config.get("worker_main_http_uri", None) @@ -215,12 +216,58 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: ) # A map from instance name to host/port of their HTTP replication endpoint. + # Check if the main process is declared. Inject it into the map if it's not, + # based first on if a 'main' block is declared then on 'worker_replication_*' + # data. If both are available, default to instance_map. The main process + # itself doesn't need this data as it would never have to talk to itself. + instance_map: Dict[str, Any] = config.get("instance_map", {}) + + # TODO: In the interest of not using 'master' as the name in the instance_map, + # opt to use 'main' instead. Or perhaps 'home'? + if instance_map and self.instance_name is not MAIN_PROCESS_INSTANCE_NAME: + # The host used to connect to the main synapse + main_host = config.get("worker_replication_host", None) + + # The port on the main synapse for HTTP replication endpoint + main_port = config.get("worker_replication_http_port") + + # The tls mode on the main synapse for HTTP replication endpoint. + # For backward compatibility this defaults to False. + main_tls = config.get("worker_replication_http_tls", False) + + # For now, accept 'main' in the instance_map, but the replication system + # expects 'master', force that into being until it's changed later. + if MAIN_PROCESS_INSTANCE_MAP_NAME in instance_map: + instance_map.setdefault(MAIN_PROCESS_INSTANCE_NAME, {}) + for k, v in instance_map[MAIN_PROCESS_INSTANCE_MAP_NAME].items(): + instance_map[MAIN_PROCESS_INSTANCE_NAME].setdefault(k, v) + del instance_map[MAIN_PROCESS_INSTANCE_MAP_NAME] + logger.error(f"config.workers: {instance_map}") + # This is the backwards compatibility bit that handles the + # worker_replication_* bits using setdefault() to not overwrite anything. + elif main_host is not None and main_port is not None: + instance_map.setdefault( + MAIN_PROCESS_INSTANCE_NAME, + { + "host": main_host, + "port": main_port, + "tls": main_tls, + }, + ) + logger.error(f"config.workers: {instance_map}") + + else: + # If we've gotten here, it means that the main process is not on the + # instance_map and that not enough worker_replication_* variables + # were declared in the worker's yaml. + raise ConfigError( + _MISSING_MAIN_PROCESS_INSTANCE_MAP_DATA + % MAIN_PROCESS_INSTANCE_MAP_NAME + ) + self.instance_map: Dict[ str, InstanceLocationConfig - ] = parse_and_validate_mapping( - config.get("instance_map", {}), - InstanceLocationConfig, - ) + ] = parse_and_validate_mapping(instance_map, InstanceLocationConfig) # Map from type of streams to source, c.f. WriterLocations. writers = config.get("stream_writers") or {} diff --git a/synapse/replication/http/_base.py b/synapse/replication/http/_base.py index b315e4b6e4cd..a9df214fc6d3 100644 --- a/synapse/replication/http/_base.py +++ b/synapse/replication/http/_base.py @@ -25,6 +25,7 @@ from twisted.web.server import Request from synapse.api.errors import HttpResponseException, SynapseError +from synapse.config.workers import MAIN_PROCESS_INSTANCE_NAME from synapse.http import RequestTimedOutError from synapse.http.server import HttpServer from synapse.http.servlet import parse_json_object_from_request @@ -213,7 +214,9 @@ def make_client(cls, hs: "HomeServer") -> Callable: ) @trace_with_opname("outgoing_replication_request") - async def send_request(*, instance_name: str = "master", **kwargs: Any) -> Any: + async def send_request( + *, instance_name: str = MAIN_PROCESS_INSTANCE_NAME, **kwargs: Any + ) -> Any: # We have to pull these out here to avoid circular dependencies... streams = hs.get_replication_command_handler().get_streams_to_replicate() replication = hs.get_replication_data_handler() @@ -221,10 +224,6 @@ async def send_request(*, instance_name: str = "master", **kwargs: Any) -> Any: with outgoing_gauge.track_inprogress(): if instance_name == local_instance_name: raise Exception("Trying to send HTTP request to self") - # if instance_name == "master": - # host = master_host - # port = master_port - # tls = master_tls if instance_name in instance_map: host = instance_map[instance_name].host port = instance_map[instance_name].port diff --git a/tests/module_api/test_api.py b/tests/module_api/test_api.py index 758b4bc38bdd..bff7114cd89c 100644 --- a/tests/module_api/test_api.py +++ b/tests/module_api/test_api.py @@ -837,6 +837,7 @@ def default_config(self) -> Dict[str, Any]: conf = super().default_config() conf["stream_writers"] = {"presence": ["presence_writer"]} conf["instance_map"] = { + "main": {"host": "testserv", "port": 8765}, "presence_writer": {"host": "testserv", "port": 1001}, } return conf diff --git a/tests/replication/_base.py b/tests/replication/_base.py index 2838f5b45799..eb9b1f1cd9be 100644 --- a/tests/replication/_base.py +++ b/tests/replication/_base.py @@ -110,7 +110,7 @@ def create_resource_dict(self) -> Dict[str, Resource]: def _get_worker_hs_config(self) -> dict: config = self.default_config() config["worker_app"] = "synapse.app.generic_worker" - config["instance_map"] = {"master": {"host": "testserv", "port": 8765}} + config["instance_map"] = {"main": {"host": "testserv", "port": 8765}} return config def _build_replication_data_handler(self) -> "TestReplicationDataHandler": @@ -248,7 +248,7 @@ def default_config(self) -> Dict[str, Any]: """ base = super().default_config() base["redis"] = {"enabled": True} - base["instance_map"] = {"master": {"host": "testserv", "port": 8765}} + base["instance_map"] = {"main": {"host": "testserv", "port": 8765}} return base def setUp(self) -> None: diff --git a/tests/replication/test_sharded_event_persister.py b/tests/replication/test_sharded_event_persister.py index d3d489d174a4..4623d737fbf5 100644 --- a/tests/replication/test_sharded_event_persister.py +++ b/tests/replication/test_sharded_event_persister.py @@ -50,7 +50,7 @@ def default_config(self) -> dict: conf = super().default_config() conf["stream_writers"] = {"events": ["worker1", "worker2"]} conf["instance_map"] = { - "master": {"host": "testserv", "port": 8765}, + "main": {"host": "testserv", "port": 8765}, "worker1": {"host": "testserv", "port": 1001}, "worker2": {"host": "testserv", "port": 1002}, } From 1c1044c2cfeb0589aba81f054142ca2a8cc85abf Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 26 Apr 2023 04:33:02 -0500 Subject: [PATCH 05/15] Initial Docs upload --- docs/upgrade.md | 22 ++++++++++++++++++ .../configuration/config_documentation.md | 23 +++++++++++++------ docs/workers.md | 14 +++++++---- 3 files changed, 48 insertions(+), 11 deletions(-) diff --git a/docs/upgrade.md b/docs/upgrade.md index 0886b0311571..d94961e14f49 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -88,6 +88,28 @@ process, for example: dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb ``` +# Upgrading to v1.83.0 + +## Worker Yaml setting deprecations + +When using workers, +* `worker_replication_host` +* `worker_replication_http_port` +* `worker_replication_http_tls` + +can now be removed from individual worker yaml if you add the main process to the `instance_map` in the shared yaml configuration. + +Example: +```yaml +instance_map: + main: + host: localhost + port: 3456 + tls: false +``` +Notes: +* `tls` is optional but mirrors the functionality of `worker_replication_http_tls` +* Ensure these values match up with the `replication` listener declared for the main process. # Upgrading to v1.81.0 ## Application service path & authentication deprecations diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 1b6f2569490e..058f896a79ad 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3846,15 +3846,20 @@ federation_sender_instances: ### `instance_map` When using workers this should be a map from [`worker_name`](#worker_name) to the -HTTP replication listener of the worker, if configured. +HTTP replication listener of the worker, if configured, and to the main process. Each worker declared under [`stream_writers`](../../workers.md#stream-writers) needs a HTTP replication listener, and that listener should be included in the `instance_map`. -(The main process also needs an HTTP replication listener, but it should not be -listed in the `instance_map`.) +The main process also needs an entry on the `instance_map`, and it should be listed under +`main` **if even one other worker exists**. Ensure the port matches with what is declared +inside the `listener` block for a `replication` listener. + Example configuration: ```yaml instance_map: + main: + host: localhost + port: 8030 worker1: host: localhost port: 8034 @@ -3864,7 +3869,8 @@ instance_map: Experimental: When using workers you can define which workers should handle writing to streams such as event persistence and typing notifications. -Any worker specified here must also be in the [`instance_map`](#instance_map). +Any worker specified here must also be in the [`instance_map`](#instance_map), the +exception being the main process. See the list of available streams in the [worker documentation](../../workers.md#stream-writers). @@ -3986,6 +3992,7 @@ worker_name: generic_worker1 ``` --- ### `worker_replication_host` +*Deprecated as of version 1.83.0. Place `host` under `main` entry on the [`instance_map`](usage/configuration/config_documentation.md#instance_map) in your shared yaml configuration instead.* The HTTP replication endpoint that it should talk to on the main Synapse process. The main Synapse process defines this with a `replication` resource in @@ -3997,6 +4004,7 @@ worker_replication_host: 127.0.0.1 ``` --- ### `worker_replication_http_port` +*Deprecated as of version 1.83.0. Place `port` under `main` entry on the [`instance_map`](usage/configuration/config_documentation.md#instance_map) in your shared yaml configuration instead.* The HTTP replication port that it should talk to on the main Synapse process. The main Synapse process defines this with a `replication` resource in @@ -4008,6 +4016,7 @@ worker_replication_http_port: 9093 ``` --- ### `worker_replication_http_tls` +*Deprecated as of version 1.83.0. Place `tls` under `main` entry on the [`instance_map`](usage/configuration/config_documentation.md#instance_map) in your shared yaml configuration instead.* Whether TLS should be used for talking to the HTTP replication port on the main Synapse process. @@ -4033,9 +4042,9 @@ A worker can handle HTTP requests. To do so, a `worker_listeners` option must be declared, in the same way as the [`listeners` option](#listeners) in the shared config. -Workers declared in [`stream_writers`](#stream_writers) will need to include a -`replication` listener here, in order to accept internal HTTP requests from -other workers. +Workers declared in [`stream_writers`](#stream_writers) and [`instance_map`](usage/configuration/config_documentation.md#instance_map) + will need to include a `replication` listener here, in order to accept internal HTTP +requests from other workers. Example configuration: ```yaml diff --git a/docs/workers.md b/docs/workers.md index 6192a46e0950..a5bc1da5d650 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -355,11 +355,14 @@ effects of bursts of events from that bridge on events sent by normal users. Additionally, the writing of specific streams (such as events) can be moved off of the main process to a particular worker. -To enable this, the worker must have a -[HTTP `replication` listener](usage/configuration/config_documentation.md#listeners) configured, -have a [`worker_name`](usage/configuration/config_documentation.md#worker_name) +To enable this, the worker must have: +* An [HTTP `replication` listener](usage/configuration/config_documentation.md#listeners) configured, +* Have a [`worker_name`](usage/configuration/config_documentation.md#worker_name) and be listed in the [`instance_map`](usage/configuration/config_documentation.md#instance_map) -config. The same worker can handle multiple streams, but unless otherwise documented, +config. +* Have the main process declared on the [`instance_map`](usage/configuration/config_documentation.md#instance_map) as well. + +Note: The same worker can handle multiple streams, but unless otherwise documented, each stream can only have a single writer. For example, to move event persistence off to a dedicated worker, the shared @@ -367,6 +370,9 @@ configuration would include: ```yaml instance_map: + main: + host: localhost + port: 8030 event_persister1: host: localhost port: 8034 From 76928cad654f1238657caf461783eb2ba1c15a5f Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 26 Apr 2023 04:50:52 -0500 Subject: [PATCH 06/15] Changelog --- changelog.d/15491.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/15491.misc diff --git a/changelog.d/15491.misc b/changelog.d/15491.misc new file mode 100644 index 000000000000..98f88dbf19b2 --- /dev/null +++ b/changelog.d/15491.misc @@ -0,0 +1 @@ +Remove need for `worker_replication_*` based settings in worker configuration yaml by placing this data directly on the `instance_map` instead. From 940514d9f8daba03ce588e7c31cb60ce9763fd35 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 26 Apr 2023 05:00:32 -0500 Subject: [PATCH 07/15] Missed some commented out code that can go now --- synapse/replication/http/_base.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/synapse/replication/http/_base.py b/synapse/replication/http/_base.py index a9df214fc6d3..41e988f637a6 100644 --- a/synapse/replication/http/_base.py +++ b/synapse/replication/http/_base.py @@ -198,11 +198,6 @@ def make_client(cls, hs: "HomeServer") -> Callable: client = hs.get_simple_http_client() local_instance_name = hs.get_instance_name() - # The value of these option should match the replication listener settings - # master_host = hs.config.worker.worker_replication_host - # master_port = hs.config.worker.worker_replication_http_port - # master_tls = hs.config.worker.worker_replication_http_tls - instance_map = hs.config.worker.instance_map outgoing_gauge = _pending_outgoing_requests.labels(cls.NAME) From a78a82bb0d8b41aaa76d1d4b7c1735b004806086 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 26 Apr 2023 05:06:29 -0500 Subject: [PATCH 08/15] Remove TODO comment that no longer holds true. --- synapse/config/workers.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/synapse/config/workers.py b/synapse/config/workers.py index 99fd42396595..c0d09dcabc4c 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -222,8 +222,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # itself doesn't need this data as it would never have to talk to itself. instance_map: Dict[str, Any] = config.get("instance_map", {}) - # TODO: In the interest of not using 'master' as the name in the instance_map, - # opt to use 'main' instead. Or perhaps 'home'? if instance_map and self.instance_name is not MAIN_PROCESS_INSTANCE_NAME: # The host used to connect to the main synapse main_host = config.get("worker_replication_host", None) From 7dcf949a7c8a8ef0ff26f77820f04d942ea2c913 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 26 Apr 2023 05:14:59 -0500 Subject: [PATCH 09/15] Fix links in docs --- docs/usage/configuration/config_documentation.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 058f896a79ad..f3a5ba8a08fb 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3992,7 +3992,7 @@ worker_name: generic_worker1 ``` --- ### `worker_replication_host` -*Deprecated as of version 1.83.0. Place `host` under `main` entry on the [`instance_map`](usage/configuration/config_documentation.md#instance_map) in your shared yaml configuration instead.* +*Deprecated as of version 1.83.0. Place `host` under `main` entry on the [`instance_map`](#instance_map) in your shared yaml configuration instead.* The HTTP replication endpoint that it should talk to on the main Synapse process. The main Synapse process defines this with a `replication` resource in @@ -4004,7 +4004,7 @@ worker_replication_host: 127.0.0.1 ``` --- ### `worker_replication_http_port` -*Deprecated as of version 1.83.0. Place `port` under `main` entry on the [`instance_map`](usage/configuration/config_documentation.md#instance_map) in your shared yaml configuration instead.* +*Deprecated as of version 1.83.0. Place `port` under `main` entry on the [`instance_map`](#instance_map) in your shared yaml configuration instead.* The HTTP replication port that it should talk to on the main Synapse process. The main Synapse process defines this with a `replication` resource in @@ -4016,7 +4016,7 @@ worker_replication_http_port: 9093 ``` --- ### `worker_replication_http_tls` -*Deprecated as of version 1.83.0. Place `tls` under `main` entry on the [`instance_map`](usage/configuration/config_documentation.md#instance_map) in your shared yaml configuration instead.* +*Deprecated as of version 1.83.0. Place `tls` under `main` entry on the [`instance_map`](#instance_map) in your shared yaml configuration instead.* Whether TLS should be used for talking to the HTTP replication port on the main Synapse process. @@ -4042,7 +4042,7 @@ A worker can handle HTTP requests. To do so, a `worker_listeners` option must be declared, in the same way as the [`listeners` option](#listeners) in the shared config. -Workers declared in [`stream_writers`](#stream_writers) and [`instance_map`](usage/configuration/config_documentation.md#instance_map) +Workers declared in [`stream_writers`](#stream_writers) and [`instance_map`](#instance_map) will need to include a `replication` listener here, in order to accept internal HTTP requests from other workers. From dec47e7fe9140ae14633d9c1a246712385e7ece5 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 26 Apr 2023 15:02:02 -0500 Subject: [PATCH 10/15] More docs --- docs/workers.md | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/docs/workers.md b/docs/workers.md index a5bc1da5d650..2a28bd47f7cb 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -87,12 +87,18 @@ shared configuration file. ### Shared configuration -Normally, only a couple of changes are needed to make an existing configuration -file suitable for use with workers. First, you need to enable an +Normally, only a few changes are needed to make an existing configuration +file suitable for use with workers: +* First, you need to enable an ["HTTP replication listener"](usage/configuration/config_documentation.md#listeners) -for the main process; and secondly, you need to enable -[redis-based replication](usage/configuration/config_documentation.md#redis). -Optionally, a [shared secret](usage/configuration/config_documentation.md#worker_replication_secret) +for the main process +* Secondly, you need to enable +[redis-based replication](usage/configuration/config_documentation.md#redis) +* You will need to add an [`instance_map`](usage/configuration/config_documentation.md#instance_map) +with the `main` process defined, as well as the relevant connection information from +it's HTTP`replication` listener(defined in step 1 above). Note that the `host` defined +is the address the worker needs to look for the `main` process at, not necessarily the same address that is bound to. +* Optionally, a [shared secret](usage/configuration/config_documentation.md#worker_replication_secret) can be used to authenticate HTTP traffic between workers. For example: ```yaml @@ -111,6 +117,11 @@ worker_replication_secret: "" redis: enabled: true + +instance_map: + main: + host: 'localhost' + port: 9093 ``` See the [configuration manual](usage/configuration/config_documentation.md) @@ -130,13 +141,13 @@ In the config file for each worker, you must specify: * The type of worker ([`worker_app`](usage/configuration/config_documentation.md#worker_app)). The currently available worker applications are listed [below](#available-worker-applications). * A unique name for the worker ([`worker_name`](usage/configuration/config_documentation.md#worker_name)). - * The HTTP replication endpoint that it should talk to on the main synapse process - ([`worker_replication_host`](usage/configuration/config_documentation.md#worker_replication_host) and - [`worker_replication_http_port`](usage/configuration/config_documentation.md#worker_replication_http_port)). * If handling HTTP requests, a [`worker_listeners`](usage/configuration/config_documentation.md#worker_listeners) option with an `http` listener. * **Synapse 1.72 and older:** if handling the `^/_matrix/client/v3/keys/upload` endpoint, the HTTP URI for the main process (`worker_main_http_uri`). This config option is no longer required and is ignored when running Synapse 1.73 and newer. + * **Synapse 1.82 and older:** The HTTP replication endpoint that it should talk to on the main synapse process + ([`worker_replication_host`](usage/configuration/config_documentation.md#worker_replication_host) and + [`worker_replication_http_port`](usage/configuration/config_documentation.md#worker_replication_http_port)). If using Synapse 1.83 and newer, these are not needed if `main` is defined on the [shared configuration](#shared-configuration) `instance_map` For example: From 7655e97caed33334ecc0b5dbf3d5022e09a217ad Mon Sep 17 00:00:00 2001 From: Jason Little Date: Tue, 2 May 2023 06:28:44 -0500 Subject: [PATCH 11/15] Remove debug logging --- synapse/config/workers.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/synapse/config/workers.py b/synapse/config/workers.py index c0d09dcabc4c..fd05e1224c72 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -240,7 +240,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: for k, v in instance_map[MAIN_PROCESS_INSTANCE_MAP_NAME].items(): instance_map[MAIN_PROCESS_INSTANCE_NAME].setdefault(k, v) del instance_map[MAIN_PROCESS_INSTANCE_MAP_NAME] - logger.error(f"config.workers: {instance_map}") + # This is the backwards compatibility bit that handles the # worker_replication_* bits using setdefault() to not overwrite anything. elif main_host is not None and main_port is not None: @@ -252,7 +252,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: "tls": main_tls, }, ) - logger.error(f"config.workers: {instance_map}") else: # If we've gotten here, it means that the main process is not on the From 34f7454135ca0425c1e28fd9f0da7bd534f8ed0e Mon Sep 17 00:00:00 2001 From: Jason Little Date: Fri, 5 May 2023 15:47:20 -0500 Subject: [PATCH 12/15] Apply suggestions from code review Co-authored-by: reivilibre --- docs/upgrade.md | 2 ++ docs/workers.md | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/upgrade.md b/docs/upgrade.md index d94961e14f49..dc693a7a819d 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -110,6 +110,8 @@ instance_map: Notes: * `tls` is optional but mirrors the functionality of `worker_replication_http_tls` * Ensure these values match up with the `replication` listener declared for the main process. + + # Upgrading to v1.81.0 ## Application service path & authentication deprecations diff --git a/docs/workers.md b/docs/workers.md index 2a28bd47f7cb..c415a0d1ee05 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -96,7 +96,7 @@ for the main process [redis-based replication](usage/configuration/config_documentation.md#redis) * You will need to add an [`instance_map`](usage/configuration/config_documentation.md#instance_map) with the `main` process defined, as well as the relevant connection information from -it's HTTP`replication` listener(defined in step 1 above). Note that the `host` defined +it's HTTP `replication` listener (defined in step 1 above). Note that the `host` defined is the address the worker needs to look for the `main` process at, not necessarily the same address that is bound to. * Optionally, a [shared secret](usage/configuration/config_documentation.md#worker_replication_secret) can be used to authenticate HTTP traffic between workers. For example: From a0ff5c4ee058463cf36a115655d1dd5556e1cd67 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 10 May 2023 04:55:55 -0500 Subject: [PATCH 13/15] Apply suggestions from code review Co-authored-by: reivilibre --- docs/upgrade.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/upgrade.md b/docs/upgrade.md index dc693a7a819d..3115eb4ff244 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -97,7 +97,8 @@ When using workers, * `worker_replication_http_port` * `worker_replication_http_tls` -can now be removed from individual worker yaml if you add the main process to the `instance_map` in the shared yaml configuration. +can now be removed from individual worker YAML configuration if you add the main process to the `instance_map` in the shared YAML configuration, +using the name `main`. Example: ```yaml From e3f7936f1d2a54a9f242c035262399d626286b36 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 10 May 2023 05:56:08 -0500 Subject: [PATCH 14/15] Update version to latest, include completeish before/after examples in upgrade notes. --- .../workers/generic_worker.yaml | 4 -- docs/upgrade.md | 63 +++++++++++++++++-- .../configuration/config_documentation.md | 6 +- docs/workers.md | 4 +- 4 files changed, 63 insertions(+), 14 deletions(-) diff --git a/docs/systemd-with-workers/workers/generic_worker.yaml b/docs/systemd-with-workers/workers/generic_worker.yaml index a858f99ed1d9..db6436ee6ebb 100644 --- a/docs/systemd-with-workers/workers/generic_worker.yaml +++ b/docs/systemd-with-workers/workers/generic_worker.yaml @@ -1,10 +1,6 @@ worker_app: synapse.app.generic_worker worker_name: generic_worker1 -# The replication listener on the main synapse process. -worker_replication_host: 127.0.0.1 -worker_replication_http_port: 9093 - worker_listeners: - type: http port: 8083 diff --git a/docs/upgrade.md b/docs/upgrade.md index 3115eb4ff244..0625de8afb7f 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -88,29 +88,82 @@ process, for example: dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb ``` -# Upgrading to v1.83.0 +# Upgrading to v1.84.0 -## Worker Yaml setting deprecations +## Deprecation of `worker_replication_*` configuration settings When using workers, * `worker_replication_host` * `worker_replication_http_port` * `worker_replication_http_tls` -can now be removed from individual worker YAML configuration if you add the main process to the `instance_map` in the shared YAML configuration, +can now be removed from individual worker YAML configuration ***if*** you add the main process to the `instance_map` in the shared YAML configuration, using the name `main`. -Example: +### Before: +Shared YAML +```yaml +instance_map: + generic_worker1: + host: localhost + port: 5678 + tls: false +``` +Worker YAML +```yaml +worker_app: synapse.app.generic_worker +worker_name: generic_worker1 + +worker_replication_host: localhost +worker_replication_http_port: 3456 +worker_replication_http_tls: false + +worker_listeners: + - type: http + port: 1234 + resources: + - names: [client, federation] + - type: http + port: 5678 + resources: + - names: [replication] + +worker_log_config: /etc/matrix-synapse/generic-worker-log.yaml +``` +### After: +Shared YAML ```yaml instance_map: main: host: localhost port: 3456 tls: false + generic_worker1: + host: localhost + port: 5678 + tls: false +``` +Worker YAML +```yaml +worker_app: synapse.app.generic_worker +worker_name: generic_worker1 + +worker_listeners: + - type: http + port: 1234 + resources: + - names: [client, federation] + - type: http + port: 5678 + resources: + - names: [replication] + +worker_log_config: /etc/matrix-synapse/generic-worker-log.yaml + ``` Notes: * `tls` is optional but mirrors the functionality of `worker_replication_http_tls` -* Ensure these values match up with the `replication` listener declared for the main process. + # Upgrading to v1.81.0 diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index f3a5ba8a08fb..43ce3485b0e9 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3992,7 +3992,7 @@ worker_name: generic_worker1 ``` --- ### `worker_replication_host` -*Deprecated as of version 1.83.0. Place `host` under `main` entry on the [`instance_map`](#instance_map) in your shared yaml configuration instead.* +*Deprecated as of version 1.84.0. Place `host` under `main` entry on the [`instance_map`](#instance_map) in your shared yaml configuration instead.* The HTTP replication endpoint that it should talk to on the main Synapse process. The main Synapse process defines this with a `replication` resource in @@ -4004,7 +4004,7 @@ worker_replication_host: 127.0.0.1 ``` --- ### `worker_replication_http_port` -*Deprecated as of version 1.83.0. Place `port` under `main` entry on the [`instance_map`](#instance_map) in your shared yaml configuration instead.* +*Deprecated as of version 1.84.0. Place `port` under `main` entry on the [`instance_map`](#instance_map) in your shared yaml configuration instead.* The HTTP replication port that it should talk to on the main Synapse process. The main Synapse process defines this with a `replication` resource in @@ -4016,7 +4016,7 @@ worker_replication_http_port: 9093 ``` --- ### `worker_replication_http_tls` -*Deprecated as of version 1.83.0. Place `tls` under `main` entry on the [`instance_map`](#instance_map) in your shared yaml configuration instead.* +*Deprecated as of version 1.84.0. Place `tls` under `main` entry on the [`instance_map`](#instance_map) in your shared yaml configuration instead.* Whether TLS should be used for talking to the HTTP replication port on the main Synapse process. diff --git a/docs/workers.md b/docs/workers.md index c415a0d1ee05..76e052e8f4f1 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -145,9 +145,9 @@ In the config file for each worker, you must specify: with an `http` listener. * **Synapse 1.72 and older:** if handling the `^/_matrix/client/v3/keys/upload` endpoint, the HTTP URI for the main process (`worker_main_http_uri`). This config option is no longer required and is ignored when running Synapse 1.73 and newer. - * **Synapse 1.82 and older:** The HTTP replication endpoint that it should talk to on the main synapse process + * **Synapse 1.83 and older:** The HTTP replication endpoint that the worker should talk to on the main synapse process ([`worker_replication_host`](usage/configuration/config_documentation.md#worker_replication_host) and - [`worker_replication_http_port`](usage/configuration/config_documentation.md#worker_replication_http_port)). If using Synapse 1.83 and newer, these are not needed if `main` is defined on the [shared configuration](#shared-configuration) `instance_map` + [`worker_replication_http_port`](usage/configuration/config_documentation.md#worker_replication_http_port)). If using Synapse 1.84 and newer, these are not needed if `main` is defined on the [shared configuration](#shared-configuration) `instance_map` For example: From f6c24e49751b5f9790b0686b6a8d36c27745f8eb Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 10 May 2023 06:39:44 -0500 Subject: [PATCH 15/15] Fix up and docs too --- docs/usage/configuration/config_documentation.md | 3 +-- synapse/config/workers.py | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 43ce3485b0e9..9d0729d065d0 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3869,8 +3869,7 @@ instance_map: Experimental: When using workers you can define which workers should handle writing to streams such as event persistence and typing notifications. -Any worker specified here must also be in the [`instance_map`](#instance_map), the -exception being the main process. +Any worker specified here must also be in the [`instance_map`](#instance_map). See the list of available streams in the [worker documentation](../../workers.md#stream-writers). diff --git a/synapse/config/workers.py b/synapse/config/workers.py index fd05e1224c72..d2311cc857bb 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -236,9 +236,9 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # For now, accept 'main' in the instance_map, but the replication system # expects 'master', force that into being until it's changed later. if MAIN_PROCESS_INSTANCE_MAP_NAME in instance_map: - instance_map.setdefault(MAIN_PROCESS_INSTANCE_NAME, {}) - for k, v in instance_map[MAIN_PROCESS_INSTANCE_MAP_NAME].items(): - instance_map[MAIN_PROCESS_INSTANCE_NAME].setdefault(k, v) + instance_map[MAIN_PROCESS_INSTANCE_NAME] = instance_map[ + MAIN_PROCESS_INSTANCE_MAP_NAME + ] del instance_map[MAIN_PROCESS_INSTANCE_MAP_NAME] # This is the backwards compatibility bit that handles the