From 214b7a46edff18f443e5da987491b96ac3fd5d27 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 24 May 2023 19:37:21 -0500 Subject: [PATCH 01/37] Fixup how the HTTPConnectionPool key is declared and comments around/about it(including some spoilers) --- synapse/http/replicationagent.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/synapse/http/replicationagent.py b/synapse/http/replicationagent.py index 800f21873d66..6a12023f8da3 100644 --- a/synapse/http/replicationagent.py +++ b/synapse/http/replicationagent.py @@ -142,9 +142,10 @@ def request( This is copied from twisted.web.client.Agent, except: - * It uses a different pool key (combining the host & port). - * It does not call _ensureValidURI(...) since it breaks on some - UNIX paths. + * It uses a different pool key (combining the scheme with either host & port or + socket path). + * It does not call _ensureValidURI(...) since it breaks when using a worker's + name as a 'hostname'. See: twisted.web.iweb.IAgent.request """ @@ -154,9 +155,12 @@ def request( except SchemeNotSupported: return defer.fail(Failure()) + worker_name = parsedURI.netloc.decode("utf-8") + key_scheme = self._endpointFactory.instance_map[worker_name].scheme() + key_netloc = self._endpointFactory.instance_map[worker_name].netloc() # This sets the Pool key to be: - # (http(s), ) - key = (parsedURI.scheme, parsedURI.netloc) + # (http(s), ) or (unix, ) + key = (key_scheme, key_netloc) # _requestWithEndpoint comes from _AgentBase class return self._requestWithEndpoint( From b41435a1e54042ee4aaaf058a9c5b4df0c0153ab Mon Sep 17 00:00:00 2001 From: Jason Little Date: Thu, 25 May 2023 19:57:16 -0500 Subject: [PATCH 02/37] Drive by fix: remove the disambiguity of seeing 'master' instead of 'main' when referencing the instance_map. --- synapse/replication/http/_base.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/synapse/replication/http/_base.py b/synapse/replication/http/_base.py index 63cf24a14d94..6e6f4ff262c2 100644 --- a/synapse/replication/http/_base.py +++ b/synapse/replication/http/_base.py @@ -220,6 +220,9 @@ async def send_request( if instance_name == local_instance_name: raise Exception("Trying to send HTTP request to self") if instance_name not in instance_map: + # TODO: remove below condition after master officially becomes main + if instance_name == "master": + instance_name = "main" raise Exception( "Instance %r not in 'instance_map' config" % (instance_name,) ) From ca21dce739b5e349710ac955f0f795a13c01862a Mon Sep 17 00:00:00 2001 From: Jason Little Date: Thu, 25 May 2023 20:09:21 -0500 Subject: [PATCH 03/37] Create new InstanceUNIXLocationConfig, and finish wiring up UNIX Socket support. --- synapse/config/workers.py | 19 ++++++++++++++++++- synapse/http/replicationagent.py | 29 ++++++++++++++++++++--------- tests/replication/_base.py | 3 ++- 3 files changed, 40 insertions(+), 11 deletions(-) diff --git a/synapse/config/workers.py b/synapse/config/workers.py index 38e13dd7b5e0..86463cd70ab8 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -88,7 +88,7 @@ class Config: allow_mutation = False -class InstanceLocationConfig(ConfigModel): +class InstanceTCPLocationConfig(ConfigModel): """The host and port to talk to an instance via HTTP replication.""" host: StrictStr @@ -104,6 +104,23 @@ def netloc(self) -> str: return f"{self.host}:{self.port}" +class InstanceUNIXLocationConfig(ConfigModel): + """The socket file to talk to an instance via HTTP replication.""" + + path: StrictStr + + def scheme(self) -> str: + """Hardcode a retrievable scheme""" + return "unix" + + def netloc(self) -> str: + """Nicely format the address location data""" + return f"{self.path}" + + +InstanceLocationConfig = Union[InstanceTCPLocationConfig, InstanceUNIXLocationConfig] + + @attr.s class WriterLocations: """Specifies the instances that write various streams. diff --git a/synapse/http/replicationagent.py b/synapse/http/replicationagent.py index 6a12023f8da3..c5c005a2d782 100644 --- a/synapse/http/replicationagent.py +++ b/synapse/http/replicationagent.py @@ -18,7 +18,11 @@ from zope.interface import implementer from twisted.internet import defer -from twisted.internet.endpoints import HostnameEndpoint, wrapClientTLS +from twisted.internet.endpoints import ( + HostnameEndpoint, + UNIXClientEndpoint, + wrapClientTLS, +) from twisted.internet.interfaces import IStreamClientEndpoint from twisted.python.failure import Failure from twisted.web.client import URI, HTTPConnectionPool, _AgentBase @@ -32,7 +36,11 @@ IResponse, ) -from synapse.config.workers import InstanceLocationConfig +from synapse.config.workers import ( + InstanceLocationConfig, + InstanceTCPLocationConfig, + InstanceUNIXLocationConfig, +) from synapse.types import ISynapseReactor logger = logging.getLogger(__name__) @@ -40,7 +48,7 @@ @implementer(IAgentEndpointFactory) class ReplicationEndpointFactory: - """Connect to a given TCP socket""" + """Connect to a given TCP or UNIX socket""" def __init__( self, @@ -64,24 +72,27 @@ def endpointForURI(self, uri: URI) -> IStreamClientEndpoint: # The given URI has a special scheme and includes the worker name. The # actual connection details are pulled from the instance map. worker_name = uri.netloc.decode("utf-8") - scheme = self.instance_map[worker_name].scheme() + location_config = self.instance_map[worker_name] + scheme = location_config.scheme() - if scheme in ("http", "https"): + if isinstance(location_config, InstanceTCPLocationConfig): endpoint = HostnameEndpoint( self.reactor, - self.instance_map[worker_name].host, - self.instance_map[worker_name].port, + location_config.host, + location_config.port, ) if scheme == "https": endpoint = wrapClientTLS( # The 'port' argument below isn't actually used by the function self.context_factory.creatorForNetloc( - self.instance_map[worker_name].host, - self.instance_map[worker_name].port, + location_config.host, + location_config.port, ), endpoint, ) return endpoint + elif isinstance(location_config, InstanceUNIXLocationConfig): + return UNIXClientEndpoint(self.reactor, location_config.path) else: raise SchemeNotSupported(f"Unsupported scheme: {scheme}") diff --git a/tests/replication/_base.py b/tests/replication/_base.py index eb9b1f1cd9be..cbce4d39abec 100644 --- a/tests/replication/_base.py +++ b/tests/replication/_base.py @@ -22,6 +22,7 @@ from twisted.web.resource import Resource from synapse.app.generic_worker import GenericWorkerServer +from synapse.config.workers import InstanceTCPLocationConfig from synapse.http.site import SynapseRequest, SynapseSite from synapse.replication.http import ReplicationRestResource from synapse.replication.tcp.client import ReplicationDataHandler @@ -339,7 +340,7 @@ def make_worker_hs( # `_handle_http_replication_attempt` like we do with the master HS. instance_name = worker_hs.get_instance_name() instance_loc = worker_hs.config.worker.instance_map.get(instance_name) - if instance_loc: + if instance_loc and isinstance(instance_loc, InstanceTCPLocationConfig): # Ensure the host is one that has a fake DNS entry. if instance_loc.host not in self.reactor.lookups: raise Exception( From e75b177ce1c70a0b644b4de4c9d22c53181ea35a Mon Sep 17 00:00:00 2001 From: Jason Little Date: Fri, 2 Jun 2023 04:57:00 -0500 Subject: [PATCH 04/37] Changelog --- changelog.d/15708.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/15708.feature diff --git a/changelog.d/15708.feature b/changelog.d/15708.feature new file mode 100644 index 000000000000..65fd857f6497 --- /dev/null +++ b/changelog.d/15708.feature @@ -0,0 +1 @@ +Add Unix Socket support for HTTP Replication Listeners. Contributed by Jason Little. From 7d03b2d3f17b2ac496ac48d7511dcb66f31c5d8c Mon Sep 17 00:00:00 2001 From: Jason Little Date: Sun, 4 Jun 2023 02:43:02 -0500 Subject: [PATCH 05/37] Adapt test_workers to handle Class rename --- tests/config/test_workers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/config/test_workers.py b/tests/config/test_workers.py index 086359fd71a2..f3149077865d 100644 --- a/tests/config/test_workers.py +++ b/tests/config/test_workers.py @@ -17,7 +17,7 @@ from immutabledict import immutabledict from synapse.config import ConfigError -from synapse.config.workers import InstanceLocationConfig, WorkerConfig +from synapse.config.workers import InstanceTCPLocationConfig, WorkerConfig from tests.unittest import TestCase @@ -343,7 +343,7 @@ def test_worker_instance_map_compat(self) -> None: self.assertEqual( worker1_config.instance_map, { - "master": InstanceLocationConfig( + "master": InstanceTCPLocationConfig( host="127.0.0.42", port=1979, tls=False ), }, From 0d79903f106d77624070dfc6cc939d65f99c4d72 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Sun, 4 Jun 2023 02:43:20 -0500 Subject: [PATCH 06/37] [OPTIONAL REVERT] Change Type[Model] to TypeAlias as a Hack I don't know how to correctly Type this, now that there is a Union in the mix and I couldn't pin a good example to show me how it's done properly. This can be reverted to cleanly take it back to how it was before, so it can be fixed correctly. --- synapse/config/_util.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/synapse/config/_util.py b/synapse/config/_util.py index acccca413b52..6c97cd2772dc 100644 --- a/synapse/config/_util.py +++ b/synapse/config/_util.py @@ -11,10 +11,11 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from typing import Any, Dict, Type, TypeVar +from typing import Any, Dict, TypeVar import jsonschema from pydantic import BaseModel, ValidationError, parse_obj_as +from typing_extensions import TypeAlias from synapse.config._base import ConfigError from synapse.types import JsonDict, StrSequence @@ -70,9 +71,11 @@ def json_error_to_config_error( Model = TypeVar("Model", bound=BaseModel) +# Here is where I need help with the Type. I put in TypeAlias because I know it works, +# for what I suspect is the wrong reason. def parse_and_validate_mapping( config: Any, - model_type: Type[Model], + model_type: TypeAlias, ) -> Dict[str, Model]: """Parse `config` as a mapping from strings to a given `Model` type. Args: @@ -86,7 +89,7 @@ def parse_and_validate_mapping( try: # type-ignore: mypy doesn't like constructing `Dict[str, model_type]` because # `model_type` is a runtime variable. Pydantic is fine with this. - instances = parse_obj_as(Dict[str, model_type], config) # type: ignore[valid-type] + instances = parse_obj_as(Dict[str, model_type], config) except ValidationError as e: raise ConfigError(str(e)) from e return instances From 7bc139a247872c5486c8260c1578bf7eced55662 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Sun, 4 Jun 2023 04:28:41 -0500 Subject: [PATCH 07/37] Try writing some docs --- .../configuration/config_documentation.md | 56 ++++++++++++++++++- docs/workers.md | 10 +++- 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 0cf6e075ff11..1263db20cd96 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -462,6 +462,21 @@ See the docs [request log format](../administration/request_log.md). * `additional_resources`: Only valid for an 'http' listener. A map of additional endpoints which should be loaded via dynamic modules. + _Added in Synapse 1.86.0_ + +* UNIX socket support: + * `path`: A path and filename for a UNIX socket. Ensure it is in a readable/writeable + directory. No default + * **Note**: Declaring a `path` is not compatible while also having a `port` + configuration for the same `listener` + * **Note**: `metrics` and `manhole` are not supported as `types` on UNIX socket `listeners` + * The `x_forwarded` option will default to true and may be left out. + * Other options that would not make sense to use with a UNIX socket, such as + `bind_address[es]` and `tls` will be ignored and can be removed. + * `mode`: The file permissions to set on the UNIX socket. Defaults to `666` + * `type: http` is valid (provided `metrics` is not included in the `resources`:`names`) + + Valid resource names are: * `client`: the client-server API (/_matrix/client), and the synapse admin API (/_synapse/admin). Also implies `media` and `static`. @@ -474,7 +489,7 @@ Valid resource names are: * `media`: the media API (/_matrix/media). -* `metrics`: the metrics interface. See [here](../../metrics-howto.md). +* `metrics`: the metrics interface. See [here](../../metrics-howto.md). (Not usable on a UNIX Socket `listener`) * `openid`: OpenID authentication. See [here](../../openid.md). @@ -533,6 +548,21 @@ listeners: bind_addresses: ['::1', '127.0.0.1'] type: manhole ``` +Example configuration #3: +```yaml +listeners: + # UNIX socket listener: for when Synapse is behind a reverse proxy that can utilise such. + # + # Note that x_forwarded will default to true, when using a UNIX socket. Please see + # https://matrix-org.github.io/synapse/latest/reverse_proxy.html. + # + - path: /var/run/synapse/main_public.sock + mode: 660 + type: http + resources: + - names: [client, federation] +``` + --- ### `manhole_settings` @@ -3909,8 +3939,8 @@ 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 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. +`main` **if even one other worker exists**. Ensure the `port` or `path` matches with +what is declared inside the `listener` or `worker_listener` block for a `replication` listener. Example configuration: @@ -3923,6 +3953,14 @@ instance_map: host: localhost port: 8034 ``` +Example configuration(#2, for UNIX sockets): +```yaml +instance_map: + main: + path: /var/run/synapse/main_replication.sock + worker1: + path: /var/run/synapse/worker1_replication.sock +``` --- ### `stream_writers` @@ -4127,6 +4165,18 @@ worker_listeners: resources: - names: [client, federation] ``` +Example configuration(#2, using UNIX sockets with a `replication` listener): +```yaml +worker_listeners: + - type: http + path: /var/run/synapse/worker_public.sock + resources: + - names: [client, federation] + - type: http + path: /var/run/synapse/worker_replication.sock + resources: + - names: [replication] +``` --- ### `worker_manhole` diff --git a/docs/workers.md b/docs/workers.md index 991814c0bc81..0f4af5fb6378 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -95,9 +95,13 @@ 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. +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. + * If you are using a UNIX socket for replication to the `main` process, make sure to + use a `path` to the socket file instead of a `port`(It should match the `replication` + entry in the `listeners` for the `main` process). * Optionally, a [shared secret](usage/configuration/config_documentation.md#worker_replication_secret) can be used to authenticate HTTP traffic between workers. For example: From acd56dbd61323259699d29f1ba215da7fac1145d Mon Sep 17 00:00:00 2001 From: Jason Little Date: Sun, 4 Jun 2023 06:48:32 -0500 Subject: [PATCH 08/37] Experimental testing setup for Unix sockets with Complement. Enable testing Unix sockets by passing UNIX_SOCKETS=1 (yes, it's plural) to the command line when running Complement, or export to the environment ahead of time. I apologize for the liberties I took with Opinions for this implementation. 1. The main process gets two sockets, one for public(client and federation) traffic, and one for private(replication) traffic. These were placed at /run/mainpublic.sock and /run/mainprivate.sock, respectively. 2. Additionally, for workers, the sockets were placed at /run/worker.{port_number}. I would have preferred to name them according to worker_name, but that would have taken a much deeper restructure of the configure_workers_and_start.py than I was prepared to make. This approach still allows them to be sequentially numbered just as the ports are now. 3. Redis and Postgres also get the Unix socket treatment while here. --- docker/conf-workers/nginx.conf.j2 | 4 + docker/conf-workers/shared.yaml.j2 | 3 + docker/conf-workers/supervisord.conf.j2 | 4 + docker/conf-workers/worker.yaml.j2 | 4 + docker/conf/homeserver.yaml | 10 ++- docker/configure_workers_and_start.py | 102 ++++++++++++++++++------ scripts-dev/complement.sh | 4 + 7 files changed, 104 insertions(+), 27 deletions(-) diff --git a/docker/conf-workers/nginx.conf.j2 b/docker/conf-workers/nginx.conf.j2 index 967fc65e798c..3a948ec44dac 100644 --- a/docker/conf-workers/nginx.conf.j2 +++ b/docker/conf-workers/nginx.conf.j2 @@ -35,7 +35,11 @@ server { # Send all other traffic to the main process location ~* ^(\\/_matrix|\\/_synapse) { +{% if using_unix_sockets %} + proxy_pass http://unix:/run/mainpublic.sock; +{% else %} proxy_pass http://localhost:8080; +{% endif %} proxy_set_header X-Forwarded-For $remote_addr; proxy_set_header X-Forwarded-Proto $scheme; proxy_set_header Host $host; diff --git a/docker/conf-workers/shared.yaml.j2 b/docker/conf-workers/shared.yaml.j2 index 92d25386dc34..1dfc60ad1104 100644 --- a/docker/conf-workers/shared.yaml.j2 +++ b/docker/conf-workers/shared.yaml.j2 @@ -6,6 +6,9 @@ {% if enable_redis %} redis: enabled: true + {% if using_unix_sockets %} + path: /tmp/redis.sock + {% endif %} {% endif %} {% if appservice_registrations is not none %} diff --git a/docker/conf-workers/supervisord.conf.j2 b/docker/conf-workers/supervisord.conf.j2 index 9f1e03cfc0a2..da9335805175 100644 --- a/docker/conf-workers/supervisord.conf.j2 +++ b/docker/conf-workers/supervisord.conf.j2 @@ -19,7 +19,11 @@ username=www-data autorestart=true [program:redis] +{% if using_unix_sockets %} +command=/usr/local/bin/prefix-log /usr/local/bin/redis-server --unixsocket /tmp/redis.sock +{% else %} command=/usr/local/bin/prefix-log /usr/local/bin/redis-server +{% endif %} priority=1 stdout_logfile=/dev/stdout stdout_logfile_maxbytes=0 diff --git a/docker/conf-workers/worker.yaml.j2 b/docker/conf-workers/worker.yaml.j2 index 44c6e413cf94..29ec74b4ea04 100644 --- a/docker/conf-workers/worker.yaml.j2 +++ b/docker/conf-workers/worker.yaml.j2 @@ -8,7 +8,11 @@ worker_name: "{{ name }}" worker_listeners: - type: http +{% if using_unix_sockets %} + path: "/run/worker.{{ port }}" +{% else %} port: {{ port }} +{% endif %} {% if listener_resources %} resources: - names: diff --git a/docker/conf/homeserver.yaml b/docker/conf/homeserver.yaml index f10f78a48cd2..f6d90c92e9fb 100644 --- a/docker/conf/homeserver.yaml +++ b/docker/conf/homeserver.yaml @@ -36,12 +36,17 @@ listeners: # Allow configuring in case we want to reverse proxy 8008 # using another process in the same container +{% if SYNAPSE_USE_UNIX_SOCKET %} + # Unix sockets don't care about TLS or IP addresses or ports + - path: '/run/mainpublic.sock' + type: http +{% else %} - port: {{ SYNAPSE_HTTP_PORT or 8008 }} tls: false bind_addresses: ['::'] type: http x_forwarded: false - +{% endif %} resources: - names: [client] compress: true @@ -57,8 +62,11 @@ database: user: "{{ POSTGRES_USER or "synapse" }}" password: "{{ POSTGRES_PASSWORD }}" database: "{{ POSTGRES_DB or "synapse" }}" +{% if not SYNAPSE_USE_UNIX_SOCKET %} +{# Fun fact: if left with no host or port, Synapse looks for the default Unix socket instead. #} host: "{{ POSTGRES_HOST or "db" }}" port: "{{ POSTGRES_PORT or "5432" }}" +{% endif %} cp_min: 5 cp_max: 10 {% else %} diff --git a/docker/configure_workers_and_start.py b/docker/configure_workers_and_start.py index 87a740e3d433..8ceca1dd3081 100755 --- a/docker/configure_workers_and_start.py +++ b/docker/configure_workers_and_start.py @@ -74,6 +74,9 @@ MAIN_PROCESS_INSTANCE_NAME = "main" MAIN_PROCESS_LOCALHOST_ADDRESS = "127.0.0.1" MAIN_PROCESS_REPLICATION_PORT = 9093 +# Obviously, these would only be used with the UNIX socket option +MAIN_PROCESS_UNIX_SOCKET_PUBLIC_PATH = "/run/mainpublic.sock" +MAIN_PROCESS_UNIX_SOCKET_PRIVATE_PATH = "/run/mainprivate.sock" # A simple name used as a placeholder in the WORKERS_CONFIG below. This will be replaced # during processing with the name of the worker. @@ -408,11 +411,15 @@ def add_worker_roles_to_shared_config( ) # Map of stream writer instance names to host/ports combos - instance_map[worker_name] = { - "host": "localhost", - "port": worker_port, - } - + if os.environ.get("SYNAPSE_USE_UNIX_SOCKET", False): + instance_map[worker_name] = { + "path": f"/run/worker.{worker_port}", + } + else: + instance_map[worker_name] = { + "host": "localhost", + "port": worker_port, + } # Update the list of stream writers. It's convenient that the name of the worker # type is the same as the stream to write. Iterate over the whole list in case there # is more than one. @@ -424,10 +431,15 @@ def add_worker_roles_to_shared_config( # Map of stream writer instance names to host/ports combos # For now, all stream writers need http replication ports - instance_map[worker_name] = { - "host": "localhost", - "port": worker_port, - } + if os.environ.get("SYNAPSE_USE_UNIX_SOCKET", False): + instance_map[worker_name] = { + "path": f"/run/worker.{worker_port}", + } + else: + instance_map[worker_name] = { + "host": "localhost", + "port": worker_port, + } def merge_worker_template_configs( @@ -719,17 +731,29 @@ def generate_worker_files( # Note that yaml cares about indentation, so care should be taken to insert lines # into files at the correct indentation below. + # Convenience helper for if using unix sockets instead of host:port + using_unix_sockets = environ.get("SYNAPSE_USE_UNIX_SOCKET", False) # First read the original config file and extract the listeners block. Then we'll # add another listener for replication. Later we'll write out the result to the # shared config file. - listeners = [ - { - "port": MAIN_PROCESS_REPLICATION_PORT, - "bind_address": MAIN_PROCESS_LOCALHOST_ADDRESS, - "type": "http", - "resources": [{"names": ["replication"]}], - } - ] + listeners: List[Any] + if using_unix_sockets: + listeners = [ + { + "path": MAIN_PROCESS_UNIX_SOCKET_PRIVATE_PATH, + "type": "http", + "resources": [{"names": ["replication"]}], + } + ] + else: + listeners = [ + { + "port": MAIN_PROCESS_REPLICATION_PORT, + "bind_address": MAIN_PROCESS_LOCALHOST_ADDRESS, + "type": "http", + "resources": [{"names": ["replication"]}], + } + ] with open(config_path) as file_stream: original_config = yaml.safe_load(file_stream) original_listeners = original_config.get("listeners") @@ -770,7 +794,15 @@ def generate_worker_files( # A list of internal endpoints to healthcheck, starting with the main process # which exists even if no workers do. - healthcheck_urls = ["http://localhost:8080/health"] + # This list ends up being part of the command line to curl, which added unix socket + # support in version 7.40. The scheme and hostname are ignored, the path is not. + if using_unix_sockets: + healthcheck_urls = [ + f"--unix-socket {MAIN_PROCESS_UNIX_SOCKET_PUBLIC_PATH} " + "http://localhost/health" + ] + else: + healthcheck_urls = ["http://localhost:8080/health"] # Get the set of all worker types that we have configured all_worker_types_in_use = set(chain(*requested_worker_types.values())) @@ -807,8 +839,12 @@ def generate_worker_files( # given worker_type needs to stay assigned and not be replaced. worker_config["shared_extra_conf"].update(shared_config) shared_config = worker_config["shared_extra_conf"] - - healthcheck_urls.append("http://localhost:%d/health" % (worker_port,)) + if using_unix_sockets: + healthcheck_urls.append( + f"--unix-socket /run/worker.{worker_port} http://localhost/health" + ) + else: + healthcheck_urls.append("http://localhost:%d/health" % (worker_port,)) # Update the shared config with sharding-related options if necessary add_worker_roles_to_shared_config( @@ -827,6 +863,7 @@ def generate_worker_files( "/conf/workers/{name}.yaml".format(name=worker_name), **worker_config, worker_log_config_filepath=log_config_filepath, + using_unix_sockets=using_unix_sockets, ) # Save this worker's port number to the correct nginx upstreams @@ -847,8 +884,13 @@ def generate_worker_files( nginx_upstream_config = "" for upstream_worker_base_name, upstream_worker_ports in nginx_upstreams.items(): body = "" - for port in upstream_worker_ports: - body += f" server localhost:{port};\n" + if using_unix_sockets: + for port in upstream_worker_ports: + body += f" server unix:/run/worker.{port};\n" + + else: + for port in upstream_worker_ports: + body += f" server localhost:{port};\n" # Add to the list of configured upstreams nginx_upstream_config += NGINX_UPSTREAM_CONFIG_BLOCK.format( @@ -878,10 +920,15 @@ def generate_worker_files( # 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, - } + if using_unix_sockets: + instance_map[MAIN_PROCESS_INSTANCE_NAME] = { + "path": MAIN_PROCESS_UNIX_SOCKET_PRIVATE_PATH, + } + else: + instance_map[MAIN_PROCESS_INSTANCE_NAME] = { + "host": MAIN_PROCESS_LOCALHOST_ADDRESS, + "port": MAIN_PROCESS_REPLICATION_PORT, + } # Shared homeserver config convert( @@ -891,6 +938,7 @@ def generate_worker_files( appservice_registrations=appservice_registrations, enable_redis=workers_in_use, workers_in_use=workers_in_use, + using_unix_sockets=using_unix_sockets, ) # Nginx config @@ -901,6 +949,7 @@ def generate_worker_files( upstream_directives=nginx_upstream_config, tls_cert_path=os.environ.get("SYNAPSE_TLS_CERT"), tls_key_path=os.environ.get("SYNAPSE_TLS_KEY"), + using_unix_sockets=using_unix_sockets, ) # Supervisord config @@ -910,6 +959,7 @@ def generate_worker_files( "/etc/supervisor/supervisord.conf", main_config_path=config_path, enable_redis=workers_in_use, + using_unix_sockets=using_unix_sockets, ) convert( diff --git a/scripts-dev/complement.sh b/scripts-dev/complement.sh index 131f26234ece..7604aaf49804 100755 --- a/scripts-dev/complement.sh +++ b/scripts-dev/complement.sh @@ -257,6 +257,10 @@ if [[ -n "$ASYNCIO_REACTOR" ]]; then export PASS_SYNAPSE_COMPLEMENT_USE_ASYNCIO_REACTOR=true fi +if [[ -n "$UNIX_SOCKETS" ]]; then + # Enable full on Unix socket mode for Synapse, Redis and Postgresql + export PASS_SYNAPSE_USE_UNIX_SOCKET=1 +fi if [[ -n "$SYNAPSE_TEST_LOG_LEVEL" ]]; then # Set the log level to what is desired From e14422fe044f95972d428bff4bbe9e3aba03c96f Mon Sep 17 00:00:00 2001 From: Jason Little Date: Sun, 4 Jun 2023 07:05:35 -0500 Subject: [PATCH 09/37] [REVERT THIS] Enable testing for the Complement CI, each test will use unix sockets --- .github/workflows/tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index cf1899b580e8..cac797d719d4 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -571,6 +571,7 @@ jobs: env: POSTGRES: ${{ (matrix.database == 'Postgres') && 1 || '' }} WORKERS: ${{ (matrix.arrangement == 'workers') && 1 || '' }} + UNIX_SOCKETS: 1 name: Run Complement Tests cargo-test: From eb65470718333478f1eb68c155fab7318cb68fa4 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 14 Jun 2023 16:42:43 -0500 Subject: [PATCH 10/37] Revert "Drive by fix: remove the disambiguity of seeing 'master' instead of 'main' when referencing the instance_map." This reverts commit b41435a1e54042ee4aaaf058a9c5b4df0c0153ab. --- synapse/replication/http/_base.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/synapse/replication/http/_base.py b/synapse/replication/http/_base.py index 6e6f4ff262c2..63cf24a14d94 100644 --- a/synapse/replication/http/_base.py +++ b/synapse/replication/http/_base.py @@ -220,9 +220,6 @@ async def send_request( if instance_name == local_instance_name: raise Exception("Trying to send HTTP request to self") if instance_name not in instance_map: - # TODO: remove below condition after master officially becomes main - if instance_name == "master": - instance_name = "main" raise Exception( "Instance %r not in 'instance_map' config" % (instance_name,) ) From e234ce99896888fb014bcd0eca6ad8df3264a1ac Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 14 Jun 2023 16:55:06 -0500 Subject: [PATCH 11/37] Apply suggestions from code review Co-authored-by: Eric Eastwood --- docker/conf/homeserver.yaml | 2 +- docker/configure_workers_and_start.py | 6 ++++-- docs/usage/configuration/config_documentation.md | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/docker/conf/homeserver.yaml b/docker/conf/homeserver.yaml index f6d90c92e9fb..2f45b6476f07 100644 --- a/docker/conf/homeserver.yaml +++ b/docker/conf/homeserver.yaml @@ -63,7 +63,7 @@ database: password: "{{ POSTGRES_PASSWORD }}" database: "{{ POSTGRES_DB or "synapse" }}" {% if not SYNAPSE_USE_UNIX_SOCKET %} -{# Fun fact: if left with no host or port, Synapse looks for the default Unix socket instead. #} +{# Synapse will use a default unix socket for Postgres when host/port is not specified (behavior from `psycopg2`). #} host: "{{ POSTGRES_HOST or "db" }}" port: "{{ POSTGRES_PORT or "5432" }}" {% endif %} diff --git a/docker/configure_workers_and_start.py b/docker/configure_workers_and_start.py index 8ceca1dd3081..c8354a96f977 100755 --- a/docker/configure_workers_and_start.py +++ b/docker/configure_workers_and_start.py @@ -794,11 +794,13 @@ def generate_worker_files( # A list of internal endpoints to healthcheck, starting with the main process # which exists even if no workers do. - # This list ends up being part of the command line to curl, which added unix socket - # support in version 7.40. The scheme and hostname are ignored, the path is not. + # This list ends up being part of the command line to curl, (curl added support unix sockets + # in version 7.40). if using_unix_sockets: healthcheck_urls = [ f"--unix-socket {MAIN_PROCESS_UNIX_SOCKET_PUBLIC_PATH} " + # The scheme and hostname from the following URL are ignored. + # The only thing that matters is the path `/health` "http://localhost/health" ] else: diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 1263db20cd96..8db148101f48 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -489,7 +489,7 @@ Valid resource names are: * `media`: the media API (/_matrix/media). -* `metrics`: the metrics interface. See [here](../../metrics-howto.md). (Not usable on a UNIX Socket `listener`) +* `metrics`: the metrics interface. See [here](../../metrics-howto.md). (Not compatible with Unix sockets) * `openid`: OpenID authentication. See [here](../../openid.md). From 6a5e7050ab028f4c440827737b8f8fb1cc298518 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 14 Jun 2023 17:07:57 -0500 Subject: [PATCH 12/37] Nit over formatting --- docker/configure_workers_and_start.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker/configure_workers_and_start.py b/docker/configure_workers_and_start.py index c8354a96f977..0bf0e19e8032 100755 --- a/docker/configure_workers_and_start.py +++ b/docker/configure_workers_and_start.py @@ -794,8 +794,8 @@ def generate_worker_files( # A list of internal endpoints to healthcheck, starting with the main process # which exists even if no workers do. - # This list ends up being part of the command line to curl, (curl added support unix sockets - # in version 7.40). + # This list ends up being part of the command line to curl, (curl added support for + # Unix sockets in version 7.40). if using_unix_sockets: healthcheck_urls = [ f"--unix-socket {MAIN_PROCESS_UNIX_SOCKET_PUBLIC_PATH} " From ee55d2a275651cfa657af4bbfabbd1fc53fa63dd Mon Sep 17 00:00:00 2001 From: Jason Little Date: Thu, 15 Jun 2023 06:14:16 -0500 Subject: [PATCH 13/37] Add in error messages to unit test setup to discourage using Unix sockets for *listeners or instance_map --- tests/replication/_base.py | 6 +++++- tests/server.py | 24 +++++++++++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/tests/replication/_base.py b/tests/replication/_base.py index cbce4d39abec..457892553d05 100644 --- a/tests/replication/_base.py +++ b/tests/replication/_base.py @@ -22,7 +22,7 @@ from twisted.web.resource import Resource from synapse.app.generic_worker import GenericWorkerServer -from synapse.config.workers import InstanceTCPLocationConfig +from synapse.config.workers import InstanceTCPLocationConfig, InstanceUNIXLocationConfig from synapse.http.site import SynapseRequest, SynapseSite from synapse.replication.http import ReplicationRestResource from synapse.replication.tcp.client import ReplicationDataHandler @@ -361,6 +361,10 @@ def make_worker_hs( instance_loc.port, lambda: self._handle_http_replication_attempt(worker_hs, port), ) + elif instance_loc and isinstance(instance_loc, InstanceUNIXLocationConfig): + raise Exception( + "Unix sockets are not supported for unit tests at this time." + ) store = worker_hs.get_datastores().main store.db_pool._db_pool = self.database_pool._db_pool diff --git a/tests/server.py b/tests/server.py index a12c3e3b9a09..10869d639151 100644 --- a/tests/server.py +++ b/tests/server.py @@ -53,6 +53,7 @@ IConnector, IConsumer, IHostnameResolver, + IListeningPort, IProducer, IProtocol, IPullProducer, @@ -62,7 +63,7 @@ IResolverSimple, ITransport, ) -from twisted.internet.protocol import ClientFactory, DatagramProtocol +from twisted.internet.protocol import ClientFactory, DatagramProtocol, Factory from twisted.python import threadpool from twisted.python.failure import Failure from twisted.test.proto_helpers import AccumulatingProtocol, MemoryReactorClock @@ -523,6 +524,27 @@ def add_tcp_client_callback( """ self._tcp_callbacks[(host, port)] = callback + def connectUNIX( + self, + address: str, + factory: ClientFactory, + timeout: float = 30, + checkPID: int = 0, + ) -> IConnector: + """Unix sockets are not actually implemented for unit tests, tell them so""" + raise Exception("Unix sockets are not implemented for tests, sorry.") + + def listenUNIX( + self, + address: str, + factory: Factory, + backlog: int = 50, + mode: int = 0o666, + wantPID: int = 0, + ) -> IListeningPort: + """Unix sockets are not actually implemented for unit tests, tell them so""" + raise Exception("Unix sockets are not implemented for tests, sorry") + def connectTCP( self, host: str, From e3d4fb41772411e463d90cb343ae53c2cb97043f Mon Sep 17 00:00:00 2001 From: Jason Little Date: Thu, 15 Jun 2023 06:19:29 -0500 Subject: [PATCH 14/37] Unify various incarnations of main_public.sock(and main_private.sock) to be all the same throughout PR --- docker/conf-workers/nginx.conf.j2 | 2 +- docker/conf/homeserver.yaml | 2 +- docker/configure_workers_and_start.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docker/conf-workers/nginx.conf.j2 b/docker/conf-workers/nginx.conf.j2 index 3a948ec44dac..d1e02af72328 100644 --- a/docker/conf-workers/nginx.conf.j2 +++ b/docker/conf-workers/nginx.conf.j2 @@ -36,7 +36,7 @@ server { # Send all other traffic to the main process location ~* ^(\\/_matrix|\\/_synapse) { {% if using_unix_sockets %} - proxy_pass http://unix:/run/mainpublic.sock; + proxy_pass http://unix:/run/main_public.sock; {% else %} proxy_pass http://localhost:8080; {% endif %} diff --git a/docker/conf/homeserver.yaml b/docker/conf/homeserver.yaml index 2f45b6476f07..c46b955d6353 100644 --- a/docker/conf/homeserver.yaml +++ b/docker/conf/homeserver.yaml @@ -38,7 +38,7 @@ listeners: # using another process in the same container {% if SYNAPSE_USE_UNIX_SOCKET %} # Unix sockets don't care about TLS or IP addresses or ports - - path: '/run/mainpublic.sock' + - path: '/run/main_public.sock' type: http {% else %} - port: {{ SYNAPSE_HTTP_PORT or 8008 }} diff --git a/docker/configure_workers_and_start.py b/docker/configure_workers_and_start.py index 0bf0e19e8032..c1ff874d3a31 100755 --- a/docker/configure_workers_and_start.py +++ b/docker/configure_workers_and_start.py @@ -75,8 +75,8 @@ MAIN_PROCESS_LOCALHOST_ADDRESS = "127.0.0.1" MAIN_PROCESS_REPLICATION_PORT = 9093 # Obviously, these would only be used with the UNIX socket option -MAIN_PROCESS_UNIX_SOCKET_PUBLIC_PATH = "/run/mainpublic.sock" -MAIN_PROCESS_UNIX_SOCKET_PRIVATE_PATH = "/run/mainprivate.sock" +MAIN_PROCESS_UNIX_SOCKET_PUBLIC_PATH = "/run/main_public.sock" +MAIN_PROCESS_UNIX_SOCKET_PRIVATE_PATH = "/run/main_private.sock" # A simple name used as a placeholder in the WORKERS_CONFIG below. This will be replaced # during processing with the name of the worker. From fe18b66f3bcfbfcc2040bd7b47123b558366c559 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Thu, 15 Jun 2023 06:33:15 -0500 Subject: [PATCH 15/37] Add detail to docs about Synapse not creating a directory auto-magically for a Unix socket --- docs/usage/configuration/config_documentation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 8db148101f48..ac2f49384761 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -466,7 +466,7 @@ See the docs [request log format](../administration/request_log.md). * UNIX socket support: * `path`: A path and filename for a UNIX socket. Ensure it is in a readable/writeable - directory. No default + directory and is pre-existing(the directory will not be created). No default * **Note**: Declaring a `path` is not compatible while also having a `port` configuration for the same `listener` * **Note**: `metrics` and `manhole` are not supported as `types` on UNIX socket `listeners` From 9394941679e1928a087ad2a509e611773e1bdf62 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Thu, 15 Jun 2023 06:52:03 -0500 Subject: [PATCH 16/37] Adjust TCP and UNIX to be initial caps instead of all caps(in the scope of this PR only) --- synapse/config/workers.py | 6 +++--- synapse/http/replicationagent.py | 8 ++++---- tests/config/test_workers.py | 4 ++-- tests/replication/_base.py | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/synapse/config/workers.py b/synapse/config/workers.py index 86463cd70ab8..bc4d3a9f619d 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -88,7 +88,7 @@ class Config: allow_mutation = False -class InstanceTCPLocationConfig(ConfigModel): +class InstanceTcpLocationConfig(ConfigModel): """The host and port to talk to an instance via HTTP replication.""" host: StrictStr @@ -104,7 +104,7 @@ def netloc(self) -> str: return f"{self.host}:{self.port}" -class InstanceUNIXLocationConfig(ConfigModel): +class InstanceUnixLocationConfig(ConfigModel): """The socket file to talk to an instance via HTTP replication.""" path: StrictStr @@ -118,7 +118,7 @@ def netloc(self) -> str: return f"{self.path}" -InstanceLocationConfig = Union[InstanceTCPLocationConfig, InstanceUNIXLocationConfig] +InstanceLocationConfig = Union[InstanceTcpLocationConfig, InstanceUnixLocationConfig] @attr.s diff --git a/synapse/http/replicationagent.py b/synapse/http/replicationagent.py index c5c005a2d782..921aee71d1e2 100644 --- a/synapse/http/replicationagent.py +++ b/synapse/http/replicationagent.py @@ -38,8 +38,8 @@ from synapse.config.workers import ( InstanceLocationConfig, - InstanceTCPLocationConfig, - InstanceUNIXLocationConfig, + InstanceTcpLocationConfig, + InstanceUnixLocationConfig, ) from synapse.types import ISynapseReactor @@ -75,7 +75,7 @@ def endpointForURI(self, uri: URI) -> IStreamClientEndpoint: location_config = self.instance_map[worker_name] scheme = location_config.scheme() - if isinstance(location_config, InstanceTCPLocationConfig): + if isinstance(location_config, InstanceTcpLocationConfig): endpoint = HostnameEndpoint( self.reactor, location_config.host, @@ -91,7 +91,7 @@ def endpointForURI(self, uri: URI) -> IStreamClientEndpoint: endpoint, ) return endpoint - elif isinstance(location_config, InstanceUNIXLocationConfig): + elif isinstance(location_config, InstanceUnixLocationConfig): return UNIXClientEndpoint(self.reactor, location_config.path) else: raise SchemeNotSupported(f"Unsupported scheme: {scheme}") diff --git a/tests/config/test_workers.py b/tests/config/test_workers.py index f3149077865d..d149eced81e9 100644 --- a/tests/config/test_workers.py +++ b/tests/config/test_workers.py @@ -17,7 +17,7 @@ from immutabledict import immutabledict from synapse.config import ConfigError -from synapse.config.workers import InstanceTCPLocationConfig, WorkerConfig +from synapse.config.workers import InstanceTcpLocationConfig, WorkerConfig from tests.unittest import TestCase @@ -343,7 +343,7 @@ def test_worker_instance_map_compat(self) -> None: self.assertEqual( worker1_config.instance_map, { - "master": InstanceTCPLocationConfig( + "master": InstanceTcpLocationConfig( host="127.0.0.42", port=1979, tls=False ), }, diff --git a/tests/replication/_base.py b/tests/replication/_base.py index 457892553d05..39aadb9ed5c3 100644 --- a/tests/replication/_base.py +++ b/tests/replication/_base.py @@ -22,7 +22,7 @@ from twisted.web.resource import Resource from synapse.app.generic_worker import GenericWorkerServer -from synapse.config.workers import InstanceTCPLocationConfig, InstanceUNIXLocationConfig +from synapse.config.workers import InstanceTcpLocationConfig, InstanceUnixLocationConfig from synapse.http.site import SynapseRequest, SynapseSite from synapse.replication.http import ReplicationRestResource from synapse.replication.tcp.client import ReplicationDataHandler @@ -340,7 +340,7 @@ def make_worker_hs( # `_handle_http_replication_attempt` like we do with the master HS. instance_name = worker_hs.get_instance_name() instance_loc = worker_hs.config.worker.instance_map.get(instance_name) - if instance_loc and isinstance(instance_loc, InstanceTCPLocationConfig): + if instance_loc and isinstance(instance_loc, InstanceTcpLocationConfig): # Ensure the host is one that has a fake DNS entry. if instance_loc.host not in self.reactor.lookups: raise Exception( @@ -361,7 +361,7 @@ def make_worker_hs( instance_loc.port, lambda: self._handle_http_replication_attempt(worker_hs, port), ) - elif instance_loc and isinstance(instance_loc, InstanceUNIXLocationConfig): + elif instance_loc and isinstance(instance_loc, InstanceUnixLocationConfig): raise Exception( "Unix sockets are not supported for unit tests at this time." ) From 79f63fc482867504003e38fbdb6608ed28e585db Mon Sep 17 00:00:00 2001 From: Jason Little Date: Thu, 15 Jun 2023 16:50:15 -0500 Subject: [PATCH 17/37] Try seeing what the doc renderer will do with 4 #'s for a subheading(for the Added in subsection) --- docs/usage/configuration/config_documentation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index ac2f49384761..2c754cfd4ca2 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -462,7 +462,7 @@ See the docs [request log format](../administration/request_log.md). * `additional_resources`: Only valid for an 'http' listener. A map of additional endpoints which should be loaded via dynamic modules. - _Added in Synapse 1.86.0_ + #### _Added in Synapse 1.86.0_ * UNIX socket support: * `path`: A path and filename for a UNIX socket. Ensure it is in a readable/writeable From 9882e0afdc00cb92f70786ad3807598e230fe93d Mon Sep 17 00:00:00 2001 From: Jason Little Date: Thu, 15 Jun 2023 17:16:34 -0500 Subject: [PATCH 18/37] Apply suggestions from code review Co-authored-by: Eric Eastwood --- docs/usage/configuration/config_documentation.md | 1 - docs/workers.md | 5 ++--- tests/server.py | 4 ++-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 2c754cfd4ca2..8d469b2d53ac 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -557,7 +557,6 @@ listeners: # https://matrix-org.github.io/synapse/latest/reverse_proxy.html. # - path: /var/run/synapse/main_public.sock - mode: 660 type: http resources: - names: [client, federation] diff --git a/docs/workers.md b/docs/workers.md index 0f4af5fb6378..b98557277c8c 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -99,9 +99,8 @@ with the `main` process defined, as well as the relevant connection information 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. - * If you are using a UNIX socket for replication to the `main` process, make sure to - use a `path` to the socket file instead of a `port`(It should match the `replication` - entry in the `listeners` for the `main` process). + * If you are using Unix sockets for the `replication` resource, make sure to + use a `path` to the socket file instead of a `port`. * Optionally, a [shared secret](usage/configuration/config_documentation.md#worker_replication_secret) can be used to authenticate HTTP traffic between workers. For example: diff --git a/tests/server.py b/tests/server.py index 10869d639151..1efccd527ee3 100644 --- a/tests/server.py +++ b/tests/server.py @@ -531,8 +531,8 @@ def connectUNIX( timeout: float = 30, checkPID: int = 0, ) -> IConnector: - """Unix sockets are not actually implemented for unit tests, tell them so""" - raise Exception("Unix sockets are not implemented for tests, sorry.") + """Unix sockets aren't supported for unit tests yet. Make it obvious to any developer trying it out that they will need to do some work before being able to use it in tests.""" + raise Exception("Unix sockets are not implemented for tests yet, sorry.") def listenUNIX( self, From 52f84a2c9124cc8a5bd176fc78c92b3fef42bd03 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Thu, 15 Jun 2023 20:21:37 -0500 Subject: [PATCH 19/37] Fix merge derp that encompasses the TLS replication fix from 15746 --- synapse/http/replicationagent.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/http/replicationagent.py b/synapse/http/replicationagent.py index 17e57446fb87..7998b88cf0ee 100644 --- a/synapse/http/replicationagent.py +++ b/synapse/http/replicationagent.py @@ -85,8 +85,8 @@ def endpointForURI(self, uri: URI) -> IStreamClientEndpoint: endpoint = wrapClientTLS( # The 'port' argument below isn't actually used by the function self.context_factory.creatorForNetloc( - self.instance_map[worker_name].host.encode("utf-8"), - self.instance_map[worker_name].port, + location_config.host.encode("utf-8"), + location_config.port, ), endpoint, ) From 86c5fd5ed98dbeec3f3ffde7b481a513b9d24e09 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Thu, 15 Jun 2023 20:38:32 -0500 Subject: [PATCH 20/37] Fix lack of detail in contributing guide for how to use Unix socket functionality in Complement --- docs/development/contributing_guide.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/development/contributing_guide.md b/docs/development/contributing_guide.md index f5ba55afb7a3..0558e009d2c3 100644 --- a/docs/development/contributing_guide.md +++ b/docs/development/contributing_guide.md @@ -370,6 +370,7 @@ The above will run a monolithic (single-process) Synapse with SQLite as the data See the [worker documentation](../workers.md) for additional information on workers. - Passing `ASYNCIO_REACTOR=1` as an environment variable to use the Twisted asyncio reactor instead of the default one. - Passing `PODMAN=1` will use the [podman](https://podman.io/) container runtime, instead of docker. +- Passing `UNIX_SOCKETS=1` will utilise Unix socket functionality for Synapse, Redis, and Postgres(when applicable). To increase the log level for the tests, set `SYNAPSE_TEST_LOG_LEVEL`, e.g: ```sh From e091ea2df100d3248e333553636faa00d838a6e1 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Fri, 16 Jun 2023 04:58:29 -0500 Subject: [PATCH 21/37] Try and fix comment in ReplicationAgent to be clearer about why not using _ensureValidURI() (and a further drive by fix in the docstring) --- synapse/http/replicationagent.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/synapse/http/replicationagent.py b/synapse/http/replicationagent.py index 7998b88cf0ee..3ba2f22dfdc1 100644 --- a/synapse/http/replicationagent.py +++ b/synapse/http/replicationagent.py @@ -149,14 +149,16 @@ def request( An existing connection from the connection pool may be used or a new one may be created. - Currently, HTTP and HTTPS schemes are supported in uri. + Currently, HTTP, HTTPS and UNIX schemes are supported in uri. This is copied from twisted.web.client.Agent, except: * It uses a different pool key (combining the scheme with either host & port or socket path). - * It does not call _ensureValidURI(...) since it breaks when using a worker's - name as a 'hostname'. + * It does not call _ensureValidURI(...) as the strictness of IDNA2008 is not + required when using a worker's name as a 'hostname' for Synapse HTTP + Replication machinery. Specifically, this allows a range of ascii characters + such as '+' and '_' in hostnames/worker's names. See: twisted.web.iweb.IAgent.request """ From 4946cb0170c4c3162811b3efe3257544eadbe3dc Mon Sep 17 00:00:00 2001 From: Jason Little Date: Fri, 16 Jun 2023 15:53:57 -0500 Subject: [PATCH 22/37] Adjust changelog.d/15708.feature Co-authored-by: Eric Eastwood --- changelog.d/15708.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/15708.feature b/changelog.d/15708.feature index 65fd857f6497..06a6c959ab56 100644 --- a/changelog.d/15708.feature +++ b/changelog.d/15708.feature @@ -1 +1 @@ -Add Unix Socket support for HTTP Replication Listeners. Contributed by Jason Little. +Add Unix Socket support for HTTP Replication Listeners. Document and provide usage instructions for utilizing Unix sockets in Synapse. Contributed by Jason Little. From 0b1a9435bf80810048305eec27b2cfc412418e47 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Fri, 16 Jun 2023 16:23:19 -0500 Subject: [PATCH 23/37] Update docs/usage/configuration/config_documentation.md Co-authored-by: Eric Eastwood --- docs/usage/configuration/config_documentation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 8d469b2d53ac..a3484be112e8 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -551,7 +551,7 @@ listeners: Example configuration #3: ```yaml listeners: - # UNIX socket listener: for when Synapse is behind a reverse proxy that can utilise such. + # Unix socket listener: Ideal for Synapse deployments behind a reverse proxy, offering lightweight interprocess communication without TCP/IP overhead, avoid port conflicts, and providing enhanced security through system file permissions. # # Note that x_forwarded will default to true, when using a UNIX socket. Please see # https://matrix-org.github.io/synapse/latest/reverse_proxy.html. From 39597fcf0bb13d41964cf400a5b6d06992eec926 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Mon, 19 Jun 2023 19:41:40 -0500 Subject: [PATCH 24/37] Update docs a bit more --- .../configuration/config_documentation.md | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index a3484be112e8..87b157f1d8e6 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -462,19 +462,18 @@ See the docs [request log format](../administration/request_log.md). * `additional_resources`: Only valid for an 'http' listener. A map of additional endpoints which should be loaded via dynamic modules. - #### _Added in Synapse 1.86.0_ - -* UNIX socket support: - * `path`: A path and filename for a UNIX socket. Ensure it is in a readable/writeable - directory and is pre-existing(the directory will not be created). No default - * **Note**: Declaring a `path` is not compatible while also having a `port` - configuration for the same `listener` - * **Note**: `metrics` and `manhole` are not supported as `types` on UNIX socket `listeners` - * The `x_forwarded` option will default to true and may be left out. +UNIX socket support(_Added in Synapse 1.87.0_): + * `path`: A path and filename for a Unix socket. Make sure it is located in a + directory with read and write permissions, and that it already exists (the directory + will not be created). Defaults to `None`." + * **Note**: The use of both `path` and `port` options for the same `listener` is not + compatible. + * The `x_forwarded` option defaults to true when using Unix sockets and can be omitted. * Other options that would not make sense to use with a UNIX socket, such as - `bind_address[es]` and `tls` will be ignored and can be removed. + `bind_addresses` and `tls` will be ignored and can be removed. * `mode`: The file permissions to set on the UNIX socket. Defaults to `666` - * `type: http` is valid (provided `metrics` is not included in the `resources`:`names`) + * **Note:** Must be set as `type: http` (does not support `metrics` and `manhole`). + Also make sure that `metrics` is not included in `resources` -> `names` Valid resource names are: From 6dbc4022529e4643c2061604c049965216e8425e Mon Sep 17 00:00:00 2001 From: Jason Little Date: Tue, 20 Jun 2023 19:43:25 -0500 Subject: [PATCH 25/37] Apply suggestions from code review Co-authored-by: Eric Eastwood --- docs/usage/configuration/config_documentation.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 87b157f1d8e6..adfc07dded55 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -462,10 +462,10 @@ See the docs [request log format](../administration/request_log.md). * `additional_resources`: Only valid for an 'http' listener. A map of additional endpoints which should be loaded via dynamic modules. -UNIX socket support(_Added in Synapse 1.87.0_): +Unix socket support (_Added in Synapse 1.87.0_): * `path`: A path and filename for a Unix socket. Make sure it is located in a directory with read and write permissions, and that it already exists (the directory - will not be created). Defaults to `None`." + will not be created). Defaults to `None`. * **Note**: The use of both `path` and `port` options for the same `listener` is not compatible. * The `x_forwarded` option defaults to true when using Unix sockets and can be omitted. From 2f20ddae4991ce5521822baefa5bb8630c9d220f Mon Sep 17 00:00:00 2001 From: Jason Little Date: Tue, 20 Jun 2023 19:50:54 -0500 Subject: [PATCH 26/37] From review: wrap lines(and fix a comment consistency) --- docs/usage/configuration/config_documentation.md | 4 +++- tests/server.py | 12 ++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index adfc07dded55..f105eee7c620 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -550,7 +550,9 @@ listeners: Example configuration #3: ```yaml listeners: - # Unix socket listener: Ideal for Synapse deployments behind a reverse proxy, offering lightweight interprocess communication without TCP/IP overhead, avoid port conflicts, and providing enhanced security through system file permissions. + # Unix socket listener: Ideal for Synapse deployments behind a reverse proxy, offering + # lightweight interprocess communication without TCP/IP overhead, avoid port + # conflicts, and providing enhanced security through system file permissions. # # Note that x_forwarded will default to true, when using a UNIX socket. Please see # https://matrix-org.github.io/synapse/latest/reverse_proxy.html. diff --git a/tests/server.py b/tests/server.py index 1efccd527ee3..c84a524e8ce7 100644 --- a/tests/server.py +++ b/tests/server.py @@ -531,7 +531,11 @@ def connectUNIX( timeout: float = 30, checkPID: int = 0, ) -> IConnector: - """Unix sockets aren't supported for unit tests yet. Make it obvious to any developer trying it out that they will need to do some work before being able to use it in tests.""" + """ + Unix sockets aren't supported for unit tests yet. Make it obvious to any + developer trying it out that they will need to do some work before being able + to use it in tests. + """ raise Exception("Unix sockets are not implemented for tests yet, sorry.") def listenUNIX( @@ -542,7 +546,11 @@ def listenUNIX( mode: int = 0o666, wantPID: int = 0, ) -> IListeningPort: - """Unix sockets are not actually implemented for unit tests, tell them so""" + """ + Unix sockets aren't supported for unit tests yet. Make it obvious to any + developer trying it out that they will need to do some work before being able + to use it in tests. + """ raise Exception("Unix sockets are not implemented for tests, sorry") def connectTCP( From 85eba85a4eb09496ccf7a2c77848f06e0cfe0178 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 21 Jun 2023 17:28:56 -0500 Subject: [PATCH 27/37] unindent entire block --- .../configuration/config_documentation.md | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index f105eee7c620..4e8a98b92fc7 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -463,17 +463,17 @@ See the docs [request log format](../administration/request_log.md). additional endpoints which should be loaded via dynamic modules. Unix socket support (_Added in Synapse 1.87.0_): - * `path`: A path and filename for a Unix socket. Make sure it is located in a - directory with read and write permissions, and that it already exists (the directory - will not be created). Defaults to `None`. - * **Note**: The use of both `path` and `port` options for the same `listener` is not - compatible. - * The `x_forwarded` option defaults to true when using Unix sockets and can be omitted. - * Other options that would not make sense to use with a UNIX socket, such as - `bind_addresses` and `tls` will be ignored and can be removed. - * `mode`: The file permissions to set on the UNIX socket. Defaults to `666` - * **Note:** Must be set as `type: http` (does not support `metrics` and `manhole`). - Also make sure that `metrics` is not included in `resources` -> `names` +* `path`: A path and filename for a Unix socket. Make sure it is located in a + directory with read and write permissions, and that it already exists (the directory + will not be created). Defaults to `None`. + * **Note**: The use of both `path` and `port` options for the same `listener` is not + compatible. + * The `x_forwarded` option defaults to true when using Unix sockets and can be omitted. + * Other options that would not make sense to use with a UNIX socket, such as + `bind_addresses` and `tls` will be ignored and can be removed. +* `mode`: The file permissions to set on the UNIX socket. Defaults to `666` +* **Note:** Must be set as `type: http` (does not support `metrics` and `manhole`). + Also make sure that `metrics` is not included in `resources` -> `names` Valid resource names are: From 4c9de6f6d56f1f1c295e58537993d71921930186 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 5 Jul 2023 12:04:39 +0100 Subject: [PATCH 28/37] Revert to `Type[Model]` to keep type variable constrained; add type ignores to work around mypy deficiency --- synapse/config/_util.py | 6 +++--- synapse/config/workers.py | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/synapse/config/_util.py b/synapse/config/_util.py index 6c97cd2772dc..0de58c00cc26 100644 --- a/synapse/config/_util.py +++ b/synapse/config/_util.py @@ -11,7 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from typing import Any, Dict, TypeVar +from typing import Any, Dict, TypeVar, Type import jsonschema from pydantic import BaseModel, ValidationError, parse_obj_as @@ -75,7 +75,7 @@ def json_error_to_config_error( # for what I suspect is the wrong reason. def parse_and_validate_mapping( config: Any, - model_type: TypeAlias, + model_type: Type[Model], ) -> Dict[str, Model]: """Parse `config` as a mapping from strings to a given `Model` type. Args: @@ -89,7 +89,7 @@ def parse_and_validate_mapping( try: # type-ignore: mypy doesn't like constructing `Dict[str, model_type]` because # `model_type` is a runtime variable. Pydantic is fine with this. - instances = parse_obj_as(Dict[str, model_type], config) + instances = parse_obj_as(Dict[str, model_type], config) # type: ignore except ValidationError as e: raise ConfigError(str(e)) from e return instances diff --git a/synapse/config/workers.py b/synapse/config/workers.py index bc4d3a9f619d..75181aae450d 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -279,9 +279,10 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: % MAIN_PROCESS_INSTANCE_MAP_NAME ) + # type-ignore: the expression `Union[A, B]` is not a Type[Union[A, B]] currently self.instance_map: Dict[ str, InstanceLocationConfig - ] = parse_and_validate_mapping(instance_map, InstanceLocationConfig) + ] = parse_and_validate_mapping(instance_map, InstanceLocationConfig) # type: ignore # Map from type of streams to source, c.f. WriterLocations. writers = config.get("stream_writers") or {} From 67c278af753cfc998b89348cec102d2b978dba9c Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 5 Jul 2023 07:01:39 -0500 Subject: [PATCH 29/37] Fix some nits and some lint needed to be burned off --- synapse/config/_util.py | 5 +---- synapse/config/workers.py | 4 +++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/synapse/config/_util.py b/synapse/config/_util.py index 0de58c00cc26..cc8c2f15ba7c 100644 --- a/synapse/config/_util.py +++ b/synapse/config/_util.py @@ -11,11 +11,10 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from typing import Any, Dict, TypeVar, Type +from typing import Any, Dict, Type, TypeVar import jsonschema from pydantic import BaseModel, ValidationError, parse_obj_as -from typing_extensions import TypeAlias from synapse.config._base import ConfigError from synapse.types import JsonDict, StrSequence @@ -71,8 +70,6 @@ def json_error_to_config_error( Model = TypeVar("Model", bound=BaseModel) -# Here is where I need help with the Type. I put in TypeAlias because I know it works, -# for what I suspect is the wrong reason. def parse_and_validate_mapping( config: Any, model_type: Type[Model], diff --git a/synapse/config/workers.py b/synapse/config/workers.py index 75181aae450d..aab57ae9bc33 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -282,7 +282,9 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # type-ignore: the expression `Union[A, B]` is not a Type[Union[A, B]] currently self.instance_map: Dict[ str, InstanceLocationConfig - ] = parse_and_validate_mapping(instance_map, InstanceLocationConfig) # type: ignore + ] = parse_and_validate_mapping( + instance_map, InstanceLocationConfig # type: ignore + ) # Map from type of streams to source, c.f. WriterLocations. writers = config.get("stream_writers") or {} From a5d500fa46b6bc2062428f0d324b5802e0fc33d1 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Thu, 6 Jul 2023 09:53:05 +0100 Subject: [PATCH 30/37] Use more specific mypy ignores --- synapse/config/_util.py | 2 +- synapse/config/workers.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/config/_util.py b/synapse/config/_util.py index cc8c2f15ba7c..acccca413b52 100644 --- a/synapse/config/_util.py +++ b/synapse/config/_util.py @@ -86,7 +86,7 @@ def parse_and_validate_mapping( try: # type-ignore: mypy doesn't like constructing `Dict[str, model_type]` because # `model_type` is a runtime variable. Pydantic is fine with this. - instances = parse_obj_as(Dict[str, model_type], config) # type: ignore + instances = parse_obj_as(Dict[str, model_type], config) # type: ignore[valid-type] except ValidationError as e: raise ConfigError(str(e)) from e return instances diff --git a/synapse/config/workers.py b/synapse/config/workers.py index aab57ae9bc33..e44de8baed15 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -283,7 +283,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: self.instance_map: Dict[ str, InstanceLocationConfig ] = parse_and_validate_mapping( - instance_map, InstanceLocationConfig # type: ignore + instance_map, InstanceLocationConfig # type: ignore[arg-type] ) # Map from type of streams to source, c.f. WriterLocations. From fdb07e827f66cb0e3a4721e64da7d81d9e410d53 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Thu, 6 Jul 2023 06:23:05 -0500 Subject: [PATCH 31/37] Fixup for new proxyagent --- synapse/http/proxyagent.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/synapse/http/proxyagent.py b/synapse/http/proxyagent.py index 1fa3adbef20c..348fecde5894 100644 --- a/synapse/http/proxyagent.py +++ b/synapse/http/proxyagent.py @@ -42,7 +42,7 @@ from twisted.web.http_headers import Headers from twisted.web.iweb import IAgent, IBodyProducer, IPolicyForHTTPS, IResponse -from synapse.config.workers import InstanceLocationConfig +from synapse.config.workers import InstanceLocationConfig, InstanceTcpLocationConfig from synapse.http import redact_uri from synapse.http.connectproxyclient import HTTPConnectProxyEndpoint, ProxyCredentials from synapse.logging.context import run_in_background @@ -144,20 +144,23 @@ def __init__( if federation_proxies: endpoints = [] for federation_proxy in federation_proxies: - endpoint = HostnameEndpoint( - self.proxy_reactor, - federation_proxy.host, - federation_proxy.port, - ) - - if federation_proxy.tls: - tls_connection_creator = self._policy_for_https.creatorForNetloc( + if isinstance(federation_proxy, InstanceTcpLocationConfig): + endpoint = HostnameEndpoint( + self.proxy_reactor, federation_proxy.host, federation_proxy.port, ) - endpoint = wrapClientTLS(tls_connection_creator, endpoint) - endpoints.append(endpoint) + if federation_proxy.tls: + tls_connection_creator = ( + self._policy_for_https.creatorForNetloc( + federation_proxy.host, + federation_proxy.port, + ) + ) + endpoint = wrapClientTLS(tls_connection_creator, endpoint) + + endpoints.append(endpoint) self._federation_proxy_endpoint = _ProxyEndpoints(endpoints) From cd1579b58a229a72c68bcd42c6912e1cc8464f43 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Thu, 6 Jul 2023 17:14:19 -0500 Subject: [PATCH 32/37] Add Unix socket support to the proxyagent --- synapse/http/proxyagent.py | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/synapse/http/proxyagent.py b/synapse/http/proxyagent.py index 348fecde5894..7431e9e1da68 100644 --- a/synapse/http/proxyagent.py +++ b/synapse/http/proxyagent.py @@ -24,7 +24,11 @@ from zope.interface import implementer from twisted.internet import defer -from twisted.internet.endpoints import HostnameEndpoint, wrapClientTLS +from twisted.internet.endpoints import ( + HostnameEndpoint, + UNIXClientEndpoint, + wrapClientTLS, +) from twisted.internet.interfaces import ( IProtocol, IProtocolFactory, @@ -42,7 +46,11 @@ from twisted.web.http_headers import Headers from twisted.web.iweb import IAgent, IBodyProducer, IPolicyForHTTPS, IResponse -from synapse.config.workers import InstanceLocationConfig, InstanceTcpLocationConfig +from synapse.config.workers import ( + InstanceLocationConfig, + InstanceTcpLocationConfig, + InstanceUnixLocationConfig, +) from synapse.http import redact_uri from synapse.http.connectproxyclient import HTTPConnectProxyEndpoint, ProxyCredentials from synapse.logging.context import run_in_background @@ -142,8 +150,9 @@ def __init__( self._federation_proxy_endpoint: Optional[IStreamClientEndpoint] = None if federation_proxies: - endpoints = [] + endpoints: List[IStreamClientEndpoint] = [] for federation_proxy in federation_proxies: + endpoint: IStreamClientEndpoint if isinstance(federation_proxy, InstanceTcpLocationConfig): endpoint = HostnameEndpoint( self.proxy_reactor, @@ -160,7 +169,18 @@ def __init__( ) endpoint = wrapClientTLS(tls_connection_creator, endpoint) - endpoints.append(endpoint) + elif isinstance(federation_proxy, InstanceUnixLocationConfig): + endpoint = UNIXClientEndpoint( + self.proxy_reactor, federation_proxy.path + ) + + else: + # It is supremely unlikely we ever hit this + raise SchemeNotSupported( + f"Unknown type of Endpoint requested, check {federation_proxy}" + ) + + endpoints.append(endpoint) self._federation_proxy_endpoint = _ProxyEndpoints(endpoints) From 4f079d85b58ec792fbb4ad58c57453332b416518 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Thu, 6 Jul 2023 17:14:47 -0500 Subject: [PATCH 33/37] [REVERT THIS] Hardwire testing the proxy agent into Complement temporarily --- docker/conf-workers/shared.yaml.j2 | 3 +++ docker/configure_workers_and_start.py | 14 +++----------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/docker/conf-workers/shared.yaml.j2 b/docker/conf-workers/shared.yaml.j2 index 1dfc60ad1104..ba0b82b046db 100644 --- a/docker/conf-workers/shared.yaml.j2 +++ b/docker/conf-workers/shared.yaml.j2 @@ -9,6 +9,9 @@ redis: {% if using_unix_sockets %} path: /tmp/redis.sock {% endif %} + +outbound_federation_restricted_to: + - federation_sender1 {% endif %} {% if appservice_registrations is not none %} diff --git a/docker/configure_workers_and_start.py b/docker/configure_workers_and_start.py index dc824038b553..e5aaf6b5aa1c 100755 --- a/docker/configure_workers_and_start.py +++ b/docker/configure_workers_and_start.py @@ -136,7 +136,7 @@ }, "federation_sender": { "app": "synapse.app.generic_worker", - "listener_resources": [], + "listener_resources": ["replication"], "endpoint_patterns": [], "shared_extra_conf": {}, "worker_extra_conf": "", @@ -409,16 +409,6 @@ def add_worker_roles_to_shared_config( worker_name ) - # Map of stream writer instance names to host/ports combos - if os.environ.get("SYNAPSE_USE_UNIX_SOCKET", False): - instance_map[worker_name] = { - "path": f"/run/worker.{worker_port}", - } - else: - instance_map[worker_name] = { - "host": "localhost", - "port": worker_port, - } # Update the list of stream writers. It's convenient that the name of the worker # type is the same as the stream to write. Iterate over the whole list in case there # is more than one. @@ -428,6 +418,8 @@ def add_worker_roles_to_shared_config( worker, [] ).append(worker_name) + # Force adding a replication listener if a worker type is defined as having one + if "replication" in WORKERS_CONFIG[worker].get("listener_resources", []): # Map of stream writer instance names to host/ports combos # For now, all stream writers need http replication ports if os.environ.get("SYNAPSE_USE_UNIX_SOCKET", False): From 9f41f3e56f7b9c600f9da8a26628c7959a9a7d00 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Thu, 6 Jul 2023 19:32:54 -0500 Subject: [PATCH 34/37] Revert "[REVERT THIS] Hardwire testing the proxy agent into Complement temporarily" This reverts commit 4f079d85b58ec792fbb4ad58c57453332b416518. --- docker/conf-workers/shared.yaml.j2 | 3 --- docker/configure_workers_and_start.py | 14 +++++++++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/docker/conf-workers/shared.yaml.j2 b/docker/conf-workers/shared.yaml.j2 index ba0b82b046db..1dfc60ad1104 100644 --- a/docker/conf-workers/shared.yaml.j2 +++ b/docker/conf-workers/shared.yaml.j2 @@ -9,9 +9,6 @@ redis: {% if using_unix_sockets %} path: /tmp/redis.sock {% endif %} - -outbound_federation_restricted_to: - - federation_sender1 {% endif %} {% if appservice_registrations is not none %} diff --git a/docker/configure_workers_and_start.py b/docker/configure_workers_and_start.py index e5aaf6b5aa1c..dc824038b553 100755 --- a/docker/configure_workers_and_start.py +++ b/docker/configure_workers_and_start.py @@ -136,7 +136,7 @@ }, "federation_sender": { "app": "synapse.app.generic_worker", - "listener_resources": ["replication"], + "listener_resources": [], "endpoint_patterns": [], "shared_extra_conf": {}, "worker_extra_conf": "", @@ -409,6 +409,16 @@ def add_worker_roles_to_shared_config( worker_name ) + # Map of stream writer instance names to host/ports combos + if os.environ.get("SYNAPSE_USE_UNIX_SOCKET", False): + instance_map[worker_name] = { + "path": f"/run/worker.{worker_port}", + } + else: + instance_map[worker_name] = { + "host": "localhost", + "port": worker_port, + } # Update the list of stream writers. It's convenient that the name of the worker # type is the same as the stream to write. Iterate over the whole list in case there # is more than one. @@ -418,8 +428,6 @@ def add_worker_roles_to_shared_config( worker, [] ).append(worker_name) - # Force adding a replication listener if a worker type is defined as having one - if "replication" in WORKERS_CONFIG[worker].get("listener_resources", []): # Map of stream writer instance names to host/ports combos # For now, all stream writers need http replication ports if os.environ.get("SYNAPSE_USE_UNIX_SOCKET", False): From 0ae603683fc135395fc882579b3da95074e84bcd Mon Sep 17 00:00:00 2001 From: Jason Little Date: Fri, 7 Jul 2023 03:52:13 -0500 Subject: [PATCH 35/37] Revert "[REVERT THIS] Enable testing for the Complement CI, each test will use unix sockets" This reverts commit e14422fe044f95972d428bff4bbe9e3aba03c96f. --- .github/workflows/tests.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 745ab8df58bd..0a01e8298468 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -571,7 +571,6 @@ jobs: env: POSTGRES: ${{ (matrix.database == 'Postgres') && 1 || '' }} WORKERS: ${{ (matrix.arrangement == 'workers') && 1 || '' }} - UNIX_SOCKETS: 1 name: Run Complement Tests cargo-test: From 92c6c7b31de24e0174d12df241bd6bf713eb5786 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Fri, 7 Jul 2023 03:55:59 -0500 Subject: [PATCH 36/37] Update version number of Synapse this was added in --- docs/usage/configuration/config_documentation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 96894cdbc06c..35b3141e84ef 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -462,7 +462,7 @@ See the docs [request log format](../administration/request_log.md). * `additional_resources`: Only valid for an 'http' listener. A map of additional endpoints which should be loaded via dynamic modules. -Unix socket support (_Added in Synapse 1.87.0_): +Unix socket support (_Added in Synapse 1.88.0_): * `path`: A path and filename for a Unix socket. Make sure it is located in a directory with read and write permissions, and that it already exists (the directory will not be created). Defaults to `None`. From 3789a4f5d06e3a35a6f43a95b126161f5a7b395e Mon Sep 17 00:00:00 2001 From: Jason Little Date: Mon, 10 Jul 2023 18:28:03 -0500 Subject: [PATCH 37/37] Swap getClientAddress().host call for the UNIXAddress compatible replacment, allowing tracing to correctly tag spans. --- synapse/logging/opentracing.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/logging/opentracing.py b/synapse/logging/opentracing.py index 75217e3f45bb..be910128aa4e 100644 --- a/synapse/logging/opentracing.py +++ b/synapse/logging/opentracing.py @@ -1070,7 +1070,7 @@ def trace_servlet( tags.SPAN_KIND: tags.SPAN_KIND_RPC_SERVER, tags.HTTP_METHOD: request.get_method(), tags.HTTP_URL: request.get_redacted_uri(), - tags.PEER_HOST_IPV6: request.getClientAddress().host, + tags.PEER_HOST_IPV6: request.get_client_ip_if_available(), } request_name = request.request_metrics.name @@ -1091,9 +1091,11 @@ def trace_servlet( # with JsonResource). scope.span.set_operation_name(request.request_metrics.name) + # Mypy seems to think that start_context.tag below can be Optional[str], but + # that doesn't appear to be correct and works in practice. request_tags[ SynapseTags.REQUEST_TAG - ] = request.request_metrics.start_context.tag + ] = request.request_metrics.start_context.tag # type: ignore[assignment] # set the tags *after* the servlet completes, in case it decided to # prioritise the span (tags will get dropped on unprioritised spans)