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

Update worker docs to update preferred settings for pusher and federation_sender #14493

Merged
merged 10 commits into from
Dec 2, 2022
Merged

Update worker docs to update preferred settings for pusher and federation_sender #14493

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
df80dba
Fix one typo on line 3700(and apparently do something to other lines,…
realtyem Nov 18, 2022
51d689b
Update config_documentation.md with more information about how federa…
realtyem Nov 18, 2022
1011f98
Extra line here for consistency and appearance.
realtyem Nov 18, 2022
4c9a1fe
Add link to sygnal repo.
realtyem Nov 18, 2022
344d7f6
Add deprecation notice to workers.md and point to the newer alternati…
realtyem Nov 18, 2022
801cbd6
Changelog
realtyem Nov 19, 2022
abbdffd
Merge branch 'develop' into update-worker-docs-part-1
realtyem Nov 19, 2022
8b5f89d
Merge branch 'develop' into update-worker-docs-part-1
realtyem Nov 25, 2022
ac4653e
Correct version number of Synapse the deprecation is happening in.
realtyem Nov 25, 2022
a9efe4d
Update quiet deprecation with simple notice and suggestion.
realtyem Dec 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/14493.doc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Update worker settings for `pusher` and `federation_sender` functionality.
75 changes: 44 additions & 31 deletions docs/usage/configuration/config_documentation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,7 @@ which are older than the room's maximum retention period. Synapse will also
filter events received over federation so that events that should have been
purged are ignored and not stored again.

The message retention policies feature is disabled by default. Please be advised
The message retention policies feature is disabled by default. Please be advised
that enabling this feature carries some risk. There are known bugs with the implementation
which can cause database corruption. Setting retention to delete older history
is less risky than deleting newer history but in general caution is advised when enabling this
Expand Down Expand Up @@ -3021,7 +3021,7 @@ Options for each entry include:
which is set to the claims returned by the UserInfo Endpoint and/or
in the ID Token.

* `backchannel_logout_enabled`: set to `true` to process OIDC Back-Channel Logout notifications.
* `backchannel_logout_enabled`: set to `true` to process OIDC Back-Channel Logout notifications.
Those notifications are expected to be received on `/_synapse/client/oidc/backchannel_logout`.
Defaults to `false`.

Expand Down Expand Up @@ -3438,7 +3438,7 @@ This option has the following sub-options:
NB. If you set this to true, and the last time the user_directory search
indexes were (re)built was before Synapse 1.44, you'll have to
rebuild the indexes in order to search through all known users.

These indexes are built the first time Synapse starts; admins can
manually trigger a rebuild via the API following the instructions
[for running background updates](../administration/admin_api/background_updates.md#run),
Expand Down Expand Up @@ -3697,7 +3697,7 @@ As a result, the worker configuration is divided into two parts.

1. The first part (in this section of the manual) defines which shardable tasks
are delegated to privileged workers. This allows unprivileged workers to make
request a privileged worker to act on their behalf.
requests to a privileged worker to act on their behalf.
realtyem marked this conversation as resolved.
Show resolved Hide resolved
1. [The second part](#individual-worker-configuration)
controls the behaviour of individual workers in isolation.

Expand All @@ -3709,7 +3709,7 @@ For guidance on setting up workers, see the [worker documentation](../../workers
A shared secret used by the replication APIs on the main process to authenticate
HTTP requests from workers.

The default, this value is omitted (equivalently `null`), which means that
The default, this value is omitted (equivalently `null`), which means that
traffic between the workers and the main process is not authenticated.

Example configuration:
Expand All @@ -3719,6 +3719,8 @@ worker_replication_secret: "secret_secret"
---
### `start_pushers`

Unnecessary to set if using [`pusher_instances`](#pusher_instances) with [`generic_workers`](../../workers.md#synapseappgeneric_worker).

Controls sending of push notifications on the main process. Set to `false`
if using a [pusher worker](../../workers.md#synapseapppusher). Defaults to `true`.

Expand All @@ -3729,25 +3731,30 @@ start_pushers: false
---
### `pusher_instances`

It is possible to run multiple [pusher workers](../../workers.md#synapseapppusher),
in which case the work is balanced across them. Use this setting to list the pushers by
[`worker_name`](#worker_name). Ensure the main process and all pusher workers are
restarted after changing this option.

If no or only one pusher worker is configured, this setting is not necessary.
The main process will send out push notifications by default if you do not disable
it by setting [`start_pushers: false`](#start_pushers).
It is possible to scale the processes that handle sending push notifications to [sygnal](https://github.com/matrix-org/sygnal)
and email by running a [`generic_worker`](../../workers.md#synapseappgeneric_worker) and adding it's [`worker_name`](#worker_name) to
a `pusher_instances` map. Doing so will remove handling of this function from the main
process. Multiple workers can be added to this map, in which case the work is balanced
across them. Ensure the main process and all pusher workers are restarted after changing
this option.

Example configuration:
Example configuration for a single worker:
```yaml
pusher_instances:
- pusher_worker1
```
And for multiple workers:
```yaml
start_pushers: false
pusher_instances:
- pusher_worker1
- pusher_worker2
```

---
### `send_federation`

Unnecessary to set if using [`federation_sender_instances`](#federation_sender_instances) with [`generic_workers`](../../workers.md#synapseappgeneric_worker).

Controls sending of outbound federation transactions on the main process.
Set to `false` if using a [federation sender worker](../../workers.md#synapseappfederation_sender).
Defaults to `true`.
Expand All @@ -3759,29 +3766,36 @@ send_federation: false
---
### `federation_sender_instances`

It is possible to run multiple
[federation sender worker](../../workers.md#synapseappfederation_sender), in which
case the work is balanced across them. Use this setting to list the senders.
It is possible to scale the processes that handle sending outbound federation requests
by running a [`generic_worker`](../../workers.md#synapseappgeneric_worker) and adding it's [`worker_name`](#worker_name) to
a `federation_sender_instances` map. Doing so will remove handling of this function from
the main process. Multiple workers can be added to this map, in which case the work is
balanced across them.

This configuration setting must be shared between all federation sender workers, and if
changed all federation sender workers must be stopped at the same time and then
started, to ensure that all instances are running with the same config (otherwise
This configuration setting must be shared between all workers handling federation
sending, and if changed all federation sender workers must be stopped at the same time
and then started, to ensure that all instances are running with the same config (otherwise
events may be dropped).

Example configuration:
Example configuration for a single worker:
```yaml
send_federation: false
federation_sender_instances:
- federation_sender1
```
And for multiple workers:
```yaml
federation_sender_instances:
- federation_sender1
- federation_sender2
```
---
### `instance_map`

When using workers this should be a map from [`worker_name`](#worker_name) to the
HTTP replication listener of the worker, if configured.
Each worker declared under [`stream_writers`](../../workers.md#stream-writers) needs
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 HTTP replication listener, but it should not be
(The main process also needs an HTTP replication listener, but it should not be
listed in the `instance_map`.)

Example configuration:
Expand Down Expand Up @@ -3915,8 +3929,8 @@ worker_replication_http_tls: true
---
### `worker_listeners`

A worker can handle HTTP requests. To do so, a `worker_listeners` option
must be declared, in the same way as the [`listeners` option](#listeners)
A worker can handle HTTP requests. To do so, a `worker_listeners` option
must be declared, in the same way as the [`listeners` option](#listeners)
in the shared config.

Workers declared in [`stream_writers`](#stream_writers) will need to include a
Expand All @@ -3935,7 +3949,7 @@ worker_listeners:
### `worker_daemonize`

Specifies whether the worker should be started as a daemon process.
If Synapse is being managed by [systemd](../../systemd-with-workers/README.md), this option
If Synapse is being managed by [systemd](../../systemd-with-workers/README.md), this option
must be omitted or set to `false`.

Defaults to `false`.
Expand All @@ -3947,11 +3961,11 @@ worker_daemonize: true
---
### `worker_pid_file`

When running a worker as a daemon, we need a place to store the
When running a worker as a daemon, we need a place to store the
[PID](https://en.wikipedia.org/wiki/Process_identifier) of the worker.
This option defines the location of that "pid file".

This option is required if `worker_daemonize` is `true` and ignored
This option is required if `worker_daemonize` is `true` and ignored
otherwise. It has no default.

See also the [`pid_file` option](#pid_file) option for the main Synapse process.
Expand Down Expand Up @@ -4001,4 +4015,3 @@ background_updates:
min_batch_size: 10
default_batch_size: 50
```

6 changes: 6 additions & 0 deletions docs/workers.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,8 @@ worker application type.

### `synapse.app.pusher`

**Deprecated as of Synapse v1.74.** [Use `synapse.app.generic_worker` with the `pusher_instances` map instead](usage/configuration/config_documentation.md#pusher_instances).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This I'm really not sure how to handle. Is it really deprecated? It's not mentioned anywhere in source that is important outside of backwards-compatibility and the contrib folder. The one has to stay until the 'go-ahead' is given to rip it out, and the other...am I allowed to edit other people's work in this case? I don't want to step on any toes...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I gave it a longer window than I did for the last PR, just in case)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO in a normal case deperactions are announced in changelog and upgrade notes. This can perhaps be done in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw user_dir and appservice and thought it should be duplicated in a like manner.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the next version is v1.73.0 :-).

Otherwise, yes, you just note it here and add to the docs/upgrade.md — you should see an existing section for anything that's already deprecated.

Also, I would make the changelog fragment a .removal (full title is 'Removals and deprecations', so it does fit) and change it to say what you're deprecating, rather than just being a pure doc change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the deal: the functionality hasn't changed and there are a bunch of the synapse.app.* that are technically part of that change. I didn't want to make it a full on 'removal' until all the apps are ready(working on media_repository now) and all the tests are updated before doing a blanket "You all need to change your settings now because this is going away in XX weeks". Plus, once it's actually ready and documented, I'll need to know how long is acceptable to add that to the doc/upgrade.md change. I'm not sure how long to give for that. I will change this to reflect Synapse v1.73 for this PR, but the other....I think that's a bit out of scope for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rudimentary timeline:

  1. This change to docs.
  2. Update tests relevant to this doc change.
  3. Update other tests that are already including apps.* that were previously deprecated(like appservice, user_dir, client_reader, etc) and need their settings adjusted.
  4. Fix synapse.app.media_repository in tests.
  5. Update settings configuration logic for synapse.app.media_repository to handle new methods.
  6. Fix synapse.app.media_repository in tests, again.
  7. Triple check nothing got orphaned.
  8. (Profit???)
  9. Blanket update in docs/workers.md and docs/upgrade.md saying these are being retired and to update settings to new recommended options. Probably going to see a whole restructure of how docs/workers.md is written out as right now it's a bunch of words and almost needs an index or a table of contents(for the record, not looking forward to this part). Maybe break out the 'Historical Apps' section into a separate page that gets linked to. I'll need a respectable schedule for this. Six months, 4 weeks, something. Point being that doing this all at once and giving them a deadline will mean no disgruntled admins having to keep changing things a bunch of times, just the once.
  10. Add log warnings as a visual reminder that they need to update their settings. Prior art is available.
  11. There's more, like dealing with synctl.py and it's reference to synapse.app.homeserver but I think that's the bulk of it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, we've missed v1.73 anyway (my fault for being off, sorry).
I'd be reluctant to say 'Deprecated since v1.xx' if we're not going to make noise about it in the upgrade notes at the same time: we don't expect admins to read random pieces of documentation periodically but we are happy to expect them to read the upgrade notes. (So in a way, we haven't really been honest about giving fair warning before removing it if we 'quietly' deprecate it this way.)
That said, I agree that it'd be good to knock them all out at once, so if some of them aren't ready, let's wait. In that case, I'd rephrase these changes to the documentation as 'suggestions' rather than calling them deprecated — perhaps saying they are likely to be deprecated in the future and we don't recommend their use for new installations.
Once we have all the worker types converted, then it's fair to deprecate them all and announce it in the upgrade notes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this?
a9efe4d


Handles sending push notifications to sygnal and email. Doesn't handle any
REST endpoints itself, but you should set
[`start_pushers: false`](usage/configuration/config_documentation.md#start_pushers) in the
Expand Down Expand Up @@ -541,6 +543,8 @@ Note this worker cannot be load-balanced: only one instance should be active.

### `synapse.app.federation_sender`

**Deprecated as of Synapse v1.74.** [Use `synapse.app.generic_worker` with the `federation_sender_instances` map instead](usage/configuration/config_documentation.md#federation_sender_instances).

Handles sending federation traffic to other servers. Doesn't handle any
REST endpoints itself, but you should set
[`send_federation: false`](usage/configuration/config_documentation.md#send_federation)
Expand Down Expand Up @@ -637,7 +641,9 @@ equivalent to `synapse.app.generic_worker`:
* `synapse.app.client_reader`
* `synapse.app.event_creator`
* `synapse.app.federation_reader`
* `synapse.app.federation_sender`
* `synapse.app.frontend_proxy`
* `synapse.app.pusher`
* `synapse.app.synchrotron`


Expand Down