From 7c1c011942ebeb7ab781ccb3640922290159a98c Mon Sep 17 00:00:00 2001 From: Georg Date: Tue, 20 Feb 2024 17:15:58 +0100 Subject: [PATCH 01/17] Add HAProxy example for single port operation (#16768) --- changelog.d/16768.doc | 1 + docs/reverse_proxy.md | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 changelog.d/16768.doc diff --git a/changelog.d/16768.doc b/changelog.d/16768.doc new file mode 100644 index 000000000..4f574c2ac --- /dev/null +++ b/changelog.d/16768.doc @@ -0,0 +1 @@ +Add HAProxy example for single port operation to reverse proxy documentation. Contributed by Georg Pfuetzenreuter (@tacerus). diff --git a/docs/reverse_proxy.md b/docs/reverse_proxy.md index de72fbde9..7128af114 100644 --- a/docs/reverse_proxy.md +++ b/docs/reverse_proxy.md @@ -186,6 +186,25 @@ Example configuration, if using a UNIX socket. The configuration lines regarding backend matrix server matrix unix@/run/synapse/main_public.sock ``` +Example configuration when using a single port for both client and federation traffic. +``` +frontend https + bind *:443,[::]:443 ssl crt /etc/ssl/haproxy/ strict-sni alpn h2,http/1.1 + http-request set-header X-Forwarded-Proto https if { ssl_fc } + http-request set-header X-Forwarded-Proto http if !{ ssl_fc } + http-request set-header X-Forwarded-For %[src] + + acl matrix-host hdr(host) -i matrix.example.com matrix.example.com:443 + acl matrix-sni ssl_fc_sni matrix.example.com + acl matrix-path path_beg /_matrix + acl matrix-path path_beg /_synapse/client + + use_backend matrix if matrix-host matrix-path + use_backend matrix if matrix-sni + +backend matrix + server matrix 127.0.0.1:8008 +``` [Delegation](delegate.md) example: ``` From 0c55c76da807c8ca0c37b7394594d3757ffab8a6 Mon Sep 17 00:00:00 2001 From: kegsay <7190048+kegsay@users.noreply.github.com> Date: Tue, 20 Feb 2024 17:14:50 +0000 Subject: [PATCH 02/17] Better complement docs (#16946) --- changelog.d/16946.doc | 1 + docker/complement/README.md | 11 +++++++++++ 2 files changed, 12 insertions(+) create mode 100644 changelog.d/16946.doc diff --git a/changelog.d/16946.doc b/changelog.d/16946.doc new file mode 100644 index 000000000..8037a6545 --- /dev/null +++ b/changelog.d/16946.doc @@ -0,0 +1 @@ +Improve the documentation around running Complement tests with new configuration parameters. \ No newline at end of file diff --git a/docker/complement/README.md b/docker/complement/README.md index 62682219e..1ce841281 100644 --- a/docker/complement/README.md +++ b/docker/complement/README.md @@ -30,3 +30,14 @@ Consult `scripts-dev/complement.sh` in the repository root for a real example. [complement]: https://github.com/matrix-org/complement [complementEnv]: https://github.com/matrix-org/complement/pull/382 + +## How to modify homeserver.yaml for Complement tests + +It's common for MSCs to be gated behind a feature flag like this: +```yaml +experimental_features: + faster_joins: true +``` +To modify this for the Complement image, modify `./conf/workers-shared-extra.yaml.j2`. Despite the name, +this will affect non-worker mode as well. Remember to _rebuild_ the image (so don't use `-e` if using +`complement.sh`). From e0b19a4777559110b46ae5268a82b654acddf8c6 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 21 Feb 2024 10:34:03 +0000 Subject: [PATCH 03/17] Bump dawidd6/action-download-artifact from 3.0.0 to 3.1.1 (#16933) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/docs-pr-netlify.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/docs-pr-netlify.yaml b/.github/workflows/docs-pr-netlify.yaml index 4c0a7989a..c42077d3a 100644 --- a/.github/workflows/docs-pr-netlify.yaml +++ b/.github/workflows/docs-pr-netlify.yaml @@ -14,7 +14,7 @@ jobs: # There's a 'download artifact' action, but it hasn't been updated for the workflow_run action # (https://github.com/actions/download-artifact/issues/60) so instead we get this mess: - name: 📥 Download artifact - uses: dawidd6/action-download-artifact@e7466d1a7587ed14867642c2ca74b5bcc1e19a2d # v3.0.0 + uses: dawidd6/action-download-artifact@72aaadce3bc708349fc665eee3785cbb1b6e51d0 # v3.1.1 with: workflow: docs-pr.yaml run_id: ${{ github.event.workflow_run.id }} From 91694907da1a9484464fe40ca1401dc3e3272e11 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 21 Feb 2024 10:35:05 +0000 Subject: [PATCH 04/17] Bump furo from 2023.9.10 to 2024.1.29 (#16939) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- poetry.lock | 8 ++++---- pyproject.toml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/poetry.lock b/poetry.lock index 834d6512d..3fa49250c 100644 --- a/poetry.lock +++ b/poetry.lock @@ -559,13 +559,13 @@ dev = ["Sphinx", "coverage", "flake8", "lxml", "lxml-stubs", "memory-profiler", [[package]] name = "furo" -version = "2023.9.10" +version = "2024.1.29" description = "A clean customisable Sphinx documentation theme." optional = false python-versions = ">=3.8" files = [ - {file = "furo-2023.9.10-py3-none-any.whl", hash = "sha256:513092538537dc5c596691da06e3c370714ec99bc438680edc1debffb73e5bfc"}, - {file = "furo-2023.9.10.tar.gz", hash = "sha256:5707530a476d2a63b8cad83b4f961f3739a69f4b058bcf38a03a39fa537195b2"}, + {file = "furo-2024.1.29-py3-none-any.whl", hash = "sha256:3548be2cef45a32f8cdc0272d415fcb3e5fa6a0eb4ddfe21df3ecf1fe45a13cf"}, + {file = "furo-2024.1.29.tar.gz", hash = "sha256:4d6b2fe3f10a6e36eb9cc24c1e7beb38d7a23fc7b3c382867503b7fcac8a1e02"}, ] [package.dependencies] @@ -3434,4 +3434,4 @@ user-search = ["pyicu"] [metadata] lock-version = "2.0" python-versions = "^3.8.0" -content-hash = "053bda662e95c273f4eda41d7ece8de0e404783ac66d54cdbedc396e196fb63a" +content-hash = "e4ca55af1dcb6b28b8064b7551008fd16f6cdfa9cb9bf90d18c6b47766b56ae6" diff --git a/pyproject.toml b/pyproject.toml index e1ffb9ffe..7735f0826 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -372,7 +372,7 @@ optional = true sphinx = {version = "^6.1", python = "^3.8"} sphinx-autodoc2 = {version = ">=0.4.2,<0.6.0", python = "^3.8"} myst-parser = {version = "^1.0.0", python = "^3.8"} -furo = ">=2022.12.7,<2024.0.0" +furo = ">=2022.12.7,<2025.0.0" [build-system] From f2c5f1564ef73550178366263f7b9214577dc0a6 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 21 Feb 2024 10:36:35 +0000 Subject: [PATCH 05/17] Bump types-netaddr from 0.10.0.20240106 to 1.2.0.20240219 (#16938) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- poetry.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/poetry.lock b/poetry.lock index 3fa49250c..d183efd39 100644 --- a/poetry.lock +++ b/poetry.lock @@ -3070,13 +3070,13 @@ referencing = "*" [[package]] name = "types-netaddr" -version = "0.10.0.20240106" +version = "1.2.0.20240219" description = "Typing stubs for netaddr" optional = false python-versions = ">=3.8" files = [ - {file = "types-netaddr-0.10.0.20240106.tar.gz", hash = "sha256:7cc6c16bc76f57faf4a042184f748a05e9642b189caf7fe7e36c07cb87c057b3"}, - {file = "types_netaddr-0.10.0.20240106-py3-none-any.whl", hash = "sha256:0acd8116293b06abe66484cf033c2d597f039326c28e3df83b8abd5709f6c65d"}, + {file = "types-netaddr-1.2.0.20240219.tar.gz", hash = "sha256:984e70ad838218d3032f37f05a7e294f7b007fe274ec9d774265c8c06698395f"}, + {file = "types_netaddr-1.2.0.20240219-py3-none-any.whl", hash = "sha256:b26144e878acb8a1a9008e6997863714db04f8029a0f7f6bfe483c977d21b522"}, ] [[package]] From 5ce949804755465050800d248523b40641d48faf Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 21 Feb 2024 10:37:32 +0000 Subject: [PATCH 06/17] Bump JasonEtco/create-an-issue from 2.9.1 to 2.9.2 (#16934) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/latest_deps.yml | 2 +- .github/workflows/twisted_trunk.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/latest_deps.yml b/.github/workflows/latest_deps.yml index a754515c9..b9e9a401b 100644 --- a/.github/workflows/latest_deps.yml +++ b/.github/workflows/latest_deps.yml @@ -226,7 +226,7 @@ jobs: steps: - uses: actions/checkout@v4 - - uses: JasonEtco/create-an-issue@e27dddc79c92bc6e4562f268fffa5ed752639abd # v2.9.1 + - uses: JasonEtco/create-an-issue@1b14a70e4d8dc185e5cc76d3bec9eab20257b2c5 # v2.9.2 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: diff --git a/.github/workflows/twisted_trunk.yml b/.github/workflows/twisted_trunk.yml index b7b93b356..76609c211 100644 --- a/.github/workflows/twisted_trunk.yml +++ b/.github/workflows/twisted_trunk.yml @@ -207,7 +207,7 @@ jobs: steps: - uses: actions/checkout@v4 - - uses: JasonEtco/create-an-issue@e27dddc79c92bc6e4562f268fffa5ed752639abd # v2.9.1 + - uses: JasonEtco/create-an-issue@1b14a70e4d8dc185e5cc76d3bec9eab20257b2c5 # v2.9.2 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: From 3778cea776e2d9ec0685801c1d37031057c53820 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 21 Feb 2024 10:38:38 +0000 Subject: [PATCH 07/17] Bump pyopenssl from 23.3.0 to 24.0.0 (#16937) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- poetry.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/poetry.lock b/poetry.lock index d183efd39..9257d2ccf 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2081,17 +2081,17 @@ tests = ["hypothesis (>=3.27.0)", "pytest (>=3.2.1,!=3.3.0)"] [[package]] name = "pyopenssl" -version = "23.3.0" +version = "24.0.0" description = "Python wrapper module around the OpenSSL library" optional = false python-versions = ">=3.7" files = [ - {file = "pyOpenSSL-23.3.0-py3-none-any.whl", hash = "sha256:6756834481d9ed5470f4a9393455154bc92fe7a64b7bc6ee2c804e78c52099b2"}, - {file = "pyOpenSSL-23.3.0.tar.gz", hash = "sha256:6b2cba5cc46e822750ec3e5a81ee12819850b11303630d575e98108a079c2b12"}, + {file = "pyOpenSSL-24.0.0-py3-none-any.whl", hash = "sha256:ba07553fb6fd6a7a2259adb9b84e12302a9a8a75c44046e8bb5d3e5ee887e3c3"}, + {file = "pyOpenSSL-24.0.0.tar.gz", hash = "sha256:6aa33039a93fffa4563e655b61d11364d01264be8ccb49906101e02a334530bf"}, ] [package.dependencies] -cryptography = ">=41.0.5,<42" +cryptography = ">=41.0.5,<43" [package.extras] docs = ["sphinx (!=5.2.0,!=5.2.0.post0,!=7.2.5)", "sphinx-rtd-theme"] From 4ad70f115b670a8ef964afb3ab9fbb30ecafc3c2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 21 Feb 2024 10:40:34 +0000 Subject: [PATCH 08/17] Bump anyhow from 1.0.79 to 1.0.80 (#16935) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b1568dc7d..d41a498fc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13,9 +13,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.79" +version = "1.0.80" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "080e9890a082662b09c1ad45f567faeeb47f22b5fb23895fbe1e651e718e25ca" +checksum = "5ad32ce52e4161730f7098c077cd2ed6229b5804ccf99e5366be1ab72a98b4e1" [[package]] name = "arc-swap" From 8de3283ebe520e945ccfb66242f4795ecdd0d1f4 Mon Sep 17 00:00:00 2001 From: Twilight Sparkle <19155609+Twi1ightSparkle@users.noreply.github.com> Date: Thu, 22 Feb 2024 17:36:41 +0000 Subject: [PATCH 09/17] Add docs on upgrading from a very old version (#16951) Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- changelog.d/16951.doc | 1 + docs/upgrade.md | 20 ++++++++++++++++++++ docs/usage/administration/admin_faq.md | 5 +++++ 3 files changed, 26 insertions(+) create mode 100644 changelog.d/16951.doc diff --git a/changelog.d/16951.doc b/changelog.d/16951.doc new file mode 100644 index 000000000..a7fb33979 --- /dev/null +++ b/changelog.d/16951.doc @@ -0,0 +1 @@ +Add docs on upgrading from a very old version. \ No newline at end of file diff --git a/docs/upgrade.md b/docs/upgrade.md index 7f67ef880..640fed3ae 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -97,6 +97,26 @@ v1.61.0. +## Upgrading from a very old version + +You need to read all of the upgrade notes for each version between your current +version and the latest so that you can update your dependencies, environment, +config files, etc. if necessary. But you do not need to perform an +upgrade to each individual version that was missed. + +We do not have a list of which versions must be installed. Instead, we recommend +that you upgrade through each incompatible database schema version, which would +give you the ability to roll back the maximum number of versions should anything +go wrong. See [Rolling back to older versions](#rolling-back-to-older-versions) +above. + +Additionally, new versions of Synapse will occasionally run database migrations +and background updates to update the database. Synapse will not start until +database migrations are complete. You should wait until background updates from +each upgrade are complete before moving on to the next upgrade, to avoid +stacking them up. You can monitor the currently running background updates with +[the Admin API](usage/administration/admin_api/background_updates.html#status). + # Upgrading to v1.100.0 ## Minimum supported Rust version diff --git a/docs/usage/administration/admin_faq.md b/docs/usage/administration/admin_faq.md index 5c9ee7d0a..092dcc1c8 100644 --- a/docs/usage/administration/admin_faq.md +++ b/docs/usage/administration/admin_faq.md @@ -120,6 +120,11 @@ for file in $source_directory/*; do done ``` +How do I upgrade from a very old version of Synapse to the latest? +--- +See [this](../../upgrade.html#upgrading-from-a-very-old-version) section in the +upgrade docs. + Manually resetting passwords --- Users can reset their password through their client. Alternatively, a server admin From 274f289a52a0097784d7179c1d912dbc3cfda3ee Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Fri, 23 Feb 2024 14:12:10 +0000 Subject: [PATCH 10/17] Ignore notification counts from rooms you've left (#16954) Co-authored-by: reivilibre --- changelog.d/16954.bugfix | 1 + .../databases/main/event_push_actions.py | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 changelog.d/16954.bugfix diff --git a/changelog.d/16954.bugfix b/changelog.d/16954.bugfix new file mode 100644 index 000000000..7e5ad6909 --- /dev/null +++ b/changelog.d/16954.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in v1.100.0 where notifications from rooms you've left would continue to be counted. \ No newline at end of file diff --git a/synapse/storage/databases/main/event_push_actions.py b/synapse/storage/databases/main/event_push_actions.py index d7aa8a0ee..56c549ae6 100644 --- a/synapse/storage/databases/main/event_push_actions.py +++ b/synapse/storage/databases/main/event_push_actions.py @@ -404,7 +404,11 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas SELECT e.room_id, notif_count, e.stream_ordering, e.thread_id, last_receipt_stream_ordering, ev.stream_ordering AS receipt_stream_ordering FROM event_push_summary AS e - INNER JOIN local_current_membership USING (user_id, room_id) + INNER JOIN local_current_membership AS lcm ON ( + e.user_id = lcm.user_id + AND e.room_id = lcm.room_id + AND lcm.membership = 'join' + ) LEFT JOIN receipts_linearized AS r ON ( e.user_id = r.user_id AND e.room_id = r.room_id @@ -472,7 +476,11 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas SELECT e.room_id, e.stream_ordering, e.thread_id, ev.stream_ordering AS receipt_stream_ordering FROM event_push_actions AS e - INNER JOIN local_current_membership USING (user_id, room_id) + INNER JOIN local_current_membership AS lcm ON ( + e.user_id = lcm.user_id + AND e.room_id = lcm.room_id + AND lcm.membership = 'join' + ) LEFT JOIN receipts_linearized AS r ON ( e.user_id = r.user_id AND e.room_id = r.room_id @@ -514,7 +522,11 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas SELECT e.room_id, e.stream_ordering, e.thread_id, ev.stream_ordering AS receipt_stream_ordering FROM event_push_actions AS e - INNER JOIN local_current_membership USING (user_id, room_id) + INNER JOIN local_current_membership AS lcm ON ( + e.user_id = lcm.user_id + AND e.room_id = lcm.room_id + AND lcm.membership = 'join' + ) LEFT JOIN receipts_linearized AS r ON ( e.user_id = r.user_id AND e.room_id = r.room_id From ab80b3412e314b25827bcc5c61628e024c8cf571 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Tue, 5 Mar 2024 16:02:54 +0000 Subject: [PATCH 11/17] Revert "Ignore notification counts from rooms you've left" (#16981) --- changelog.d/16954.bugfix | 1 - .../databases/main/event_push_actions.py | 18 +++--------------- 2 files changed, 3 insertions(+), 16 deletions(-) delete mode 100644 changelog.d/16954.bugfix diff --git a/changelog.d/16954.bugfix b/changelog.d/16954.bugfix deleted file mode 100644 index 7e5ad6909..000000000 --- a/changelog.d/16954.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix a bug introduced in v1.100.0 where notifications from rooms you've left would continue to be counted. \ No newline at end of file diff --git a/synapse/storage/databases/main/event_push_actions.py b/synapse/storage/databases/main/event_push_actions.py index 56c549ae6..d7aa8a0ee 100644 --- a/synapse/storage/databases/main/event_push_actions.py +++ b/synapse/storage/databases/main/event_push_actions.py @@ -404,11 +404,7 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas SELECT e.room_id, notif_count, e.stream_ordering, e.thread_id, last_receipt_stream_ordering, ev.stream_ordering AS receipt_stream_ordering FROM event_push_summary AS e - INNER JOIN local_current_membership AS lcm ON ( - e.user_id = lcm.user_id - AND e.room_id = lcm.room_id - AND lcm.membership = 'join' - ) + INNER JOIN local_current_membership USING (user_id, room_id) LEFT JOIN receipts_linearized AS r ON ( e.user_id = r.user_id AND e.room_id = r.room_id @@ -476,11 +472,7 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas SELECT e.room_id, e.stream_ordering, e.thread_id, ev.stream_ordering AS receipt_stream_ordering FROM event_push_actions AS e - INNER JOIN local_current_membership AS lcm ON ( - e.user_id = lcm.user_id - AND e.room_id = lcm.room_id - AND lcm.membership = 'join' - ) + INNER JOIN local_current_membership USING (user_id, room_id) LEFT JOIN receipts_linearized AS r ON ( e.user_id = r.user_id AND e.room_id = r.room_id @@ -522,11 +514,7 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas SELECT e.room_id, e.stream_ordering, e.thread_id, ev.stream_ordering AS receipt_stream_ordering FROM event_push_actions AS e - INNER JOIN local_current_membership AS lcm ON ( - e.user_id = lcm.user_id - AND e.room_id = lcm.room_id - AND lcm.membership = 'join' - ) + INNER JOIN local_current_membership USING (user_id, room_id) LEFT JOIN receipts_linearized AS r ON ( e.user_id = r.user_id AND e.room_id = r.room_id From 4af33015af280b4716e812e47d5631fcac088128 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Wed, 6 Mar 2024 16:00:20 +0100 Subject: [PATCH 12/17] Fix joining remote rooms when a `on_new_event` callback is registered (#16973) Since Synapse 1.76.0, any module which registers a `on_new_event` callback would brick the ability to join remote rooms. This is because this callback tried to get the full state of the room, which would end up in a deadlock. Related: https://github.com/matrix-org/synapse-auto-accept-invite/issues/18 The following module would brick the ability to join remote rooms: ```python from typing import Any, Dict, Literal, Union import logging from synapse.module_api import ModuleApi, EventBase logger = logging.getLogger(__name__) class MyModule: def __init__(self, config: None, api: ModuleApi): self._api = api self._config = config self._api.register_third_party_rules_callbacks( on_new_event=self.on_new_event, ) async def on_new_event(self, event: EventBase, _state_map: Any) -> None: logger.info(f"Received new event: {event}") @staticmethod def parse_config(_config: Dict[str, Any]) -> None: return None ``` This is technically a breaking change, as we are now passing partial state on the `on_new_event` callback. However, this callback was broken for federated rooms since 1.76.0, and local rooms have full state anyway, so it's unlikely that it would change anything. --- changelog.d/16973.bugfix | 1 + docs/modules/third_party_rules_callbacks.md | 4 ++++ .../third_party_event_rules_callbacks.py | 23 ++++++++----------- synapse/storage/controllers/state.py | 9 ++++++-- 4 files changed, 21 insertions(+), 16 deletions(-) create mode 100644 changelog.d/16973.bugfix diff --git a/changelog.d/16973.bugfix b/changelog.d/16973.bugfix new file mode 100644 index 000000000..a0e3a1230 --- /dev/null +++ b/changelog.d/16973.bugfix @@ -0,0 +1 @@ +Fix joining remote rooms when a module uses the `on_new_event` callback. This callback may now pass partial state events instead of the full state for remote rooms. diff --git a/docs/modules/third_party_rules_callbacks.md b/docs/modules/third_party_rules_callbacks.md index d06bff25e..b97e28db1 100644 --- a/docs/modules/third_party_rules_callbacks.md +++ b/docs/modules/third_party_rules_callbacks.md @@ -142,6 +142,10 @@ Called after sending an event into a room. The module is passed the event, as we as the state of the room _after_ the event. This means that if the event is a state event, it will be included in this state. +The state map may not be complete if Synapse hasn't yet loaded the full state +of the room. This can happen for events in rooms that were just joined from +a remote server. + Note that this callback is called when the event has already been processed and stored into the room, which means this callback cannot be used to deny persisting the event. To deny an incoming event, see [`check_event_for_spam`](spam_checker_callbacks.md#check_event_for_spam) instead. diff --git a/synapse/module_api/callbacks/third_party_event_rules_callbacks.py b/synapse/module_api/callbacks/third_party_event_rules_callbacks.py index 7a3255aee..9f7a04372 100644 --- a/synapse/module_api/callbacks/third_party_event_rules_callbacks.py +++ b/synapse/module_api/callbacks/third_party_event_rules_callbacks.py @@ -366,7 +366,7 @@ class ThirdPartyEventRulesModuleApiCallbacks: if len(self._check_threepid_can_be_invited_callbacks) == 0: return True - state_events = await self._get_state_map_for_room(room_id) + state_events = await self._storage_controllers.state.get_current_state(room_id) for callback in self._check_threepid_can_be_invited_callbacks: try: @@ -399,7 +399,7 @@ class ThirdPartyEventRulesModuleApiCallbacks: if len(self._check_visibility_can_be_modified_callbacks) == 0: return True - state_events = await self._get_state_map_for_room(room_id) + state_events = await self._storage_controllers.state.get_current_state(room_id) for callback in self._check_visibility_can_be_modified_callbacks: try: @@ -427,7 +427,13 @@ class ThirdPartyEventRulesModuleApiCallbacks: return event = await self.store.get_event(event_id) - state_events = await self._get_state_map_for_room(event.room_id) + + # We *don't* want to wait for the full state here, because waiting for full + # state will persist event, which in turn will call this method. + # This would end up in a deadlock. + state_events = await self._storage_controllers.state.get_current_state( + event.room_id, await_full_state=False + ) for callback in self._on_new_event_callbacks: try: @@ -490,17 +496,6 @@ class ThirdPartyEventRulesModuleApiCallbacks: ) return True - async def _get_state_map_for_room(self, room_id: str) -> StateMap[EventBase]: - """Given a room ID, return the state events of that room. - - Args: - room_id: The ID of the room. - - Returns: - A dict mapping (event type, state key) to state event. - """ - return await self._storage_controllers.state.get_current_state(room_id) - async def on_profile_update( self, user_id: str, new_profile: ProfileInfo, by_admin: bool, deactivation: bool ) -> None: diff --git a/synapse/storage/controllers/state.py b/synapse/storage/controllers/state.py index 99666d79a..22d93a561 100644 --- a/synapse/storage/controllers/state.py +++ b/synapse/storage/controllers/state.py @@ -562,10 +562,15 @@ class StateStorageController: @trace @tag_args async def get_current_state( - self, room_id: str, state_filter: Optional[StateFilter] = None + self, + room_id: str, + state_filter: Optional[StateFilter] = None, + await_full_state: bool = True, ) -> StateMap[EventBase]: """Same as `get_current_state_ids` but also fetches the events""" - state_map_ids = await self.get_current_state_ids(room_id, state_filter) + state_map_ids = await self.get_current_state_ids( + room_id, state_filter, await_full_state + ) event_map = await self.stores.main.get_events(list(state_map_ids.values())) From 696cc9e802f63ba8657856d85f6982f49de14f27 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 8 Mar 2024 04:33:46 -0500 Subject: [PATCH 13/17] Stabilize support for Retry-After header (MSC4014) (#16947) --- changelog.d/16947.feature | 1 + synapse/api/errors.py | 5 ++--- synapse/config/experimental.py | 9 --------- tests/api/test_errors.py | 8 ++------ tests/rest/client/test_login.py | 3 --- 5 files changed, 5 insertions(+), 21 deletions(-) create mode 100644 changelog.d/16947.feature diff --git a/changelog.d/16947.feature b/changelog.d/16947.feature new file mode 100644 index 000000000..efd0dbddb --- /dev/null +++ b/changelog.d/16947.feature @@ -0,0 +1 @@ +Include `Retry-After` header by default per [MSC4041](https://github.com/matrix-org/matrix-spec-proposals/pull/4041). Contributed by @clokep. diff --git a/synapse/api/errors.py b/synapse/api/errors.py index b44088f9b..dd4a1ae70 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -517,8 +517,6 @@ class InvalidCaptchaError(SynapseError): class LimitExceededError(SynapseError): """A client has sent too many requests and is being throttled.""" - include_retry_after_header = False - def __init__( self, limiter_name: str, @@ -526,9 +524,10 @@ class LimitExceededError(SynapseError): retry_after_ms: Optional[int] = None, errcode: str = Codes.LIMIT_EXCEEDED, ): + # Use HTTP header Retry-After to enable library-assisted retry handling. headers = ( {"Retry-After": str(math.ceil(retry_after_ms / 1000))} - if self.include_retry_after_header and retry_after_ms is not None + if retry_after_ms is not None else None ) super().__init__(code, "Too Many Requests", errcode, headers=headers) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index d43d9da95..0bd3befdc 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -25,7 +25,6 @@ from typing import TYPE_CHECKING, Any, Optional import attr import attr.validators -from synapse.api.errors import LimitExceededError from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersions from synapse.config import ConfigError from synapse.config._base import Config, RootConfig @@ -415,14 +414,6 @@ class ExperimentalConfig(Config): "msc4010_push_rules_account_data", False ) - # MSC4041: Use HTTP header Retry-After to enable library-assisted retry handling - # - # This is a bit hacky, but the most reasonable way to *alway* include the - # headers. - LimitExceededError.include_retry_after_header = experimental.get( - "msc4041_enabled", False - ) - self.msc4028_push_encrypted_events = experimental.get( "msc4028_push_encrypted_events", False ) diff --git a/tests/api/test_errors.py b/tests/api/test_errors.py index 25fa93b9d..efa3addf0 100644 --- a/tests/api/test_errors.py +++ b/tests/api/test_errors.py @@ -33,18 +33,14 @@ class LimitExceededErrorTestCase(unittest.TestCase): self.assertIn("needle", err.debug_context) self.assertNotIn("needle", serialised) - # Create a sub-class to avoid mutating the class-level property. - class LimitExceededErrorHeaders(LimitExceededError): - include_retry_after_header = True - def test_limit_exceeded_header(self) -> None: - err = self.LimitExceededErrorHeaders(limiter_name="test", retry_after_ms=100) + err = LimitExceededError(limiter_name="test", retry_after_ms=100) self.assertEqual(err.error_dict(None).get("retry_after_ms"), 100) assert err.headers is not None self.assertEqual(err.headers.get("Retry-After"), "1") def test_limit_exceeded_rounding(self) -> None: - err = self.LimitExceededErrorHeaders(limiter_name="test", retry_after_ms=3001) + err = LimitExceededError(limiter_name="test", retry_after_ms=3001) self.assertEqual(err.error_dict(None).get("retry_after_ms"), 3001) assert err.headers is not None self.assertEqual(err.headers.get("Retry-After"), "4") diff --git a/tests/rest/client/test_login.py b/tests/rest/client/test_login.py index 3d3a7b0aa..3a1f15008 100644 --- a/tests/rest/client/test_login.py +++ b/tests/rest/client/test_login.py @@ -177,7 +177,6 @@ class LoginRestServletTestCase(unittest.HomeserverTestCase): # rc_login dict here, we need to set this manually as well "account": {"per_second": 10000, "burst_count": 10000}, }, - "experimental_features": {"msc4041_enabled": True}, } ) def test_POST_ratelimiting_per_address(self) -> None: @@ -229,7 +228,6 @@ class LoginRestServletTestCase(unittest.HomeserverTestCase): # rc_login dict here, we need to set this manually as well "address": {"per_second": 10000, "burst_count": 10000}, }, - "experimental_features": {"msc4041_enabled": True}, } ) def test_POST_ratelimiting_per_account(self) -> None: @@ -278,7 +276,6 @@ class LoginRestServletTestCase(unittest.HomeserverTestCase): "address": {"per_second": 10000, "burst_count": 10000}, "failed_attempts": {"per_second": 0.17, "burst_count": 5}, }, - "experimental_features": {"msc4041_enabled": True}, } ) def test_POST_ratelimiting_per_account_failed_attempts(self) -> None: From 48f59d3806477d575350fa4dc093e02cf0eca901 Mon Sep 17 00:00:00 2001 From: Alexander Fechler <141915399+afechler@users.noreply.github.com> Date: Mon, 11 Mar 2024 17:08:04 +0100 Subject: [PATCH 14/17] deactivated flag refactored to filter deactivated users. (#16874) Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- changelog.d/16874.feature | 1 + docs/admin_api/user_admin_api.md | 14 ++++++ synapse/rest/admin/__init__.py | 2 + synapse/rest/admin/users.py | 21 +++++++- synapse/storage/databases/main/__init__.py | 9 ++-- tests/rest/admin/test_user.py | 56 ++++++++++++++++++++-- 6 files changed, 95 insertions(+), 8 deletions(-) create mode 100644 changelog.d/16874.feature diff --git a/changelog.d/16874.feature b/changelog.d/16874.feature new file mode 100644 index 000000000..6e5afdde3 --- /dev/null +++ b/changelog.d/16874.feature @@ -0,0 +1 @@ +Add a new [List Accounts v3](https://element-hq.github.io/synapse/v1.103/admin_api/user_admin_api.html#list-accounts-v3) Admin API with improved deactivated user filtering capabilities. \ No newline at end of file diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index 9dc600b87..9736fe302 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -164,6 +164,7 @@ Body parameters: Other allowed options are: `bot` and `support`. ## List Accounts +### List Accounts (V2) This API returns all local user accounts. By default, the response is ordered by ascending user ID. @@ -287,6 +288,19 @@ The following fields are returned in the JSON response body: *Added in Synapse 1.93:* the `locked` query parameter and response field. +### List Accounts (V3) + +This API returns all local user accounts (see v2). In contrast to v2, the query parameter `deactivated` is handled differently. + +``` +GET /_synapse/admin/v3/users +``` + +**Parameters** +- `deactivated` - Optional flag to filter deactivated users. If `true`, only deactivated users are returned. + If `false`, deactivated users are excluded from the query. When the flag is absent (the default), + users are not filtered by deactivation status. + ## Query current sessions for a user This API returns information about the active sessions for a specific user. diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 0e413f02a..07e0fb71f 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -109,6 +109,7 @@ from synapse.rest.admin.users import ( UserReplaceMasterCrossSigningKeyRestServlet, UserRestServletV2, UsersRestServletV2, + UsersRestServletV3, UserTokenRestServlet, WhoisRestServlet, ) @@ -289,6 +290,7 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: UserTokenRestServlet(hs).register(http_server) UserRestServletV2(hs).register(http_server) UsersRestServletV2(hs).register(http_server) + UsersRestServletV3(hs).register(http_server) UserMediaStatisticsRestServlet(hs).register(http_server) LargestRoomsStatistics(hs).register(http_server) EventReportDetailRestServlet(hs).register(http_server) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 0229e87f4..a9645e4af 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -23,7 +23,7 @@ import hmac import logging import secrets from http import HTTPStatus -from typing import TYPE_CHECKING, Dict, List, Optional, Tuple +from typing import TYPE_CHECKING, Dict, List, Optional, Tuple, Union import attr @@ -118,7 +118,8 @@ class UsersRestServletV2(RestServlet): errcode=Codes.INVALID_PARAM, ) - deactivated = parse_boolean(request, "deactivated", default=False) + deactivated = self._parse_parameter_deactivated(request) + locked = parse_boolean(request, "locked", default=False) admins = parse_boolean(request, "admins") @@ -182,6 +183,22 @@ class UsersRestServletV2(RestServlet): return HTTPStatus.OK, ret + def _parse_parameter_deactivated(self, request: SynapseRequest) -> Optional[bool]: + """ + Return None (no filtering) if `deactivated` is `true`, otherwise return `False` + (exclude deactivated users from the results). + """ + return None if parse_boolean(request, "deactivated") else False + + +class UsersRestServletV3(UsersRestServletV2): + PATTERNS = admin_patterns("/users$", "v3") + + def _parse_parameter_deactivated( + self, request: SynapseRequest + ) -> Union[bool, None]: + return parse_boolean(request, "deactivated") + class UserRestServletV2(RestServlet): PATTERNS = admin_patterns("/users/(?P[^/]*)$", "v2") diff --git a/synapse/storage/databases/main/__init__.py b/synapse/storage/databases/main/__init__.py index 394985d87..bf779587d 100644 --- a/synapse/storage/databases/main/__init__.py +++ b/synapse/storage/databases/main/__init__.py @@ -176,7 +176,7 @@ class DataStore( user_id: Optional[str] = None, name: Optional[str] = None, guests: bool = True, - deactivated: bool = False, + deactivated: Optional[bool] = None, admins: Optional[bool] = None, order_by: str = UserSortOrder.NAME.value, direction: Direction = Direction.FORWARDS, @@ -232,8 +232,11 @@ class DataStore( if not guests: filters.append("is_guest = 0") - if not deactivated: - filters.append("deactivated = 0") + if deactivated is not None: + if deactivated: + filters.append("deactivated = 1") + else: + filters.append("deactivated = 0") if not locked: filters.append("locked IS FALSE") diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 926585422..c5da1e968 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -503,7 +503,7 @@ class UsersListTestCase(unittest.HomeserverTestCase): channel = self.make_request( "GET", - self.url + "?deactivated=true", + f"{self.url}?deactivated=true", {}, access_token=self.admin_user_tok, ) @@ -982,6 +982,56 @@ class UsersListTestCase(unittest.HomeserverTestCase): self.assertEqual(1, channel.json_body["total"]) self.assertFalse(channel.json_body["users"][0]["admin"]) + def test_filter_deactivated_users(self) -> None: + """ + Tests whether the various values of the query parameter `deactivated` lead to the + expected result set. + """ + users_url_v3 = self.url.replace("v2", "v3") + + # Register an additional non admin user + user_id = self.register_user("user", "pass", admin=False) + + # Deactivate that user, requesting erasure. + deactivate_account_handler = self.hs.get_deactivate_account_handler() + self.get_success( + deactivate_account_handler.deactivate_account( + user_id, erase_data=True, requester=create_requester(user_id) + ) + ) + + # Query all users + channel = self.make_request( + "GET", + users_url_v3, + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, channel.code, channel.result) + self.assertEqual(2, channel.json_body["total"]) + + # Query deactivated users + channel = self.make_request( + "GET", + f"{users_url_v3}?deactivated=true", + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, channel.code, channel.result) + self.assertEqual(1, channel.json_body["total"]) + self.assertEqual("@user:test", channel.json_body["users"][0]["name"]) + + # Query non-deactivated users + channel = self.make_request( + "GET", + f"{users_url_v3}?deactivated=false", + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, channel.code, channel.result) + self.assertEqual(1, channel.json_body["total"]) + self.assertEqual("@admin:test", channel.json_body["users"][0]["name"]) + @override_config( { "experimental_features": { @@ -1130,7 +1180,7 @@ class UsersListTestCase(unittest.HomeserverTestCase): # They should appear in the list users API, marked as not erased. channel = self.make_request( "GET", - self.url + "?deactivated=true", + f"{self.url}?deactivated=true", access_token=self.admin_user_tok, ) users = {user["name"]: user for user in channel.json_body["users"]} @@ -1194,7 +1244,7 @@ class UsersListTestCase(unittest.HomeserverTestCase): dir: The direction of ordering to give the server """ - url = self.url + "?deactivated=true&" + url = f"{self.url}?deactivated=true&" if order_by is not None: url += "order_by=%s&" % (order_by,) if dir is not None and dir in ("b", "f"): From 9d7880c0c617a1a042f0430de3753af8467e98a5 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 12 Mar 2024 15:03:45 +0000 Subject: [PATCH 15/17] 1.103.0rc1 --- CHANGES.md | 28 ++++++++++++++++++++++++++++ changelog.d/16768.doc | 1 - changelog.d/16874.feature | 1 - changelog.d/16946.doc | 1 - changelog.d/16947.feature | 1 - changelog.d/16951.doc | 1 - changelog.d/16973.bugfix | 1 - debian/changelog | 6 ++++++ pyproject.toml | 2 +- 9 files changed, 35 insertions(+), 7 deletions(-) delete mode 100644 changelog.d/16768.doc delete mode 100644 changelog.d/16874.feature delete mode 100644 changelog.d/16946.doc delete mode 100644 changelog.d/16947.feature delete mode 100644 changelog.d/16951.doc delete mode 100644 changelog.d/16973.bugfix diff --git a/CHANGES.md b/CHANGES.md index 2e7e4be5a..3d027ae25 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,31 @@ +# Synapse 1.103.0rc1 (2024-03-12) + +### Features + +- Add a new [List Accounts v3](https://element-hq.github.io/synapse/v1.103/admin_api/user_admin_api.html#list-accounts-v3) Admin API with improved deactivated user filtering capabilities. ([\#16874](https://github.com/element-hq/synapse/issues/16874)) +- Include `Retry-After` header by default per [MSC4041](https://github.com/matrix-org/matrix-spec-proposals/pull/4041). Contributed by @clokep. ([\#16947](https://github.com/element-hq/synapse/issues/16947)) + +### Bugfixes + +- Fix joining remote rooms when a module uses the `on_new_event` callback. This callback may now pass partial state events instead of the full state for remote rooms. Introduced in v1.76.0. ([\#16973](https://github.com/element-hq/synapse/issues/16973)) + +### Improved Documentation + +- Add HAProxy example for single port operation to reverse proxy documentation. Contributed by Georg Pfuetzenreuter (@tacerus). ([\#16768](https://github.com/element-hq/synapse/issues/16768)) +- Improve the documentation around running Complement tests with new configuration parameters. ([\#16946](https://github.com/element-hq/synapse/issues/16946)) +- Add docs on upgrading from a very old version. ([\#16951](https://github.com/element-hq/synapse/issues/16951)) + + +### Updates to locked dependencies + +* Bump JasonEtco/create-an-issue from 2.9.1 to 2.9.2. ([\#16934](https://github.com/element-hq/synapse/issues/16934)) +* Bump anyhow from 1.0.79 to 1.0.80. ([\#16935](https://github.com/element-hq/synapse/issues/16935)) +* Bump dawidd6/action-download-artifact from 3.0.0 to 3.1.1. ([\#16933](https://github.com/element-hq/synapse/issues/16933)) +* Bump furo from 2023.9.10 to 2024.1.29. ([\#16939](https://github.com/element-hq/synapse/issues/16939)) +* Bump pyopenssl from 23.3.0 to 24.0.0. ([\#16937](https://github.com/element-hq/synapse/issues/16937)) +* Bump types-netaddr from 0.10.0.20240106 to 1.2.0.20240219. ([\#16938](https://github.com/element-hq/synapse/issues/16938)) + + # Synapse 1.102.0 (2024-03-05) ### Bugfixes diff --git a/changelog.d/16768.doc b/changelog.d/16768.doc deleted file mode 100644 index 4f574c2ac..000000000 --- a/changelog.d/16768.doc +++ /dev/null @@ -1 +0,0 @@ -Add HAProxy example for single port operation to reverse proxy documentation. Contributed by Georg Pfuetzenreuter (@tacerus). diff --git a/changelog.d/16874.feature b/changelog.d/16874.feature deleted file mode 100644 index 6e5afdde3..000000000 --- a/changelog.d/16874.feature +++ /dev/null @@ -1 +0,0 @@ -Add a new [List Accounts v3](https://element-hq.github.io/synapse/v1.103/admin_api/user_admin_api.html#list-accounts-v3) Admin API with improved deactivated user filtering capabilities. \ No newline at end of file diff --git a/changelog.d/16946.doc b/changelog.d/16946.doc deleted file mode 100644 index 8037a6545..000000000 --- a/changelog.d/16946.doc +++ /dev/null @@ -1 +0,0 @@ -Improve the documentation around running Complement tests with new configuration parameters. \ No newline at end of file diff --git a/changelog.d/16947.feature b/changelog.d/16947.feature deleted file mode 100644 index efd0dbddb..000000000 --- a/changelog.d/16947.feature +++ /dev/null @@ -1 +0,0 @@ -Include `Retry-After` header by default per [MSC4041](https://github.com/matrix-org/matrix-spec-proposals/pull/4041). Contributed by @clokep. diff --git a/changelog.d/16951.doc b/changelog.d/16951.doc deleted file mode 100644 index a7fb33979..000000000 --- a/changelog.d/16951.doc +++ /dev/null @@ -1 +0,0 @@ -Add docs on upgrading from a very old version. \ No newline at end of file diff --git a/changelog.d/16973.bugfix b/changelog.d/16973.bugfix deleted file mode 100644 index a0e3a1230..000000000 --- a/changelog.d/16973.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix joining remote rooms when a module uses the `on_new_event` callback. This callback may now pass partial state events instead of the full state for remote rooms. diff --git a/debian/changelog b/debian/changelog index df0ab0dcd..ac128e535 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +matrix-synapse-py3 (1.103.0~rc1) stable; urgency=medium + + * New Synapse release 1.103.0rc1. + + -- Synapse Packaging team Tue, 12 Mar 2024 15:02:56 +0000 + matrix-synapse-py3 (1.102.0) stable; urgency=medium * New Synapse release 1.102.0. diff --git a/pyproject.toml b/pyproject.toml index c360d60ec..2bda9596f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -96,7 +96,7 @@ module-name = "synapse.synapse_rust" [tool.poetry] name = "matrix-synapse" -version = "1.102.0" +version = "1.103.0rc1" description = "Homeserver for the Matrix decentralised comms protocol" authors = ["Matrix.org Team and Contributors "] license = "AGPL-3.0-or-later" From 1f887907649c068993d63d32cde5e18b391dc15e Mon Sep 17 00:00:00 2001 From: Gerrit Gogel Date: Tue, 12 Mar 2024 16:07:36 +0100 Subject: [PATCH 16/17] Prevent locking up while processing batched_auth_events (#16968) This PR aims to fix #16895, caused by a regression in #7 and not fixed by #16903. The PR #16903 only fixes a starvation issue, where the CPU isn't released. There is a second issue, where the execution is blocked. This theory is supported by the flame graphs provided in #16895 and the fact that I see the CPU usage reducing and far below the limit. Since the changes in #7, the method `check_state_independent_auth_rules` is called with the additional parameter `batched_auth_events`: https://github.com/element-hq/synapse/blob/6fa13b4f927c10b5f4e9495be746ec28849f5cb6/synapse/handlers/federation_event.py#L1741-L1743 It makes the execution enter this if clause, introduced with #15195 https://github.com/element-hq/synapse/blob/6fa13b4f927c10b5f4e9495be746ec28849f5cb6/synapse/event_auth.py#L178-L189 There are two issues in the above code snippet. First, there is the blocking issue. I'm not entirely sure if this is a deadlock, starvation, or something different. In the beginning, I thought the copy operation was responsible. It wasn't. Then I investigated the nested `store.get_events` inside the function `update`. This was also not causing the blocking issue. Only when I replaced the set difference operation (`-` ) with a list comprehension, the blocking was resolved. Creating and comparing sets with a very large amount of events seems to be problematic. This is how the flamegraph looks now while persisting outliers. As you can see, the execution no longer locks up in the above function. ![output_2024-02-28_13-59-40](https://github.com/element-hq/synapse/assets/13143850/6db9c9ac-484f-47d0-bdde-70abfbd773ec) Second, the copying here doesn't serve any purpose, because only a shallow copy is created. This means the same objects from the original dict are referenced. This fails the intention of protecting these objects from mutation. The review of the original PR https://github.com/matrix-org/synapse/pull/15195 had an extensive discussion about this matter. Various approaches to copying the auth_events were attempted: 1) Implementing a deepcopy caused issues due to builtins.EventInternalMetadata not being pickleable. 2) Creating a dict with new objects akin to a deepcopy. 3) Creating a dict with new objects containing only necessary attributes. Concluding, there is no easy way to create an actual copy of the objects. Opting for a deepcopy can significantly strain memory and CPU resources, making it an inefficient choice. I don't see why the copy is necessary in the first place. Therefore I'm proposing to remove it altogether. After these changes, I was able to successfully join these rooms, without the main worker locking up: - #synapse:matrix.org - #element-android:matrix.org - #element-web:matrix.org - #ecips:matrix.org - #ipfs-chatter:ipfs.io - #python:matrix.org - #matrix:matrix.org --- changelog.d/16968.bugfix | 1 + synapse/event_auth.py | 43 +++++++++++++++++++++++++++++++--------- 2 files changed, 35 insertions(+), 9 deletions(-) create mode 100644 changelog.d/16968.bugfix diff --git a/changelog.d/16968.bugfix b/changelog.d/16968.bugfix new file mode 100644 index 000000000..57ed85117 --- /dev/null +++ b/changelog.d/16968.bugfix @@ -0,0 +1 @@ +Prevent locking up when checking auth rules that are independent of room state for batched auth events. Contributed by @ggogel. \ No newline at end of file diff --git a/synapse/event_auth.py b/synapse/event_auth.py index d922c8dc3..c8b06f760 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -23,7 +23,20 @@ import collections.abc import logging import typing -from typing import Any, Dict, Iterable, List, Mapping, Optional, Set, Tuple, Union +from typing import ( + Any, + ChainMap, + Dict, + Iterable, + List, + Mapping, + MutableMapping, + Optional, + Set, + Tuple, + Union, + cast, +) from canonicaljson import encode_canonical_json from signedjson.key import decode_verify_key_bytes @@ -175,12 +188,22 @@ async def check_state_independent_auth_rules( return # 2. Reject if event has auth_events that: ... + auth_events: ChainMap[str, EventBase] = ChainMap() if batched_auth_events: - # Copy the batched auth events to avoid mutating them. - auth_events = dict(batched_auth_events) - needed_auth_event_ids = set(event.auth_event_ids()) - batched_auth_events.keys() + # batched_auth_events can become very large. To avoid repeatedly copying it, which + # would significantly impact performance, we use a ChainMap. + # batched_auth_events must be cast to MutableMapping because .new_child() requires + # this type. This casting is safe as the mapping is never mutated. + auth_events = auth_events.new_child( + cast(MutableMapping[str, "EventBase"], batched_auth_events) + ) + needed_auth_event_ids = [ + event_id + for event_id in event.auth_event_ids() + if event_id not in batched_auth_events + ] if needed_auth_event_ids: - auth_events.update( + auth_events = auth_events.new_child( await store.get_events( needed_auth_event_ids, redact_behaviour=EventRedactBehaviour.as_is, @@ -188,10 +211,12 @@ async def check_state_independent_auth_rules( ) ) else: - auth_events = await store.get_events( - event.auth_event_ids(), - redact_behaviour=EventRedactBehaviour.as_is, - allow_rejected=True, + auth_events = auth_events.new_child( + await store.get_events( + event.auth_event_ids(), + redact_behaviour=EventRedactBehaviour.as_is, + allow_rejected=True, + ) ) room_id = event.room_id From 5c0b87ff959dfa424f36f67df12a9e9e2c2e9763 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 12 Mar 2024 15:12:19 +0000 Subject: [PATCH 17/17] Update changelog --- CHANGES.md | 1 + changelog.d/16968.bugfix | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 changelog.d/16968.bugfix diff --git a/CHANGES.md b/CHANGES.md index 3d027ae25..f0c23e693 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,6 +8,7 @@ ### Bugfixes - Fix joining remote rooms when a module uses the `on_new_event` callback. This callback may now pass partial state events instead of the full state for remote rooms. Introduced in v1.76.0. ([\#16973](https://github.com/element-hq/synapse/issues/16973)) +- Fix performance issue when joining very large rooms that can cause the server to lock up. Introduced in v1.100.0. Contributed by @ggogel. ([\#16968](https://github.com/element-hq/synapse/issues/16968)) ### Improved Documentation diff --git a/changelog.d/16968.bugfix b/changelog.d/16968.bugfix deleted file mode 100644 index 57ed85117..000000000 --- a/changelog.d/16968.bugfix +++ /dev/null @@ -1 +0,0 @@ -Prevent locking up when checking auth rules that are independent of room state for batched auth events. Contributed by @ggogel. \ No newline at end of file