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

Remove need for worker_main_http_uri setting to use /keys/upload. #14400

Merged
merged 18 commits into from
Nov 16, 2022

Conversation

realtyem
Copy link
Contributor

@realtyem realtyem commented Nov 9, 2022

Fixes #13642

worker_main_http_uri is a setting used by a worker yaml config file to direct a worker to proxy the /keys/upload endpoint to the main process. Instead of using a proxy, send the request to main by way of http replication.

To-do:

  • Build a new replication http servlet based on ReplicationUserDevicesResyncRestServlet because keys are ultimately about devices anyways.
  • Add conditional to main process KeyUploadServlet to use replication if a worker.
  • Strip out the old proxy version of KeyUploadServlet that was defined in generic_worker.py, just going to use the existing one for the main process. This functionality is now carried by the replication http servlet.
  • Overly long Docstring(please check me for accuracy).
  • Update configuration manual to include deprecation of setting.

Some odd things happened while I was working on this. While poking around and trying to figure out replication http, I missed filling out the _serialize_payload() function overload. Sytest correctly caught this and errored. Complement did not. Why? There is only one test for this /keys/upload endpoint, if the request got no information(it's a post request), then shouldn't it have errored since it would not have gotten a user_id nor a device_id(apparently not getting an actual set of keys doesn't matter as the response only sends back a count of whatever onetime_keys it finds and not some kind of acknowledgement).

Pull Request Checklist

Signed-off-by: Jason Little realtyem@gmail.com

@clokep
Copy link
Member

clokep commented Nov 9, 2022

We probably want to note that the configuration setting can be removed (and maybe log a deprecation notice if it is set? I don't know how folks feel about that)

@realtyem
Copy link
Contributor Author

realtyem commented Nov 9, 2022

We probably want to note that the configuration setting can be removed (and maybe log a deprecation notice if it is set? I don't know how folks feel about that)

Update docs, ok. The other, no idea how to do that.

@realtyem realtyem changed the title Remove need for worker_http_main_uri setting to use /keys/upload. Remove need for worker_main_http_uri setting to use /keys/upload. Nov 11, 2022
docs/workers.md Outdated Show resolved Hide resolved
Comment on lines 158 to 166
self.worker_main_http_uri = config.get("worker_main_http_uri", None)

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we log a warning if "worker_main_http_uri" in config?

Something like "The config option worker_main_http_uri is unused since Synapse 1.72. It can be safely removed from your configuration."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Level INFO ok? Or would DEBUG be better?

Copy link
Contributor

Choose a reason for hiding this comment

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

Warning, please.

Copy link
Contributor

Choose a reason for hiding this comment

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

logger.warning(NON_SQLITE_DATABASE_PATH_WARNING)
for example. More generally, grep for logger.warning in synapse/config

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in 8b4e099
That look ok?

Comment on lines 86 to 87
This must happen on master so that the results can be correctly cached in
the database and streamed to workers.( Is this accurate?)
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect the point is:

  • for now, there must be exactly one worker writing keys (to avoid write races)
  • we haven't found the key writing to be heavy enough that it needs pulling off the master worker
  • so it's written by the master.

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 guess that specifically the "cached in the database and streamed to workers." is the part I wanted to make sure was accurate. This is a copy-paste dangler. The Replication HTTP Servlet itself does no caching of it's own(the CACHE modifier is set to False below)

Copy link
Contributor

Choose a reason for hiding this comment

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

Broadly speaking, the database doesn't cache anything; it is the authoritative source of truth for the state of the homeserver. The application maintains various in-memory caches of the database's data. Workers typically receive cache invalidation messages over redis rather than new updates.

Comment on lines 93 to 142
Request format(borrowed and expanded from KeyUploadServlet):

POST /_synapse/replication/upload_keys_for_user

{
"user_id": "<user_id>",
"device_id": "<device_id>",
"keys": {
"device_keys": {
"user_id": "<user_id>",
"device_id": "<device_id>",
"valid_until_ts": <millisecond_timestamp>,
"algorithms": [
"m.olm.curve25519-aes-sha2",
]
"keys": {
"<algorithm>:<device_id>": "<key_base64>",
},
"signatures:" {
"<user_id>" {
"<algorithm>:<device_id>": "<signature_base64>"
}
}
},
"fallback_keys": {
"<algorithm>:<device_id>": "<key_base64>",
"signed_<algorithm>:<device_id>": {
"fallback": true,
"key": "<key_base64>",
"signatures": {
"<user_id>": {
"<algorithm>:<device_id>": "<key_base64>"
}
}
}
}
"one_time_keys": {
"<algorithm>:<key_id>": "<key_base64>"
},
}
}
Response is equivalent to ` /_matrix/client/v3/keys/upload`
response, e.g.:

{
"one_time_key_counts": {
"curve25519": 10,
"signed_curve25519": 20
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

borrowed and expanded from KeyUploadServlet

KeyUploadServlet presumably refers to client/keys.py and not generic_worker (whose KeyUploadServelet was removed above).

I think it makes more sense to

  • update the docstring for KeyUploadServelet with your changes
  • have this docstring point us to that one. "See KeyUploadServelet for format of the request body and response body."
  • keep the POST /_synapse/replication/upload_keys_for_user line here

Copy link
Contributor Author

@realtyem realtyem Nov 14, 2022

Choose a reason for hiding this comment

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

Yeah, it's slightly different though. Because of the passing of the information in the _serialize_json() _serialize_payload() includes having the user_id and the device_id which isn't part of the same request as the original prepended to the beginning of the request...(I think). Plus it wraps it into a keys object as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Took me a minute, but I see what you mean. How about f2c780b?

synapse/replication/http/devices.py Show resolved Hide resolved
synapse/app/generic_worker.py Show resolved Hide resolved
Co-authored-by: David Robertson <david.m.robertson1@gmail.com>
Comment on lines 86 to 87
This must happen on master so that the results can be correctly cached in
the database and streamed to workers.( Is this accurate?)
Copy link
Contributor

Choose a reason for hiding this comment

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

Broadly speaking, the database doesn't cache anything; it is the authoritative source of truth for the state of the homeserver. The application maintains various in-memory caches of the database's data. Workers typically receive cache invalidation messages over redis rather than new updates.

synapse/replication/http/devices.py Outdated Show resolved Hide resolved
@DMRobertson DMRobertson enabled auto-merge (squash) November 16, 2022 12:36
@DMRobertson
Copy link
Contributor

Closes #13642.

@clokep
Copy link
Member

clokep commented Nov 29, 2022

I put up matrix-org/sytest#1319 to remove this from sytest.

clokep added a commit to matrix-org/sytest that referenced this pull request Nov 29, 2022
This was removed in matrix-org/synapse#14400 and is
currently a no-op that adds warnings to the logs.
Fizzadar added a commit to beeper/synapse-legacy-fork that referenced this pull request Dec 15, 2022
Synapse 1.73.0 (2022-12-06)
===========================

Please note that legacy Prometheus metric names have been removed in this release; see [the upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.73/docs/upgrade.md#legacy-prometheus-metric-names-have-now-been-removed) for more details.

No significant changes since 1.73.0rc2.

Synapse 1.73.0rc2 (2022-12-01)
==============================

Bugfixes
--------

- Fix a regression in Synapse 1.73.0rc1 where Synapse's main process would stop responding to HTTP requests when a user with a large number of devices logs in. ([\matrix-org#14582](matrix-org#14582))

Synapse 1.73.0rc1 (2022-11-29)
==============================

Features
--------

- Speed-up `/messages` with `filter_events_for_client` optimizations. ([\matrix-org#14527](matrix-org#14527))
- Improve DB performance by reducing amount of data that gets read in `device_lists_changes_in_room`. ([\matrix-org#14534](matrix-org#14534))
- Adds support for handling avatar in SSO OIDC login. Contributed by @ashfame. ([\matrix-org#13917](matrix-org#13917))
- Move MSC3030 `/timestamp_to_event` endpoints to stable `v1` location (`/_matrix/client/v1/rooms/<roomID>/timestamp_to_event?ts=<timestamp>&dir=<direction>`, `/_matrix/federation/v1/timestamp_to_event/<roomID>?ts=<timestamp>&dir=<direction>`). ([\matrix-org#14471](matrix-org#14471))
- Reduce database load of [Client-Server endpoints](https://spec.matrix.org/v1.5/client-server-api/#aggregations) which return bundled aggregations. ([\matrix-org#14491](matrix-org#14491), [\matrix-org#14508](matrix-org#14508), [\matrix-org#14510](matrix-org#14510))
- Add unstable support for an Extensible Events room version (`org.matrix.msc1767.10`) via [MSC1767](matrix-org/matrix-spec-proposals#1767), [MSC3931](matrix-org/matrix-spec-proposals#3931), [MSC3932](matrix-org/matrix-spec-proposals#3932), and [MSC3933](matrix-org/matrix-spec-proposals#3933). ([\matrix-org#14520](matrix-org#14520), [\matrix-org#14521](matrix-org#14521), [\matrix-org#14524](matrix-org#14524))
- Prune user's old devices on login if they have too many. ([\matrix-org#14038](matrix-org#14038), [\matrix-org#14580](matrix-org#14580))

Bugfixes
--------

- Fix a long-standing bug where paginating from the start of a room did not work. Contributed by @gnunicorn. ([\matrix-org#14149](matrix-org#14149))
- Fix a bug introduced in Synapse 1.58.0 where a user with presence state `org.matrix.msc3026.busy` would mistakenly be set to `online` when calling `/sync` or `/events` on a worker process. ([\matrix-org#14393](matrix-org#14393))
- Fix a bug introduced in Synapse 1.70.0 where a receipt's thread ID was not sent over federation. ([\matrix-org#14466](matrix-org#14466))
- Fix a long-standing bug where the [List media admin API](https://matrix-org.github.io/synapse/latest/admin_api/media_admin_api.html#list-all-media-in-a-room) would fail when processing an image with broken thumbnail information. ([\matrix-org#14537](matrix-org#14537))
- Fix a bug introduced in Synapse 1.67.0 where two logging context warnings would be logged on startup. ([\matrix-org#14574](matrix-org#14574))
- In application service transactions that include the experimental `org.matrix.msc3202.device_one_time_key_counts` key, include a duplicate key of `org.matrix.msc3202.device_one_time_keys_count` to match the name proposed by [MSC3202](matrix-org/matrix-spec-proposals#3202). ([\matrix-org#14565](matrix-org#14565))
- Fix a bug introduced in Synapse 0.9 where Synapse would fail to fetch server keys whose IDs contain a forward slash. ([\matrix-org#14490](matrix-org#14490))

Improved Documentation
----------------------

- Fixed link to 'Synapse administration endpoints'. ([\matrix-org#14499](matrix-org#14499))

Deprecations and Removals
-------------------------

- Remove legacy Prometheus metrics names. They were deprecated in Synapse v1.69.0 and disabled by default in Synapse v1.71.0. ([\matrix-org#14538](matrix-org#14538))

Internal Changes
----------------

- Improve type hinting throughout Synapse. ([\matrix-org#14055](matrix-org#14055), [\matrix-org#14412](matrix-org#14412), [\matrix-org#14529](matrix-org#14529), [\matrix-org#14452](matrix-org#14452)).
- Remove old stream ID tracking code. Contributed by Nick @beeper (@Fizzadar). ([\matrix-org#14376](matrix-org#14376), [\matrix-org#14468](matrix-org#14468))
- Remove the `worker_main_http_uri` configuration setting. This is now handled via internal replication. ([\matrix-org#14400](matrix-org#14400), [\matrix-org#14476](matrix-org#14476))
- Refactor `federation_sender` and `pusher` configuration loading. ([\matrix-org#14496](matrix-org#14496))
([\matrix-org#14509](matrix-org#14509), [\matrix-org#14573](matrix-org#14573))
- Faster joins: do not wait for full state when creating events to send. ([\matrix-org#14403](matrix-org#14403))
- Faster joins: filter out non local events when a room doesn't have its full state. ([\matrix-org#14404](matrix-org#14404))
- Faster joins: send events to initial list of servers if we don't have the full state yet. ([\matrix-org#14408](matrix-org#14408))
- Faster joins: use servers list approximation received during `send_join` (potentially updated with received membership events) in `assert_host_in_room`. ([\matrix-org#14515](matrix-org#14515))
- Fix type logic in TCP replication code that prevented correctly ignoring blank commands. ([\matrix-org#14449](matrix-org#14449))
- Remove option to skip locking of tables when performing emulated upserts, to avoid a class of bugs in future. ([\matrix-org#14469](matrix-org#14469))
- `scripts-dev/federation_client`: Fix routing on servers with `.well-known` files. ([\matrix-org#14479](matrix-org#14479))
- Reduce default third party invite rate limit to 216 invites per day. ([\matrix-org#14487](matrix-org#14487))
- Refactor conversion of device list changes in room to outbound pokes to track unconverted rows using a `(stream ID, room ID)` position instead of updating the `converted_to_destinations` flag on every row. ([\matrix-org#14516](matrix-org#14516))
- Add more prompts to the bug report form. ([\matrix-org#14522](matrix-org#14522))
- Extend editorconfig rules on indent and line length to `.pyi` files. ([\matrix-org#14526](matrix-org#14526))
- Run Rust CI when `Cargo.lock` changes. This is particularly useful for dependabot updates. ([\matrix-org#14571](matrix-org#14571))
- Fix a possible variable shadow in `create_new_client_event`. ([\matrix-org#14575](matrix-org#14575))
- Bump various dependencies in the `poetry.lock` file and in CI scripts. ([\matrix-org#14557](matrix-org#14557), [\matrix-org#14559](matrix-org#14559), [\matrix-org#14560](matrix-org#14560), [\matrix-org#14500](matrix-org#14500), [\matrix-org#14501](matrix-org#14501), [\matrix-org#14502](matrix-org#14502), [\matrix-org#14503](matrix-org#14503), [\matrix-org#14504](matrix-org#14504), [\matrix-org#14505](matrix-org#14505)).

# -----BEGIN PGP SIGNATURE-----
#
# iQIzBAABCgAdFiEE8SRSDO7gYkSP4chELS76LzL74EcFAmOPLnYACgkQLS76LzL7
# 4Edwpg/+KXpg2ZdiJ0Yaly9VHVeiqdHRi5D7WPS6n8YBsdRx9EQHzOBkD5HAW8hE
# oz0c+zDS01ORlEWD825NYXjgaE1ijtZFvGxsftYTVuTYlVRR2m+r9jhDv9pVHT53
# TKtQVKpG0IUsuyukRBrweDcEeO0MA0nGpvaaQUhmftzWgy4yD3AjZyIgx0Ckg8pg
# OwgrzGqA7FQs4MEeOxmk1H39fZg4dlo4nmI4whvAodgaGeS9sU8t+3Qj4PVod8v/
# AkVesJcruaTHuVMb+Xp8JKezb09SsIR94gmHalC5sL+41+6XAy9BtQ/cRDfCReG3
# U1I1x1h1+EQjTP6XzMmjQHLbfI2gUJBC4I2p3e2gZ4cMm9rVz94R1dBiRk8ZgRIC
# cJFD9BvaAtb2PSTvyFBoHsrrn/u12i8fYFWu4Z4rO6dOGI83dZHeZzVw4UsVeqIK
# 5+njQwcwQsrwL3AKLjbbdqmbmhXcF6LchIK2L+NuuvdiOfvXvkO0bdjBryVEbMqB
# IOtAAWzwYaoUwVucMbBtXt/EqQS7biGkbDxsL8CDvaBwM/JSsUWXBafsV1FmxF2A
# q6KAeKpfelefoegosTYD0Md+l39xdF8Z19XaKV3GeHZEY+HE3RJXJm+Pa8SJ+IF8
# Y1od9cB/H+fYSsWCWj1OJNqTIAozh6f1Pe2nFuFDxdBwABXc/pg=
# =IBEL
# -----END PGP SIGNATURE-----
# gpg: Signature made Tue Dec  6 11:58:46 2022 GMT
# gpg:                using RSA key F124520CEEE062448FE1C8442D2EFA2F32FBE047
# gpg: Can't check signature: No public key

# Conflicts:
#	poetry.lock
#	synapse/push/bulk_push_rule_evaluator.py
#	synapse/storage/databases/main/account_data.py
#	synapse/storage/databases/main/receipts.py
realtyem added a commit to realtyem/synapse-unraid that referenced this pull request Dec 18, 2022
Synapse 1.73.0 (2022-12-06)
===========================

Please note that legacy Prometheus metric names have been removed in this release; see [the upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.73/docs/upgrade.md#legacy-prometheus-metric-names-have-now-been-removed) for more details.

No significant changes since 1.73.0rc2.

Synapse 1.73.0rc2 (2022-12-01)
==============================

Bugfixes
--------

- Fix a regression in Synapse 1.73.0rc1 where Synapse's main process would stop responding to HTTP requests when a user with a large number of devices logs in. ([\#14582](matrix-org/synapse#14582))

Synapse 1.73.0rc1 (2022-11-29)
==============================

Features
--------

- Speed-up `/messages` with `filter_events_for_client` optimizations. ([\#14527](matrix-org/synapse#14527))
- Improve DB performance by reducing amount of data that gets read in `device_lists_changes_in_room`. ([\#14534](matrix-org/synapse#14534))
- Adds support for handling avatar in SSO OIDC login. Contributed by @ashfame. ([\#13917](matrix-org/synapse#13917))
- Move MSC3030 `/timestamp_to_event` endpoints to stable `v1` location (`/_matrix/client/v1/rooms/<roomID>/timestamp_to_event?ts=<timestamp>&dir=<direction>`, `/_matrix/federation/v1/timestamp_to_event/<roomID>?ts=<timestamp>&dir=<direction>`). ([\#14471](matrix-org/synapse#14471))
- Reduce database load of [Client-Server endpoints](https://spec.matrix.org/v1.5/client-server-api/#aggregations) which return bundled aggregations. ([\#14491](matrix-org/synapse#14491), [\#14508](matrix-org/synapse#14508), [\#14510](matrix-org/synapse#14510))
- Add unstable support for an Extensible Events room version (`org.matrix.msc1767.10`) via [MSC1767](matrix-org/matrix-spec-proposals#1767), [MSC3931](matrix-org/matrix-spec-proposals#3931), [MSC3932](matrix-org/matrix-spec-proposals#3932), and [MSC3933](matrix-org/matrix-spec-proposals#3933). ([\#14520](matrix-org/synapse#14520), [\#14521](matrix-org/synapse#14521), [\#14524](matrix-org/synapse#14524))
- Prune user's old devices on login if they have too many. ([\#14038](matrix-org/synapse#14038), [\#14580](matrix-org/synapse#14580))

Bugfixes
--------

- Fix a long-standing bug where paginating from the start of a room did not work. Contributed by @gnunicorn. ([\#14149](matrix-org/synapse#14149))
- Fix a bug introduced in Synapse 1.58.0 where a user with presence state `org.matrix.msc3026.busy` would mistakenly be set to `online` when calling `/sync` or `/events` on a worker process. ([\#14393](matrix-org/synapse#14393))
- Fix a bug introduced in Synapse 1.70.0 where a receipt's thread ID was not sent over federation. ([\#14466](matrix-org/synapse#14466))
- Fix a long-standing bug where the [List media admin API](https://matrix-org.github.io/synapse/latest/admin_api/media_admin_api.html#list-all-media-in-a-room) would fail when processing an image with broken thumbnail information. ([\#14537](matrix-org/synapse#14537))
- Fix a bug introduced in Synapse 1.67.0 where two logging context warnings would be logged on startup. ([\#14574](matrix-org/synapse#14574))
- In application service transactions that include the experimental `org.matrix.msc3202.device_one_time_key_counts` key, include a duplicate key of `org.matrix.msc3202.device_one_time_keys_count` to match the name proposed by [MSC3202](matrix-org/matrix-spec-proposals#3202). ([\#14565](matrix-org/synapse#14565))
- Fix a bug introduced in Synapse 0.9 where Synapse would fail to fetch server keys whose IDs contain a forward slash. ([\#14490](matrix-org/synapse#14490))

Improved Documentation
----------------------

- Fixed link to 'Synapse administration endpoints'. ([\#14499](matrix-org/synapse#14499))

Deprecations and Removals
-------------------------

- Remove legacy Prometheus metrics names. They were deprecated in Synapse v1.69.0 and disabled by default in Synapse v1.71.0. ([\#14538](matrix-org/synapse#14538))

Internal Changes
----------------

- Improve type hinting throughout Synapse. ([\#14055](matrix-org/synapse#14055), [\#14412](matrix-org/synapse#14412), [\#14529](matrix-org/synapse#14529), [\#14452](matrix-org/synapse#14452)).
- Remove old stream ID tracking code. Contributed by Nick @beeper (@Fizzadar). ([\#14376](matrix-org/synapse#14376), [\#14468](matrix-org/synapse#14468))
- Remove the `worker_main_http_uri` configuration setting. This is now handled via internal replication. ([\#14400](matrix-org/synapse#14400), [\#14476](matrix-org/synapse#14476))
- Refactor `federation_sender` and `pusher` configuration loading. ([\#14496](matrix-org/synapse#14496))
([\#14509](matrix-org/synapse#14509), [\#14573](matrix-org/synapse#14573))
- Faster joins: do not wait for full state when creating events to send. ([\#14403](matrix-org/synapse#14403))
- Faster joins: filter out non local events when a room doesn't have its full state. ([\#14404](matrix-org/synapse#14404))
- Faster joins: send events to initial list of servers if we don't have the full state yet. ([\#14408](matrix-org/synapse#14408))
- Faster joins: use servers list approximation received during `send_join` (potentially updated with received membership events) in `assert_host_in_room`. ([\#14515](matrix-org/synapse#14515))
- Fix type logic in TCP replication code that prevented correctly ignoring blank commands. ([\#14449](matrix-org/synapse#14449))
- Remove option to skip locking of tables when performing emulated upserts, to avoid a class of bugs in future. ([\#14469](matrix-org/synapse#14469))
- `scripts-dev/federation_client`: Fix routing on servers with `.well-known` files. ([\#14479](matrix-org/synapse#14479))
- Reduce default third party invite rate limit to 216 invites per day. ([\#14487](matrix-org/synapse#14487))
- Refactor conversion of device list changes in room to outbound pokes to track unconverted rows using a `(stream ID, room ID)` position instead of updating the `converted_to_destinations` flag on every row. ([\#14516](matrix-org/synapse#14516))
- Add more prompts to the bug report form. ([\#14522](matrix-org/synapse#14522))
- Extend editorconfig rules on indent and line length to `.pyi` files. ([\#14526](matrix-org/synapse#14526))
- Run Rust CI when `Cargo.lock` changes. This is particularly useful for dependabot updates. ([\#14571](matrix-org/synapse#14571))
- Fix a possible variable shadow in `create_new_client_event`. ([\#14575](matrix-org/synapse#14575))
- Bump various dependencies in the `poetry.lock` file and in CI scripts. ([\#14557](matrix-org/synapse#14557), [\#14559](matrix-org/synapse#14559), [\#14560](matrix-org/synapse#14560), [\#14500](matrix-org/synapse#14500), [\#14501](matrix-org/synapse#14501), [\#14502](matrix-org/synapse#14502), [\#14503](matrix-org/synapse#14503), [\#14504](matrix-org/synapse#14504), [\#14505](matrix-org/synapse#14505)).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get rid of worker_main_http_uri
5 participants