From e761e8b475e26341d6d26ecc1499233c5f57c7ec Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 30 Aug 2022 08:21:19 -0400 Subject: Clarify documentation about replication traffic. (#13656) It can be authenticated with the worker_replication_secret setting, but is always unencrypted. --- docs/workers.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'docs/workers.md') diff --git a/docs/workers.md b/docs/workers.md index 6969c424d8..dce584972d 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -120,7 +120,10 @@ redis: See the sample config for the full documentation of each option. Under **no circumstances** should the replication listener be exposed to the -public internet; it has no authentication and is unencrypted. +public internet; replication traffic is: + +* always unencrypted +* unauthenticated, unless `worker_replication_secret` is configured ### Worker configuration -- cgit 1.4.1 From 372136d3a8c6cd0d7506ae75b18128476ba368e4 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 30 Aug 2022 18:01:51 +0100 Subject: Remove documentation of legacy `frontend_proxy` worker app (#13645) This has been the same as a generic_worker since #6964, so let's get rid of it. Fixes #3717 --- changelog.d/13645.doc | 1 + .../workers/generic_worker.yaml | 2 + docs/workers.md | 61 +++++++--------------- 3 files changed, 23 insertions(+), 41 deletions(-) create mode 100644 changelog.d/13645.doc (limited to 'docs/workers.md') diff --git a/changelog.d/13645.doc b/changelog.d/13645.doc new file mode 100644 index 0000000000..04c302ec2c --- /dev/null +++ b/changelog.d/13645.doc @@ -0,0 +1 @@ +Remove documentation of legacy `frontend_proxy` worker app. diff --git a/docs/systemd-with-workers/workers/generic_worker.yaml b/docs/systemd-with-workers/workers/generic_worker.yaml index a82f9c161f..6e7b60886e 100644 --- a/docs/systemd-with-workers/workers/generic_worker.yaml +++ b/docs/systemd-with-workers/workers/generic_worker.yaml @@ -5,6 +5,8 @@ worker_name: generic_worker1 worker_replication_host: 127.0.0.1 worker_replication_http_port: 9093 +worker_main_http_uri: http://localhost:8008/ + worker_listeners: - type: http port: 8083 diff --git a/docs/workers.md b/docs/workers.md index dce584972d..176bb1475e 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -117,7 +117,7 @@ redis: enabled: true ``` -See the sample config for the full documentation of each option. +See the [configuration manual](usage/configuration/config_documentation.html) for the full documentation of each option. Under **no circumstances** should the replication listener be exposed to the public internet; replication traffic is: @@ -128,15 +128,15 @@ public internet; replication traffic is: ### Worker configuration -In the config file for each worker, you must specify the type of worker -application (`worker_app`), and you should specify a unique name for the worker -(`worker_name`). The currently available worker applications are listed below. -You must also specify the HTTP replication endpoint that it should talk to on -the main synapse process. `worker_replication_host` should specify the host of -the main synapse and `worker_replication_http_port` should point to the HTTP -replication port. If the worker will handle HTTP requests then the -`worker_listeners` option should be set with a `http` listener, in the same way -as the `listeners` option in the shared config. +In the config file for each worker, you must specify: + * The type of worker (`worker_app`). The currently available worker applications are listed below. + * A unique name for the worker (`worker_name`). + * The HTTP replication endpoint that it should talk to on the main synapse process + (`worker_replication_host` and `worker_replication_http_port`) + * If handling HTTP requests, a `worker_listeners` option with an `http` + listener, in the same way as the `listeners` option in the shared config. + * If handling the `^/_matrix/client/v3/keys/upload` endpoint, the HTTP URI for + the main process (`worker_main_http_uri`). For example: @@ -220,10 +220,12 @@ information. ^/_matrix/client/(api/v1|r0|v3|unstable)/search$ # Encryption requests + # Note that ^/_matrix/client/(r0|v3|unstable)/keys/upload/ requires `worker_main_http_uri` ^/_matrix/client/(r0|v3|unstable)/keys/query$ ^/_matrix/client/(r0|v3|unstable)/keys/changes$ ^/_matrix/client/(r0|v3|unstable)/keys/claim$ ^/_matrix/client/(r0|v3|unstable)/room_keys/ + ^/_matrix/client/(r0|v3|unstable)/keys/upload/ # Registration/login requests ^/_matrix/client/(api/v1|r0|v3|unstable)/login$ @@ -584,39 +586,16 @@ handle it, and are online. If `update_user_directory` is set to `false`, and this worker is not running, the above endpoint may give outdated results. -### `synapse.app.frontend_proxy` - -Proxies some frequently-requested client endpoints to add caching and remove -load from the main synapse. It can handle REST endpoints matching the following -regular expressions: - - ^/_matrix/client/(r0|v3|unstable)/keys/upload - -If `use_presence` is False in the homeserver config, it can also handle REST -endpoints matching the following regular expressions: - - ^/_matrix/client/(api/v1|r0|v3|unstable)/presence/[^/]+/status - -This "stub" presence handler will pass through `GET` request but make the -`PUT` effectively a no-op. - -It will proxy any requests it cannot handle to the main synapse instance. It -must therefore be configured with the location of the main instance, via -the `worker_main_http_uri` setting in the `frontend_proxy` worker configuration -file. For example: - -```yaml -worker_main_http_uri: http://127.0.0.1:8008 -``` - ### Historical apps -*Note:* Historically there used to be more apps, however they have been -amalgamated into a single `synapse.app.generic_worker` app. The remaining apps -are ones that do specific processing unrelated to requests, e.g. the `pusher` -that handles sending out push notifications for new events. The intention is for -all these to be folded into the `generic_worker` app and to use config to define -which processes handle the various proccessing such as push notifications. +The following used to be separate worker application types, but are now +equivalent to `synapse.app.generic_worker`: + + * `synapse.app.client_reader` + * `synapse.app.event_creator` + * `synapse.app.federation_reader` + * `synapse.app.frontend_proxy` + * `synapse.app.synchrotron` ## Migration from old config -- cgit 1.4.1 From 32fc3b7ba4702a0068a82bdd0595e2f426967d4d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 6 Sep 2022 03:50:02 -0400 Subject: Remove configuration options for direct TCP replication. (#13647) Removes the ability to configure legacy direct TCP replication. Workers now require Redis to run. --- .github/workflows/tests.yml | 1 - changelog.d/13647.removal | 1 + docs/upgrade.md | 15 ++++++ docs/usage/configuration/config_documentation.md | 2 - docs/workers.md | 22 ++------- synapse/app/homeserver.py | 11 ----- synapse/config/server.py | 16 +++++-- synapse/config/workers.py | 8 ++-- synapse/replication/tcp/handler.py | 58 +++++++++--------------- tests/app/test_openid_listener.py | 4 +- tests/test_server.py | 2 +- tests/utils.py | 1 - 12 files changed, 63 insertions(+), 78 deletions(-) create mode 100644 changelog.d/13647.removal (limited to 'docs/workers.md') diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 3ce4ffb036..bc1de2893c 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -204,7 +204,6 @@ jobs: POSTGRES: ${{ matrix.job.postgres && 1}} MULTI_POSTGRES: ${{ (matrix.job.postgres == 'multi-postgres') && 1}} WORKERS: ${{ matrix.job.workers && 1 }} - REDIS: 1 BLACKLIST: ${{ matrix.job.workers && 'synapse-blacklist-with-workers' }} TOP: ${{ github.workspace }} diff --git a/changelog.d/13647.removal b/changelog.d/13647.removal new file mode 100644 index 0000000000..0190a65dba --- /dev/null +++ b/changelog.d/13647.removal @@ -0,0 +1 @@ +Remove the ability to use direct TCP replication with workers. Direct TCP replication was deprecated in Synapse v1.18.0. Workers now require using Redis. diff --git a/docs/upgrade.md b/docs/upgrade.md index 422a3da664..c6219d06e8 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -91,6 +91,21 @@ process, for example: # Upgrading to v1.67.0 +## Direct TCP replication is no longer supported: migrate to Redis + +Redis support was added in v1.13.0 with it becoming the recommended method in +v1.18.0. It replaced the old direct TCP connections (which was deprecated as of +v1.18.0) to the main process. With Redis, rather than all the workers connecting +to the main process, all the workers and the main process connect to Redis, +which relays replication commands between processes. This can give a significant +CPU saving on the main process and is a prerequisite for upcoming +performance improvements. + +To migrate to Redis add the [`redis` config](./workers.md#shared-configuration), +and remove the TCP `replication` listener from config of the master and +`worker_replication_port` from worker config. Note that a HTTP listener with a +`replication` resource is still required. + ## Minimum version of Poetry is now v1.2.0 The minimum supported version of poetry is now 1.2. This should only affect diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 396c560822..757957a1d5 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -431,8 +431,6 @@ Sub-options for each listener include: * `metrics`: (see the docs [here](../../metrics-howto.md)), - * `replication`: (deprecated as of Synapse 1.18, see the docs [here](../../workers.md)). - * `tls`: set to true to enable TLS for this listener. Will use the TLS key/cert specified in tls_private_key_path / tls_certificate_path. * `x_forwarded`: Only valid for an 'http' listener. Set to true to use the X-Forwarded-For header as the client IP. Useful when Synapse is diff --git a/docs/workers.md b/docs/workers.md index 176bb1475e..40b1852313 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -32,13 +32,8 @@ stream between all configured Synapse processes. Additionally, processes may make HTTP requests to each other, primarily for operations which need to wait for a reply ─ such as sending an event. -Redis support was added in v1.13.0 with it becoming the recommended method in -v1.18.0. It replaced the old direct TCP connections (which is deprecated as of -v1.18.0) to the main process. With Redis, rather than all the workers connecting -to the main process, all the workers and the main process connect to Redis, -which relays replication commands between processes. This can give a significant -cpu saving on the main process and will be a prerequisite for upcoming -performance improvements. +All the workers and the main process connect to Redis, which relays replication +commands between processes. If Redis support is enabled Synapse will use it as a shared cache, as well as a pub/sub mechanism. @@ -330,7 +325,6 @@ 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. -(This is only supported with Redis-based replication.) To enable this, the worker must have a HTTP replication listener configured, have a `worker_name` and be listed in the `instance_map` config. The same worker @@ -600,15 +594,9 @@ equivalent to `synapse.app.generic_worker`: ## Migration from old config -There are two main independent changes that have been made: introducing Redis -support and merging apps into `synapse.app.generic_worker`. Both these changes -are backwards compatible and so no changes to the config are required, however -server admins are encouraged to plan to migrate to Redis as the old style direct -TCP replication config is deprecated. - -To migrate to Redis add the `redis` config as above, and optionally remove the -TCP `replication` listener from master and `worker_replication_port` from worker -config. +A main change that has occurred is the merging of worker apps into +`synapse.app.generic_worker`. This change is backwards compatible and so no +changes to the config are required. To migrate apps to use `synapse.app.generic_worker` simply update the `worker_app` option in the worker configs, and where worker are started (e.g. diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index e57a926032..883f2fd2ec 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -57,7 +57,6 @@ from synapse.http.site import SynapseSite from synapse.logging.context import LoggingContext from synapse.metrics import METRICS_PREFIX, MetricsResource, RegistryProxy from synapse.replication.http import REPLICATION_PREFIX, ReplicationRestResource -from synapse.replication.tcp.resource import ReplicationStreamProtocolFactory from synapse.rest import ClientRestResource from synapse.rest.admin import AdminRestResource from synapse.rest.health import HealthResource @@ -290,16 +289,6 @@ class SynapseHomeServer(HomeServer): manhole_settings=self.config.server.manhole_settings, manhole_globals={"hs": self}, ) - elif listener.type == "replication": - services = listen_tcp( - listener.bind_addresses, - listener.port, - ReplicationStreamProtocolFactory(self), - ) - for s in services: - self.get_reactor().addSystemEventTrigger( - "before", "shutdown", s.stopListening - ) elif listener.type == "metrics": if not self.config.metrics.enable_metrics: logger.warning( diff --git a/synapse/config/server.py b/synapse/config/server.py index 085fe22c51..c91df636d9 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -36,6 +36,12 @@ from ._util import validate_config logger = logging.Logger(__name__) +DIRECT_TCP_ERROR = """ +Using direct TCP replication for workers is no longer supported. + +Please see https://matrix-org.github.io/synapse/latest/upgrade.html#direct-tcp-replication-is-no-longer-supported-migrate-to-redis +""" + # by default, we attempt to listen on both '::' *and* '0.0.0.0' because some OSes # (Windows, macOS, other BSD/Linux where net.ipv6.bindv6only is set) will only listen # on IPv6 when '::' is set. @@ -165,7 +171,6 @@ KNOWN_LISTENER_TYPES = { "http", "metrics", "manhole", - "replication", } KNOWN_RESOURCES = { @@ -515,7 +520,9 @@ class ServerConfig(Config): ): raise ConfigError("allowed_avatar_mimetypes must be a list") - self.listeners = [parse_listener_def(x) for x in config.get("listeners", [])] + self.listeners = [ + parse_listener_def(i, x) for i, x in enumerate(config.get("listeners", [])) + ] # no_tls is not really supported any more, but let's grandfather it in # here. @@ -880,9 +887,12 @@ def read_gc_thresholds( ) -def parse_listener_def(listener: Any) -> ListenerConfig: +def parse_listener_def(num: int, listener: Any) -> ListenerConfig: """parse a listener config from the config file""" listener_type = listener["type"] + # Raise a helpful error if direct TCP replication is still configured. + if listener_type == "replication": + raise ConfigError(DIRECT_TCP_ERROR, ("listeners", str(num), "type")) port = listener.get("port") if not isinstance(port, int): diff --git a/synapse/config/workers.py b/synapse/config/workers.py index f2716422b5..0fb725dd8f 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -27,7 +27,7 @@ from ._base import ( RoutableShardedWorkerHandlingConfig, ShardedWorkerHandlingConfig, ) -from .server import ListenerConfig, parse_listener_def +from .server import DIRECT_TCP_ERROR, ListenerConfig, parse_listener_def _FEDERATION_SENDER_WITH_SEND_FEDERATION_ENABLED_ERROR = """ The send_federation config option must be disabled in the main @@ -128,7 +128,8 @@ class WorkerConfig(Config): self.worker_app = None self.worker_listeners = [ - parse_listener_def(x) for x in config.get("worker_listeners", []) + parse_listener_def(i, x) + for i, x in enumerate(config.get("worker_listeners", [])) ] self.worker_daemonize = bool(config.get("worker_daemonize")) self.worker_pid_file = config.get("worker_pid_file") @@ -142,7 +143,8 @@ class WorkerConfig(Config): self.worker_replication_host = config.get("worker_replication_host", None) # The port on the main synapse for TCP replication - self.worker_replication_port = config.get("worker_replication_port", None) + 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") diff --git a/synapse/replication/tcp/handler.py b/synapse/replication/tcp/handler.py index e1cbfa50eb..0f166d16aa 100644 --- a/synapse/replication/tcp/handler.py +++ b/synapse/replication/tcp/handler.py @@ -35,7 +35,6 @@ from twisted.internet.protocol import ReconnectingClientFactory from synapse.metrics import LaterGauge from synapse.metrics.background_process_metrics import run_as_background_process -from synapse.replication.tcp.client import DirectTcpReplicationClientFactory from synapse.replication.tcp.commands import ( ClearUserSyncsCommand, Command, @@ -332,46 +331,31 @@ class ReplicationCommandHandler: def start_replication(self, hs: "HomeServer") -> None: """Helper method to start replication.""" - if hs.config.redis.redis_enabled: - from synapse.replication.tcp.redis import ( - RedisDirectTcpReplicationClientFactory, - ) + from synapse.replication.tcp.redis import RedisDirectTcpReplicationClientFactory - # First let's ensure that we have a ReplicationStreamer started. - hs.get_replication_streamer() + # First let's ensure that we have a ReplicationStreamer started. + hs.get_replication_streamer() - # We need two connections to redis, one for the subscription stream and - # one to send commands to (as you can't send further redis commands to a - # connection after SUBSCRIBE is called). + # We need two connections to redis, one for the subscription stream and + # one to send commands to (as you can't send further redis commands to a + # connection after SUBSCRIBE is called). - # First create the connection for sending commands. - outbound_redis_connection = hs.get_outbound_redis_connection() + # First create the connection for sending commands. + outbound_redis_connection = hs.get_outbound_redis_connection() - # Now create the factory/connection for the subscription stream. - self._factory = RedisDirectTcpReplicationClientFactory( - hs, - outbound_redis_connection, - channel_names=self._channels_to_subscribe_to, - ) - hs.get_reactor().connectTCP( - hs.config.redis.redis_host, - hs.config.redis.redis_port, - self._factory, - timeout=30, - bindAddress=None, - ) - else: - client_name = hs.get_instance_name() - self._factory = DirectTcpReplicationClientFactory(hs, client_name, self) - host = hs.config.worker.worker_replication_host - port = hs.config.worker.worker_replication_port - hs.get_reactor().connectTCP( - host, - port, - self._factory, - timeout=30, - bindAddress=None, - ) + # Now create the factory/connection for the subscription stream. + self._factory = RedisDirectTcpReplicationClientFactory( + hs, + outbound_redis_connection, + channel_names=self._channels_to_subscribe_to, + ) + hs.get_reactor().connectTCP( + hs.config.redis.redis_host, + hs.config.redis.redis_port, + self._factory, + timeout=30, + bindAddress=None, + ) def get_streams(self) -> Dict[str, Stream]: """Get a map from stream name to all streams.""" diff --git a/tests/app/test_openid_listener.py b/tests/app/test_openid_listener.py index 264e101082..c7dae58eb5 100644 --- a/tests/app/test_openid_listener.py +++ b/tests/app/test_openid_listener.py @@ -61,7 +61,7 @@ class FederationReaderOpenIDListenerTests(HomeserverTestCase): } # Listen with the config - self.hs._listen_http(parse_listener_def(config)) + self.hs._listen_http(parse_listener_def(0, config)) # Grab the resource from the site that was told to listen site = self.reactor.tcpServers[0][1] @@ -109,7 +109,7 @@ class SynapseHomeserverOpenIDListenerTests(HomeserverTestCase): } # Listen with the config - self.hs._listener_http(self.hs.config, parse_listener_def(config)) + self.hs._listener_http(self.hs.config, parse_listener_def(0, config)) # Grab the resource from the site that was told to listen site = self.reactor.tcpServers[0][1] diff --git a/tests/test_server.py b/tests/test_server.py index 23975d59c3..7c66448245 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -228,7 +228,7 @@ class OptionsResourceTests(unittest.TestCase): site = SynapseSite( "test", "site_tag", - parse_listener_def({"type": "http", "port": 0}), + parse_listener_def(0, {"type": "http", "port": 0}), self.resource, "1.0", max_request_body_size=4096, diff --git a/tests/utils.py b/tests/utils.py index d2c6d1e852..65db437697 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -135,7 +135,6 @@ def default_config( "enable_registration_captcha": False, "macaroon_secret_key": "not even a little secret", "password_providers": [], - "worker_replication_url": "", "worker_app": None, "block_non_admin_invites": False, "federation_domain_whitelist": None, -- cgit 1.4.1