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

Remove worker_replication_* deprecated settings, with helpful errors on startup #15860

Merged
merged 11 commits into from Jul 7, 2023
Merged

Remove worker_replication_* deprecated settings, with helpful errors on startup #15860

merged 11 commits into from Jul 7, 2023

Conversation

realtyem
Copy link
Contributor

@realtyem realtyem commented Jul 2, 2023 •
edited

What it says on the tin. Needs Sytest modifications. Needs #sytest/1357

I'm not going to mark this as fixing the issue that's related, but if we could update that and pin it for another month, I'll finish the removal then.

Pull Request Checklist

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

@realtyem
Remove the deprecated settings for worker_replication_*, but leave Co…
6ca6200 
…nfigError statements to gently push admins in the right direction
@realtyem realtyem force-pushed the remove-worker-replication-deprecated-settings branch from c7a2ee9 to 8c760f4 Compare July 2, 2023 01:47
@realtyem realtyem marked this pull request as ready for review July 2, 2023 01:48
@realtyem realtyem requested a review from a team as a code owner July 2, 2023 01:48
@realtyem realtyem mentioned this pull request Jul 2, 2023
Merged
reivilibre
reivilibre reviewed Jul 3, 2023
View reviewed changes
Copy link
Contributor

@reivilibre reivilibre left a comment

Choose a reason for hiding this comment

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

seems fine, will poke CI to rerun

docs/upgrade.md Outdated
# Upgrading to v1.88.0

## Removal of `worker_replication_*` settings
As mentioned in v1.84.0, these settings are deprecated and are being removed. Ensure you
Copy link
Contributor

Choose a reason for hiding this comment

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

a link to the section showing you how to migrate would be useful

Copy link
Contributor Author

@realtyem realtyem Jul 3, 2023 •
edited

Choose a reason for hiding this comment

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

Attempted fix in be0bb9e

changelog.d/15860.removal Outdated Show resolved Hide resolved
@realtyem @reivilibre
Update changelog.d/15860.removal
8e38741 
Co-authored-by: reivilibre <olivier@librepush.net>
@realtyem realtyem force-pushed the remove-worker-replication-deprecated-settings branch from c352219 to be0bb9e Compare July 3, 2023 21:20
@realtyem realtyem requested a review from reivilibre July 4, 2023 09:06
MatMaul
MatMaul reviewed Jul 4, 2023
View reviewed changes
docs/workers.md Outdated
@@ -147,7 +147,9 @@ In the config file for each worker, you must specify:
the main process (`worker_main_http_uri`). This config option is no longer required and is ignored when running Synapse 1.73 and newer.
* **Synapse 1.83 and older:** The HTTP replication endpoint that the worker should talk to on the main synapse process
([`worker_replication_host`](usage/configuration/config_documentation.md#worker_replication_host) and
[`worker_replication_http_port`](usage/configuration/config_documentation.md#worker_replication_http_port)). If using Synapse 1.84 and newer, these are not needed if `main` is defined on the [shared configuration](#shared-configuration) `instance_map`
[`worker_replication_http_port`](usage/configuration/config_documentation.md#worker_replication_http_port)). If
Copy link
Contributor

@MatMaul MatMaul Jul 4, 2023 •
edited

Choose a reason for hiding this comment

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

I would probably just have nuked those from this doc. Upgrade guide is wordy and good enough I feel. Plus we have the error shouted at the operator in the code itself if the old params are in use.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, we should remove the removed options from the manual. They are still available in the old versions of the manual for people who really need them.

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'm onboard, but want to make sure not to just take an eraser to the whole thing yet, as there will invariably be someone who misses checking upgrade notes.

There are a couple places where this is mentioned:

  1. workers.md:
    • This spot, actually. The brief summary of what you need to get a worker up and running.
  2. config_documentation.md

So, number 2 I'm good with losing now. And I can update the links in 1 to point at the old manual. Is that what you mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

OMG yeah the itemized list too... Please nuke those for sure, personally I would also nuke legacy from workers doc but I'll survive if it stays there :)

I think legacy should be dealt with the upgrade notes, with a warning when we encounter an unknown option to check the notes. But we can do that later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There. I just took it all out. Since it is preserved in old versions of the manual and is on the upgrade notes, I guess that's it?

Removed in a99cdac

@reivilibre reivilibre changed the title Transition worker_replication_* deprecated settings before full removal Remove worker_replication_* deprecated settings, with helpful errors on startup Jul 4, 2023
@realtyem realtyem requested a review from MatMaul July 7, 2023 00:12
MatMaul
MatMaul approved these changes Jul 7, 2023
View reviewed changes
Copy link
Contributor

@MatMaul MatMaul left a comment

Choose a reason for hiding this comment

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

LGTM, thanks !

@MatMaul MatMaul enabled auto-merge (squash) July 7, 2023 07:18
@MatMaul MatMaul mentioned this pull request Jul 7, 2023
Merged
4 tasks
@MatMaul MatMaul merged commit 2481b7d into matrix-org:develop Jul 7, 2023
39 checks passed
yingziwu added a commit to yingziwu/synapse that referenced this pull request Jul 19, 2023
@yingziwu
Merge tag 'v1.88.0' into develop
55a9f8a 
This release
 - raises the minimum supported version of Python to 3.8, as Python 3.7 is now [end-of-life](https://devguide.python.org/versions/), and
 - removes deprecated config options related to worker deployment.

See [the upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.88/docs/upgrade.md#upgrading-to-v1880) for more information.

- Revert "Stop writing to column `user_id` of tables `profiles` and `user_filters`", which was introduced in Synapse 1.88.0rc1. ([\matrix-org#15953](matrix-org#15953))

- Add `not_user_type` param to the [list accounts admin API](https://matrix-org.github.io/synapse/v1.88/admin_api/user_admin_api.html#list-accounts). ([\matrix-org#15844](matrix-org#15844))

- Pin `pydantic` to `^=1.7.4` to avoid backwards-incompatible API changes from the 2.0.0 release.
  Contributed by @PaarthShah. ([\matrix-org#15862](matrix-org#15862))
- Correctly resize thumbnails with pillow version >=10. ([\matrix-org#15876](matrix-org#15876))

- Fixed header levels on the [Admin API "Users"](https://matrix-org.github.io/synapse/v1.87/admin_api/user_admin_api.html) documentation page. Contributed by @sumnerevans at @beeper. ([\matrix-org#15852](matrix-org#15852))
- Remove deprecated `worker_replication_host`, `worker_replication_http_port` and `worker_replication_http_tls` configuration options. ([\matrix-org#15872](matrix-org#15872))

- **Remove deprecated `worker_replication_host`, `worker_replication_http_port` and `worker_replication_http_tls` configuration options.** See the [upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.88/docs/upgrade.md#removal-of-worker_replication_-settings) for more details. ([\matrix-org#15860](matrix-org#15860))
- Remove support for Python 3.7 and hence for Debian Buster. ([\matrix-org#15851](matrix-org#15851), [\matrix-org#15892](matrix-org#15892), [\matrix-org#15893](matrix-org#15893), [\matrix-org#15917](matrix-org#15917))

- Add foreign key constraint to `event_forward_extremities`. ([\matrix-org#15751](matrix-org#15751), [\matrix-org#15907](matrix-org#15907))
- Add read/write style cross-worker locks. ([\matrix-org#15782](matrix-org#15782))
- Stop writing to column `user_id` of tables `profiles` and `user_filters`. ([\matrix-org#15787](matrix-org#15787))
- Use lower isolation level when cleaning old presence stream data to avoid serialization errors. ([\matrix-org#15826](matrix-org#15826))
- Add tracing to media `/upload` code paths. ([\matrix-org#15850](matrix-org#15850), [\matrix-org#15888](matrix-org#15888))
- Add a timeout that aborts any Postgres statement taking more than 1 hour. ([\matrix-org#15853](matrix-org#15853))
- Fix the `devenv up` configuration which was ignoring the config overrides. ([\matrix-org#15854](matrix-org#15854))
- Optimised cleanup of old entries in `device_lists_stream`. ([\matrix-org#15861](matrix-org#15861))
- Update the Matrix clients link in the _It works! Synapse is running_ landing page. ([\matrix-org#15874](matrix-org#15874))
- Fix building Synapse with the nightly Rust compiler. ([\matrix-org#15906](matrix-org#15906))
- Add `Server` to Access-Control-Expose-Headers header. ([\matrix-org#15908](matrix-org#15908))

* Bump authlib from 1.2.0 to 1.2.1. ([\matrix-org#15864](matrix-org#15864))
* Bump importlib-metadata from 6.6.0 to 6.7.0. ([\matrix-org#15865](matrix-org#15865))
* Bump lxml from 4.9.2 to 4.9.3. ([\matrix-org#15897](matrix-org#15897))
* Bump regex from 1.8.4 to 1.9.1. ([\matrix-org#15902](matrix-org#15902))
* Bump ruff from 0.0.275 to 0.0.277. ([\matrix-org#15900](matrix-org#15900))
* Bump sentry-sdk from 1.25.1 to 1.26.0. ([\matrix-org#15867](matrix-org#15867))
* Bump serde_json from 1.0.99 to 1.0.100. ([\matrix-org#15901](matrix-org#15901))
* Bump types-pyopenssl from 23.2.0.0 to 23.2.0.1. ([\matrix-org#15866](matrix-org#15866))
Fizzadar added a commit to beeper/synapse-legacy-fork that referenced this pull request Jul 27, 2023
@Fizzadar
Merge tag 'v1.88.0' into merge-1.88
f2d2258 
This release
 - raises the minimum supported version of Python to 3.8, as Python 3.7 is now [end-of-life](https://devguide.python.org/versions/), and
 - removes deprecated config options related to worker deployment.

See [the upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.88/docs/upgrade.md#upgrading-to-v1880) for more information.

- Revert "Stop writing to column `user_id` of tables `profiles` and `user_filters`", which was introduced in Synapse 1.88.0rc1. ([\matrix-org#15953](matrix-org#15953))

- Add `not_user_type` param to the [list accounts admin API](https://matrix-org.github.io/synapse/v1.88/admin_api/user_admin_api.html#list-accounts). ([\matrix-org#15844](matrix-org#15844))

- Pin `pydantic` to `^=1.7.4` to avoid backwards-incompatible API changes from the 2.0.0 release.
  Contributed by @PaarthShah. ([\matrix-org#15862](matrix-org#15862))
- Correctly resize thumbnails with pillow version >=10. ([\matrix-org#15876](matrix-org#15876))

- Fixed header levels on the [Admin API "Users"](https://matrix-org.github.io/synapse/v1.87/admin_api/user_admin_api.html) documentation page. Contributed by @sumnerevans at @beeper. ([\matrix-org#15852](matrix-org#15852))
- Remove deprecated `worker_replication_host`, `worker_replication_http_port` and `worker_replication_http_tls` configuration options. ([\matrix-org#15872](matrix-org#15872))

- **Remove deprecated `worker_replication_host`, `worker_replication_http_port` and `worker_replication_http_tls` configuration options.** See the [upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.88/docs/upgrade.md#removal-of-worker_replication_-settings) for more details. ([\matrix-org#15860](matrix-org#15860))
- Remove support for Python 3.7 and hence for Debian Buster. ([\matrix-org#15851](matrix-org#15851), [\matrix-org#15892](matrix-org#15892), [\matrix-org#15893](matrix-org#15893), [\matrix-org#15917](matrix-org#15917))

- Add foreign key constraint to `event_forward_extremities`. ([\matrix-org#15751](matrix-org#15751), [\matrix-org#15907](matrix-org#15907))
- Add read/write style cross-worker locks. ([\matrix-org#15782](matrix-org#15782))
- Stop writing to column `user_id` of tables `profiles` and `user_filters`. ([\matrix-org#15787](matrix-org#15787))
- Use lower isolation level when cleaning old presence stream data to avoid serialization errors. ([\matrix-org#15826](matrix-org#15826))
- Add tracing to media `/upload` code paths. ([\matrix-org#15850](matrix-org#15850), [\matrix-org#15888](matrix-org#15888))
- Add a timeout that aborts any Postgres statement taking more than 1 hour. ([\matrix-org#15853](matrix-org#15853))
- Fix the `devenv up` configuration which was ignoring the config overrides. ([\matrix-org#15854](matrix-org#15854))
- Optimised cleanup of old entries in `device_lists_stream`. ([\matrix-org#15861](matrix-org#15861))
- Update the Matrix clients link in the _It works! Synapse is running_ landing page. ([\matrix-org#15874](matrix-org#15874))
- Fix building Synapse with the nightly Rust compiler. ([\matrix-org#15906](matrix-org#15906))
- Add `Server` to Access-Control-Expose-Headers header. ([\matrix-org#15908](matrix-org#15908))

* Bump authlib from 1.2.0 to 1.2.1. ([\matrix-org#15864](matrix-org#15864))
* Bump importlib-metadata from 6.6.0 to 6.7.0. ([\matrix-org#15865](matrix-org#15865))
* Bump lxml from 4.9.2 to 4.9.3. ([\matrix-org#15897](matrix-org#15897))
* Bump regex from 1.8.4 to 1.9.1. ([\matrix-org#15902](matrix-org#15902))
* Bump ruff from 0.0.275 to 0.0.277. ([\matrix-org#15900](matrix-org#15900))
* Bump sentry-sdk from 1.25.1 to 1.26.0. ([\matrix-org#15867](matrix-org#15867))
* Bump serde_json from 1.0.99 to 1.0.100. ([\matrix-org#15901](matrix-org#15901))
* Bump types-pyopenssl from 23.2.0.0 to 23.2.0.1. ([\matrix-org#15866](matrix-org#15866))

# -----BEGIN PGP SIGNATURE-----
#
# iQIzBAABCAAdFiEE8SRSDO7gYkSP4chELS76LzL74EcFAmS2ncYACgkQLS76LzL7
# 4EfaiA/9HUdk1tIlnQyIZDAT0d9RhmaL+KL1Fkk4wpi16QrtJ7gqLTrq1BXDxOYM
# 2bycL0yHN9gPu3f7TI0ic4p17T/vud8Fd7FoPJTkUZ7dFHsEPICtNmIp6ZkpuDYW
# Sv8QKuEeMxe98XCKxiI+zctu8wtNsrnu2RECD0zUqf5rMgbabuYnpSge2CqKftuf
# CfmYN161QjnONavQTk4iYSFmJpRZwvwoAlpMPsqkMIrhId2ko2SkPj1HBPrAFrFs
# Fq/PaVZQRZk15wnQzrLrlRAEfq/quoOSDnJSlUvMPWjsQUVP8Ug149L9oUIiwhqq
# zQtuL9dl5xGoUWMiWOP8937gVeA/lsJpcVPka3G0g3mIR8ukbHUOm2fZReV30xp8
# 81xpu8KwzDR+/Oo3INYsqoOiQ/t7Myg8sTgiJBMParuRmfqnsbdUWG8pEeUMzDYY
# 4+yzRrHo9KnHcAMEFT94rqVhgl7OQh2Fx9zduW7YOVk0wo0EBMl/rcfEsvvl//l0
# sSykqGx1XIr3Cdynp1PjCYJsYdLJzG73aSVh5qPY5sftsHIQ8DiiKX+UlSqlguMW
# 1ndkCuz3fK/+9CFGbBiixe/RKFxn0CVp3cBU6cCVzyLJerZpXrOkyMmSuZtOuIaH
# cLLGK2bQSbOegPtg4qjpL537xmk94tUiSGhEdt7M4wsHm4uRv0M=
# =QYwZ
# -----END PGP SIGNATURE-----
# gpg: Signature made Tue Jul 18 15:12:22 2023 BST
# gpg:                using RSA key F124520CEEE062448FE1C8442D2EFA2F32FBE047
# gpg: Can't check signature: No public key

# Conflicts:
#	.github/workflows/latest_deps.yml
#	.github/workflows/release-artifacts.yml
#	.github/workflows/tests.yml
#	.github/workflows/twisted_trunk.yml
#	docker/Dockerfile
#	poetry.lock
#	synapse/api/auth/base.py
#	synapse/config/experimental.py
#	synapse/handlers/pagination.py
#	synapse/handlers/room_batch.py
#	synapse/push/bulk_push_rule_evaluator.py
#	synapse/rest/admin/users.py
#	synapse/rest/client/room_batch.py
#	synapse/storage/databases/main/__init__.py
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MatMaul MatMaul MatMaul approved these changes

@reivilibre reivilibre Awaiting requested review from reivilibre

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@realtyem @MatMaul @reivilibre