tileradotorg/synapse
1
0
Fork 0
forked from MirrorHub/synapse
Code Pull requests Activity

Add the notify_appservices_from_worker configuration option (superseding notify_appservices) to allow a generic worker to be designated as the worker to send traffic to Application Services. (#12452)

Browse source
This commit is contained in:
reivilibre 2022-05-06 11:43:53 +01:00 committed by GitHub
parent f1fbf75cfc
commit c2d50e9f6c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 447 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
changelog.d/12452.feature Normal file
View file

@ -0,0 +1 @@
Add the `notify_appservices_from_worker` configuration option (superseding `notify_appservices`) to allow a generic worker to be designated as the worker to send traffic to Application Services.

4
docker/configure_workers_and_start.py
View file

@ -69,10 +69,10 @@ WORKERS_CONFIG: Dict[str, Dict[str, Any]] = {
"worker_extra_conf": "enable_media_repo: true",
},
"appservice": {
"app": "synapse.app.appservice",
"app": "synapse.app.generic_worker",
"listener_resources": [],
"endpoint_patterns": [],
"shared_extra_conf": {"notify_appservices": False},
"shared_extra_conf": {"notify_appservices_from_worker": "appservice"},
"worker_extra_conf": "",
},
"federation_sender": {

27
docs/upgrade.md
View file

@ -100,6 +100,32 @@ To re-enable this functionality, set the
[`allow_device_name_lookup_over_federation`](https://matrix-org.github.io/synapse/v1.59/usage/configuration/config_documentation.html#federation)
homeserver config option to `true`.
## Deprecation of the `synapse.app.appservice` worker application type
The `synapse.app.appservice` worker application type allowed you to configure a
single worker to use to notify application services of new events, as long
as this functionality was disabled on the main process with `notify_appservices: False`.
To unify Synapse's worker types, the `synapse.app.appservice` worker application
type and the `notify_appservices` configuration option have been deprecated.
To get the same functionality, it's now recommended that the `synapse.app.generic_worker`
worker application type is used and that the `notify_appservices_from_worker` option
is set to the name of a worker.
For the time being, `notify_appservices_from_worker` can be used alongside
`synapse.app.appservice` and `notify_appservices` to make it easier to transition
between the two configurations, however please note that:
- the options must not contradict each other (otherwise Synapse won't start); and
- the `notify_appservices` option will be removed in a future release of Synapse.
Please see [the relevant section of the worker documentation][v1_59_notify_ases_from] for more information.
[v1_59_notify_ases_from]: workers.md#notifying-application-services
# Upgrading to v1.58.0
## Groups/communities feature has been disabled by default
@ -107,6 +133,7 @@ homeserver config option to `true`.
The non-standard groups/communities feature in Synapse has been disabled by default
and will be removed in Synapse v1.61.0.
# Upgrading to v1.57.0
## Changes to database schema for application services

20
docs/workers.md
View file

@ -435,6 +435,23 @@ An example for a dedicated background worker instance:
{{#include systemd-with-workers/workers/background_worker.yaml}}
```
#### Notifying Application Services
You can designate one worker to send output traffic to Application Services.
Specify its name in the shared configuration as follows:
```yaml
notify_appservices_from_worker: worker_name
```
This work cannot be load-balanced; please ensure the main process is restarted
after setting this option in the shared configuration!
This style of configuration supersedes the legacy `synapse.app.appservice`
worker application type.
### `synapse.app.pusher`
Handles sending push notifications to sygnal and email. Doesn't handle any
@ -453,6 +470,9 @@ pusher_instances:
### `synapse.app.appservice`
**Deprecated as of Synapse v1.58.** [Use `synapse.app.generic_worker` with the
`notify_appservices_from_worker` option instead.](#notifying-application-services)
Handles sending output traffic to Application Services. Doesn't handle any
REST endpoints itself, but you should set `notify_appservices: False` in the
shared configuration file to stop the main synapse sending appservice notifications.

16
synapse/app/generic_worker.py
View file

@ -441,22 +441,6 @@ def start(config_options: List[str]) -> None:
"synapse.app.user_dir",
)
if config.worker.worker_app == "synapse.app.appservice":
if config.appservice.notify_appservices:
sys.stderr.write(
"\nThe appservices must be disabled in the main synapse process"
"\nbefore they can be run in a separate worker."
"\nPlease add ``notify_appservices: false`` to the main config"
"\n"
)
sys.exit(1)
# Force the appservice to start since they will be disabled in the main config
config.appservice.notify_appservices = True
else:
# For other worker types we force this to off.
config.appservice.notify_appservices = False
if config.worker.worker_app == "synapse.app.user_dir":
if config.server.update_user_directory:
sys.stderr.write(

1
synapse/config/appservice.py
View file

@ -33,7 +33,6 @@ class AppServiceConfig(Config):
def read_config(self, config: JsonDict, **kwargs: Any) -> None:
self.app_service_config_files = config.get("app_service_config_files", [])
self.notify_appservices = config.get("notify_appservices", True)
self.track_appservice_user_ips = config.get("track_appservice_user_ips", False)
def generate_config_section(cls, **kwargs: Any) -> str:

109
synapse/config/workers.py
View file

@ -14,7 +14,8 @@
# limitations under the License.
import argparse
from typing import Any, List, Union
import logging
from typing import Any, Dict, List, Union
import attr
@ -42,6 +43,13 @@ synapse process before they can be run in a separate worker.
Please add ``start_pushers: false`` to the main config
"""
_DEPRECATED_WORKER_DUTY_OPTION_USED = """
The '%s' configuration option is deprecated and will be removed in a future
Synapse version. Please use ``%s: name_of_worker`` instead.
"""
logger = logging.getLogger(__name__)
def _instance_to_list_converter(obj: Union[str, List[str]]) -> List[str]:
"""Helper for allowing parsing a string or list of strings to a config
@ -296,6 +304,105 @@ class WorkerConfig(Config):
self.worker_name is None and background_tasks_instance == "master"
) or self.worker_name == background_tasks_instance
self.should_notify_appservices = self._should_this_worker_perform_duty(
config,
legacy_master_option_name="notify_appservices",
legacy_worker_app_name="synapse.app.appservice",
new_option_name="notify_appservices_from_worker",
)
def _should_this_worker_perform_duty(
self,
config: Dict[str, Any],
legacy_master_option_name: str,
legacy_worker_app_name: str,
new_option_name: str,
) -> bool:
"""
Figures out whether this worker should perform a certain duty.
This function is temporary and is only to deal with the complexity
of allowing old, transitional and new configurations all at once.
Contradictions between the legacy and new part of a transitional configuration
will lead to a ConfigError.
Parameters:
config: The config dictionary
legacy_master_option_name: The name of a legacy option, whose value is boolean,
specifying whether it's the master that should handle a certain duty.
e.g. "notify_appservices"
legacy_worker_app_name: The name of a legacy Synapse worker application
that would traditionally perform this duty.
e.g. "synapse.app.appservice"
new_option_name: The name of the new option, whose value is the name of a
designated worker to perform the duty.
e.g. "notify_appservices_from_worker"
"""
# None means 'unspecified'; True means 'run here' and False means
# 'don't run here'.
new_option_should_run_here = None
if new_option_name in config:
designated_worker = config[new_option_name] or "master"
new_option_should_run_here = (
designated_worker == "master" and self.worker_name is None
) or designated_worker == self.worker_name
legacy_option_should_run_here = None
if legacy_master_option_name in config:
run_on_master = bool(config[legacy_master_option_name])
legacy_option_should_run_here = (
self.worker_name is None and run_on_master
) or (self.worker_app == legacy_worker_app_name and not run_on_master)
# Suggest using the new option instead.
logger.warning(
_DEPRECATED_WORKER_DUTY_OPTION_USED,
legacy_master_option_name,
new_option_name,
)
if self.worker_app == legacy_worker_app_name and config.get(
legacy_master_option_name, True
):
# As an extra bit of complication, we need to check that the
# specialised worker is only used if the legacy config says the
# master isn't performing the duties.
raise ConfigError(
f"Cannot use deprecated worker app type '{legacy_worker_app_name}' whilst deprecated option '{legacy_master_option_name}' is not set to false.\n"
f"Consider setting `worker_app: synapse.app.generic_worker` and using the '{new_option_name}' option instead.\n"
f"The '{new_option_name}' option replaces '{legacy_master_option_name}'."
)
if new_option_should_run_here is None and legacy_option_should_run_here is None:
# Neither option specified; the fallback behaviour is to run on the main process
return self.worker_name is None
if (
new_option_should_run_here is not None
and legacy_option_should_run_here is not None
):
# Both options specified; ensure they match!
if new_option_should_run_here != legacy_option_should_run_here:
update_worker_type = (
" and set worker_app: synapse.app.generic_worker"
if self.worker_app == legacy_worker_app_name
else ""
)
# If the values conflict, we suggest the admin removes the legacy option
# for simplicity.
raise ConfigError(
f"Conflicting configuration options: {legacy_master_option_name} (legacy), {new_option_name} (new).\n"
f"Suggestion: remove {legacy_master_option_name}{update_worker_type}.\n"
)
# We've already validated that these aren't conflicting; now just see if
# either is True.
# (By this point, these are either the same value or only one is not None.)
return bool(new_option_should_run_here or legacy_option_should_run_here)
def generate_config_section(self, **kwargs: Any) -> str:
return """\
## Workers ##

2
synapse/handlers/appservice.py
View file

@ -59,7 +59,7 @@ class ApplicationServicesHandler:
self.scheduler = hs.get_application_service_scheduler()
self.started_scheduler = False
self.clock = hs.get_clock()
self.notify_appservices = hs.config.appservice.notify_appservices
self.notify_appservices = hs.config.worker.should_notify_appservices
self.event_sources = hs.get_event_sources()
self._msc2409_to_device_messages_enabled = (
hs.config.experimental.msc2409_to_device_messages_enabled

288
tests/config/test_workers.py Normal file
View file

@ -0,0 +1,288 @@
# Copyright 2022 The Matrix.org Foundation C.I.C.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from typing import Any, Mapping, Optional
from unittest.mock import Mock
from frozendict import frozendict
from synapse.config import ConfigError
from synapse.config.workers import WorkerConfig
from tests.unittest import TestCase
_EMPTY_FROZENDICT: Mapping[str, Any] = frozendict()
class WorkerDutyConfigTestCase(TestCase):
def _make_worker_config(
self,
worker_app: str,
worker_name: Optional[str],
extras: Mapping[str, Any] = _EMPTY_FROZENDICT,
) -> WorkerConfig:
root_config = Mock()
root_config.worker_app = worker_app
root_config.worker_name = worker_name
worker_config = WorkerConfig(root_config)
worker_config_dict = {
"worker_name": worker_name,
"worker_app": worker_app,
**extras,
}
worker_config.read_config(worker_config_dict)
return worker_config
def test_old_configs_master(self) -> None:
"""
Tests old (legacy) config options. This is for the master's config.
"""
main_process_config = self._make_worker_config(
worker_app="synapse.app.homeserver", worker_name=None
)
self.assertTrue(
main_process_config._should_this_worker_perform_duty(
{},
"notify_appservices",
"synapse.app.appservice",
"notify_appservices_from_worker",
)
)
self.assertTrue(
main_process_config._should_this_worker_perform_duty(
{
"notify_appservices": True,
},
"notify_appservices",
"synapse.app.appservice",
"notify_appservices_from_worker",
)
)
self.assertFalse(
main_process_config._should_this_worker_perform_duty(
{
"notify_appservices": False,
},
"notify_appservices",
"synapse.app.appservice",
"notify_appservices_from_worker",
)
)
def test_old_configs_appservice_worker(self) -> None:
"""
Tests old (legacy) config options. This is for the worker's config.
"""
appservice_worker_config = self._make_worker_config(
worker_app="synapse.app.appservice",
worker_name="worker1",
extras={
# Set notify_appservices to false for the initialiser's config,
# so that it doesn't raise an exception here.
# (This is not read by `_should_this_worker_perform_duty`.)
"notify_appservices": False,
},
)
with self.assertRaises(ConfigError):
# This raises because you need to set notify_appservices: False
# before using the synapse.app.appservice worker type
self.assertFalse(
appservice_worker_config._should_this_worker_perform_duty(
{},
"notify_appservices",
"synapse.app.appservice",
"notify_appservices_from_worker",
)
)
with self.assertRaises(ConfigError):
# This also raises because you need to set notify_appservices: False
# before using the synapse.app.appservice worker type
appservice_worker_config._should_this_worker_perform_duty(
{
"notify_appservices": True,
},
"notify_appservices",
"synapse.app.appservice",
"notify_appservices_from_worker",
)
self.assertTrue(
appservice_worker_config._should_this_worker_perform_duty(
{
"notify_appservices": False,
},
"notify_appservices",
"synapse.app.appservice",
"notify_appservices_from_worker",
)
)
def test_transitional_configs_master(self) -> None:
"""
Tests transitional (legacy + new) config options. This is for the master's config.
"""
main_process_config = self._make_worker_config(
worker_app="synapse.app.homeserver", worker_name=None
)
self.assertTrue(
main_process_config._should_this_worker_perform_duty(
{
"notify_appservices": True,
"notify_appservices_from_worker": "master",
},
"notify_appservices",
"synapse.app.appservice",
"notify_appservices_from_worker",
)
)
self.assertFalse(
main_process_config._should_this_worker_perform_duty(
{
"notify_appservices": False,
"notify_appservices_from_worker": "worker1",
},
"notify_appservices",
"synapse.app.appservice",
"notify_appservices_from_worker",
)
)
with self.assertRaises(ConfigError):
# Contradictory because we say the master should notify appservices,
# then we say worker1 is the designated worker to do that!
main_process_config._should_this_worker_perform_duty(
{
"notify_appservices": True,
"notify_appservices_from_worker": "worker1",
},
"notify_appservices",
"synapse.app.appservice",
"notify_appservices_from_worker",
)
with self.assertRaises(ConfigError):
# Contradictory because we say the master shouldn't notify appservices,
# then we say master is the designated worker to do that!
main_process_config._should_this_worker_perform_duty(
{
"notify_appservices": False,
"notify_appservices_from_worker": "master",
},
"notify_appservices",
"synapse.app.appservice",
"notify_appservices_from_worker",
)
def test_transitional_configs_appservice_worker(self) -> None:
"""
Tests transitional (legacy + new) config options. This is for the worker's config.
"""
appservice_worker_config = self._make_worker_config(
worker_app="synapse.app.appservice",
worker_name="worker1",
extras={
# Set notify_appservices to false for the initialiser's config,
# so that it doesn't raise an exception here.
# (This is not read by `_should_this_worker_perform_duty`.)
"notify_appservices": False,
},
)
self.assertTrue(
appservice_worker_config._should_this_worker_perform_duty(
{
"notify_appservices": False,
"notify_appservices_from_worker": "worker1",
},
"notify_appservices",
"synapse.app.appservice",
"notify_appservices_from_worker",
)
)
with self.assertRaises(ConfigError):
# This raises because this worker is the appservice app type, yet
# another worker is the designated worker!
appservice_worker_config._should_this_worker_perform_duty(
{
"notify_appservices": False,
"notify_appservices_from_worker": "worker2",
},
"notify_appservices",
"synapse.app.appservice",
"notify_appservices_from_worker",
)
def test_new_configs_master(self) -> None:
"""
Tests new config options. This is for the master's config.
"""
main_process_config = self._make_worker_config(
worker_app="synapse.app.homeserver", worker_name=None
)
self.assertTrue(
main_process_config._should_this_worker_perform_duty(
{"notify_appservices_from_worker": None},
"notify_appservices",
"synapse.app.appservice",
"notify_appservices_from_worker",
)
)
self.assertFalse(
main_process_config._should_this_worker_perform_duty(
{"notify_appservices_from_worker": "worker1"},
"notify_appservices",
"synapse.app.appservice",
"notify_appservices_from_worker",
)
)
def test_new_configs_appservice_worker(self) -> None:
"""
Tests new config options. This is for the worker's config.
"""
appservice_worker_config = self._make_worker_config(
worker_app="synapse.app.generic_worker", worker_name="worker1"
)
self.assertTrue(
appservice_worker_config._should_this_worker_perform_duty(
{
"notify_appservices_from_worker": "worker1",
},
"notify_appservices",
"synapse.app.appservice",
"notify_appservices_from_worker",
)
)
self.assertFalse(
appservice_worker_config._should_this_worker_perform_duty(
{
"notify_appservices_from_worker": "worker2",
},
"notify_appservices",
"synapse.app.appservice",
"notify_appservices_from_worker",
)
)