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

Remove the concept of worker "types" #10441

Open
2 of 6 tasks
anoadragon453 opened this issue Jul 21, 2021 · 2 comments
Open
2 of 6 tasks

Remove the concept of worker "types" #10441

anoadragon453 opened this issue Jul 21, 2021 · 2 comments
Labels
A-Workers Problems related to running Synapse in Worker Mode (or replication) T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@anoadragon453
Copy link
Member

anoadragon453 commented Jul 21, 2021

We're currently midway through the process of fading out the concept of different types for a Synapse worker process. For example synapse.app.pusher, synapse.app.federation_sender etc. While initially this provided a quick way to configure a worker to perform certain actions, it is also rather confusing and the inability to specify multiple types per worker can limit the flexibility of deployment.

For instance, it's currently not possible to configure a worker to both send events to appservices (synapse.app.appservice) and update the user directory tables (synapse.app.user_dir), as the functionality is forcefully disabled unless the worker is specifically marked as each respective type:

if config.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_app == "synapse.app.user_dir":
if config.server.update_user_directory:
sys.stderr.write(
"\nThe update_user_directory must be disabled in the main synapse process"
"\nbefore they can be run in a separate worker."
"\nPlease add ``update_user_directory: false`` to the main config"
"\n"
)
sys.exit(1)
# Force the pushers to start since they will be disabled in the main config
config.server.update_user_directory = True
else:
# For other worker types we force this to off.
config.server.update_user_directory = False

Instead, workers should be configured via worker file config options, so that one could simply set notify_appservices: true and update_user_directory: true in the worker config to enable the worker to handle both sets of tasks.

As many worker deployments currently rely on the worker_type configuration, a deprecation period is necessary. A rough plan for carrying this out could look like:

  • Allow all current worker configurations through config file values alone.
  • Announce a deprecation period for the worker_type config option.
  • After some time, remove the worker_type config option. All workers are now a "generic worker", though this term is no longer a necessary distinction.

In the short term, the first step would allow for more immediate flexibility for medium-sized deployments - which need to move tasks off the main process, but don't have the resources to spin up a separate worker for each type.

Superseded worker types so far

@anoadragon453 anoadragon453 added the T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. label Jul 21, 2021
@anoadragon453
Copy link
Member Author

anoadragon453 commented Jul 29, 2021

One small thinko for the first step was how we'd be able to keep the current functionality of warning the user that both the worker and main process was configured for the same purpose. This is necessary if you need a task to run on at max one worker process. We currently use a combination of the worker type (i.e synapse.app.appservice) and config options (notify_appservices), and we'd like to remove the former.

We already have a solution to this: simply add a config option for specify the name of the worker that should handle the task. As an example, run_background_tasks_on: <worker_name>.

@richvdh
Copy link
Member

richvdh commented Aug 10, 2022

frontend_proxy is already totally redundant: there is no difference between it and generic_worker. The only thing left is to remove it from the documentation, which I hope #13451 will do.

I also think synapse.app.pusher is superceded by pusher_instances (#9466), but again it needs documenting properly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Workers Problems related to running Synapse in Worker Mode (or replication) T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

No branches or pull requests

3 participants