Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Commit c2d50e9

Browse files
authored
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)
1 parent f1fbf75 commit c2d50e9

File tree

9 files changed

+447
-21
lines changed

9 files changed

+447
-21
lines changed

changelog.d/12452.feature

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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.

docker/configure_workers_and_start.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,10 @@
6969
"worker_extra_conf": "enable_media_repo: true",
7070
},
7171
"appservice": {
72-
"app": "synapse.app.appservice",
72+
"app": "synapse.app.generic_worker",
7373
"listener_resources": [],
7474
"endpoint_patterns": [],
75-
"shared_extra_conf": {"notify_appservices": False},
75+
"shared_extra_conf": {"notify_appservices_from_worker": "appservice"},
7676
"worker_extra_conf": "",
7777
},
7878
"federation_sender": {

docs/upgrade.md

+27
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,40 @@ To re-enable this functionality, set the
100100
[`allow_device_name_lookup_over_federation`](https://matrix-org.github.io/synapse/v1.59/usage/configuration/config_documentation.html#federation)
101101
homeserver config option to `true`.
102102
103+
104+
## Deprecation of the `synapse.app.appservice` worker application type
105+
106+
The `synapse.app.appservice` worker application type allowed you to configure a
107+
single worker to use to notify application services of new events, as long
108+
as this functionality was disabled on the main process with `notify_appservices: False`.
109+
110+
To unify Synapse's worker types, the `synapse.app.appservice` worker application
111+
type and the `notify_appservices` configuration option have been deprecated.
112+
113+
To get the same functionality, it's now recommended that the `synapse.app.generic_worker`
114+
worker application type is used and that the `notify_appservices_from_worker` option
115+
is set to the name of a worker.
116+
117+
For the time being, `notify_appservices_from_worker` can be used alongside
118+
`synapse.app.appservice` and `notify_appservices` to make it easier to transition
119+
between the two configurations, however please note that:
120+
121+
- the options must not contradict each other (otherwise Synapse won't start); and
122+
- the `notify_appservices` option will be removed in a future release of Synapse.
123+
124+
Please see [the relevant section of the worker documentation][v1_59_notify_ases_from] for more information.
125+
126+
[v1_59_notify_ases_from]: workers.md#notifying-application-services
127+
128+
103129
# Upgrading to v1.58.0
104130

105131
## Groups/communities feature has been disabled by default
106132

107133
The non-standard groups/communities feature in Synapse has been disabled by default
108134
and will be removed in Synapse v1.61.0.
109135

136+
110137
# Upgrading to v1.57.0
111138

112139
## Changes to database schema for application services

docs/workers.md

+20
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,23 @@ An example for a dedicated background worker instance:
435435
{{#include systemd-with-workers/workers/background_worker.yaml}}
436436
```
437437

438+
#### Notifying Application Services
439+
440+
You can designate one worker to send output traffic to Application Services.
441+
442+
Specify its name in the shared configuration as follows:
443+
444+
```yaml
445+
notify_appservices_from_worker: worker_name
446+
```
447+
448+
This work cannot be load-balanced; please ensure the main process is restarted
449+
after setting this option in the shared configuration!
450+
451+
This style of configuration supersedes the legacy `synapse.app.appservice`
452+
worker application type.
453+
454+
438455
### `synapse.app.pusher`
439456

440457
Handles sending push notifications to sygnal and email. Doesn't handle any
@@ -453,6 +470,9 @@ pusher_instances:
453470

454471
### `synapse.app.appservice`
455472

473+
**Deprecated as of Synapse v1.58.** [Use `synapse.app.generic_worker` with the
474+
`notify_appservices_from_worker` option instead.](#notifying-application-services)
475+
456476
Handles sending output traffic to Application Services. Doesn't handle any
457477
REST endpoints itself, but you should set `notify_appservices: False` in the
458478
shared configuration file to stop the main synapse sending appservice notifications.

synapse/app/generic_worker.py

-16
Original file line numberDiff line numberDiff line change
@@ -441,22 +441,6 @@ def start(config_options: List[str]) -> None:
441441
"synapse.app.user_dir",
442442
)
443443

444-
if config.worker.worker_app == "synapse.app.appservice":
445-
if config.appservice.notify_appservices:
446-
sys.stderr.write(
447-
"\nThe appservices must be disabled in the main synapse process"
448-
"\nbefore they can be run in a separate worker."
449-
"\nPlease add ``notify_appservices: false`` to the main config"
450-
"\n"
451-
)
452-
sys.exit(1)
453-
454-
# Force the appservice to start since they will be disabled in the main config
455-
config.appservice.notify_appservices = True
456-
else:
457-
# For other worker types we force this to off.
458-
config.appservice.notify_appservices = False
459-
460444
if config.worker.worker_app == "synapse.app.user_dir":
461445
if config.server.update_user_directory:
462446
sys.stderr.write(

synapse/config/appservice.py

-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ class AppServiceConfig(Config):
3333

3434
def read_config(self, config: JsonDict, **kwargs: Any) -> None:
3535
self.app_service_config_files = config.get("app_service_config_files", [])
36-
self.notify_appservices = config.get("notify_appservices", True)
3736
self.track_appservice_user_ips = config.get("track_appservice_user_ips", False)
3837

3938
def generate_config_section(cls, **kwargs: Any) -> str:

synapse/config/workers.py

+108-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414
# limitations under the License.
1515

1616
import argparse
17-
from typing import Any, List, Union
17+
import logging
18+
from typing import Any, Dict, List, Union
1819

1920
import attr
2021

@@ -42,6 +43,13 @@
4243
Please add ``start_pushers: false`` to the main config
4344
"""
4445

46+
_DEPRECATED_WORKER_DUTY_OPTION_USED = """
47+
The '%s' configuration option is deprecated and will be removed in a future
48+
Synapse version. Please use ``%s: name_of_worker`` instead.
49+
"""
50+
51+
logger = logging.getLogger(__name__)
52+
4553

4654
def _instance_to_list_converter(obj: Union[str, List[str]]) -> List[str]:
4755
"""Helper for allowing parsing a string or list of strings to a config
@@ -296,6 +304,105 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
296304
self.worker_name is None and background_tasks_instance == "master"
297305
) or self.worker_name == background_tasks_instance
298306

307+
self.should_notify_appservices = self._should_this_worker_perform_duty(
308+
config,
309+
legacy_master_option_name="notify_appservices",
310+
legacy_worker_app_name="synapse.app.appservice",
311+
new_option_name="notify_appservices_from_worker",
312+
)
313+
314+
def _should_this_worker_perform_duty(
315+
self,
316+
config: Dict[str, Any],
317+
legacy_master_option_name: str,
318+
legacy_worker_app_name: str,
319+
new_option_name: str,
320+
) -> bool:
321+
"""
322+
Figures out whether this worker should perform a certain duty.
323+
324+
This function is temporary and is only to deal with the complexity
325+
of allowing old, transitional and new configurations all at once.
326+
327+
Contradictions between the legacy and new part of a transitional configuration
328+
will lead to a ConfigError.
329+
330+
Parameters:
331+
config: The config dictionary
332+
legacy_master_option_name: The name of a legacy option, whose value is boolean,
333+
specifying whether it's the master that should handle a certain duty.
334+
e.g. "notify_appservices"
335+
legacy_worker_app_name: The name of a legacy Synapse worker application
336+
that would traditionally perform this duty.
337+
e.g. "synapse.app.appservice"
338+
new_option_name: The name of the new option, whose value is the name of a
339+
designated worker to perform the duty.
340+
e.g. "notify_appservices_from_worker"
341+
"""
342+
343+
# None means 'unspecified'; True means 'run here' and False means
344+
# 'don't run here'.
345+
new_option_should_run_here = None
346+
if new_option_name in config:
347+
designated_worker = config[new_option_name] or "master"
348+
new_option_should_run_here = (
349+
designated_worker == "master" and self.worker_name is None
350+
) or designated_worker == self.worker_name
351+
352+
legacy_option_should_run_here = None
353+
if legacy_master_option_name in config:
354+
run_on_master = bool(config[legacy_master_option_name])
355+
356+
legacy_option_should_run_here = (
357+
self.worker_name is None and run_on_master
358+
) or (self.worker_app == legacy_worker_app_name and not run_on_master)
359+
360+
# Suggest using the new option instead.
361+
logger.warning(
362+
_DEPRECATED_WORKER_DUTY_OPTION_USED,
363+
legacy_master_option_name,
364+
new_option_name,
365+
)
366+
367+
if self.worker_app == legacy_worker_app_name and config.get(
368+
legacy_master_option_name, True
369+
):
370+
# As an extra bit of complication, we need to check that the
371+
# specialised worker is only used if the legacy config says the
372+
# master isn't performing the duties.
373+
raise ConfigError(
374+
f"Cannot use deprecated worker app type '{legacy_worker_app_name}' whilst deprecated option '{legacy_master_option_name}' is not set to false.\n"
375+
f"Consider setting `worker_app: synapse.app.generic_worker` and using the '{new_option_name}' option instead.\n"
376+
f"The '{new_option_name}' option replaces '{legacy_master_option_name}'."
377+
)
378+
379+
if new_option_should_run_here is None and legacy_option_should_run_here is None:
380+
# Neither option specified; the fallback behaviour is to run on the main process
381+
return self.worker_name is None
382+
383+
if (
384+
new_option_should_run_here is not None
385+
and legacy_option_should_run_here is not None
386+
):
387+
# Both options specified; ensure they match!
388+
if new_option_should_run_here != legacy_option_should_run_here:
389+
update_worker_type = (
390+
" and set worker_app: synapse.app.generic_worker"
391+
if self.worker_app == legacy_worker_app_name
392+
else ""
393+
)
394+
# If the values conflict, we suggest the admin removes the legacy option
395+
# for simplicity.
396+
raise ConfigError(
397+
f"Conflicting configuration options: {legacy_master_option_name} (legacy), {new_option_name} (new).\n"
398+
f"Suggestion: remove {legacy_master_option_name}{update_worker_type}.\n"
399+
)
400+
401+
# We've already validated that these aren't conflicting; now just see if
402+
# either is True.
403+
# (By this point, these are either the same value or only one is not None.)
404+
return bool(new_option_should_run_here or legacy_option_should_run_here)
405+
299406
def generate_config_section(self, **kwargs: Any) -> str:
300407
return """\
301408
## Workers ##

synapse/handlers/appservice.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ def __init__(self, hs: "HomeServer"):
5959
self.scheduler = hs.get_application_service_scheduler()
6060
self.started_scheduler = False
6161
self.clock = hs.get_clock()
62-
self.notify_appservices = hs.config.appservice.notify_appservices
62+
self.notify_appservices = hs.config.worker.should_notify_appservices
6363
self.event_sources = hs.get_event_sources()
6464
self._msc2409_to_device_messages_enabled = (
6565
hs.config.experimental.msc2409_to_device_messages_enabled

0 commit comments

Comments
 (0)