Backgrounds membership updates when changing the avatar or the display name by MatMaul · Pull Request #19311 · element-hq/synapse

MatMaul · 2025-12-16T16:51:20Z

Membership updates can be quite long since it can trigger state resolution.

In the meantime the user will get a timeout error, and may try again, putting even more load on the server.

Pull Request Checklist

Pull request is based on the develop branch
Pull request includes a changelog file.
Code style is correct (run the linters)

…y name

anoadragon453

Thanks for doing this! Have you tried it out and seen what the user experience is like?

A few notes below, but nothing major.

anoadragon453 · 2026-02-25T16:46:06Z

synapse/handlers/profile.py

+        if last_room_id:
+            unhandled_room_ids = []
+            for room_id in room_ids:
+                if room_id > last_room_id:
+                    unhandled_room_ids.append(room_id)
+            room_ids = unhandled_room_ids


Since room_ids is already sorted, we can break after we find a room_id is greater than last_room_id.

I don't think so, the unhandled rooms being at the end of the list and not the beginning.

I can however reversed the list and break in an else, on it.

Ah! Or alternatively, take the remainder of the list once you reach the first unhandled room.

But both work.

Looking at this again, it's starting to be pretty difficult to read. We could simplify this greatly using the built-in bisect module's bisect_right method to find the index of where to insert an item in a sorted list:

from bisect import bisect_right # ... if last_room_id: room_ids = room_ids[bisect_right(room_ids, last_room_id):]

this is dangerous I think : we may have left the room in the meantime :)

I am going to try your alternate proposal and see if it looks clearer.

Well copilot is right so let's go for the alternate.

synapse/handlers/profile.py

Co-authored-by: Andrew Morgan <[email protected]>

…-task

synapse/handlers/profile.py

MatMaul · 2026-03-03T11:20:49Z

Thanks for doing this! Have you tried it out and seen what the user experience is like?

I tested it, it is instant as expected.

Copilot

Pull request overview

This PR aims to prevent long-running profile-driven membership updates (display name / avatar changes) from causing client timeouts by moving the work into a resumable background task.

Changes:

Add profile-handler logic to schedule/cancel a per-user background task to update join state events after profile changes.
Extend task scheduler execution wrapper to handle task cancellation explicitly.
Add tests covering background membership update behavior and restart resumption, plus a changelog entry.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
`synapse/handlers/profile.py`	Schedules a resumable `update_join_states` task instead of performing membership updates inline.
`synapse/util/task_scheduler.py`	Adjusts scheduled task wrapper behavior around cancellation/error handling.
`tests/handlers/test_profile.py`	Adds tests for membership propagation, background behavior, and “resume after restart”.
`changelog.d/19311.misc`	Documents the behavior change.

Comments suppressed due to low confidence (1)

synapse/util/task_scheduler.py:497

Catching defer.CancelledError here and then unconditionally calling _store.update_scheduled_task(... result=None ...) will overwrite any previously persisted task.result (breaking the existing cancellation semantics/tests that expect progress to be preserved). It can also raise KeyError at self._running_tasks.pop(task.id) because on_cancel_task already removes the entry. Consider either letting CancelledError propagate (relying on on_cancel_task to update status), or explicitly preserve the stored result on cancellation and make the _running_tasks removal idempotent.

                result = None
                error = None
                try:
                    (status, result, error) = await function(task)
                except defer.CancelledError:
                    status = TaskStatus.CANCELLED
                except Exception:
                    f = Failure()
                    logger.error(
                        "scheduled task %s failed",
                        task.id,
                        exc_info=(f.type, f.value, f.getTracebackObject()),
                    )
                    status = TaskStatus.FAILED
                    error = f.getErrorMessage()

                await self._store.update_scheduled_task(
                    task.id,
                    self._clock.time_msec(),
                    status=status,
                    result=result,
                    error=error,
                )
                self._running_tasks.pop(task.id)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

synapse/handlers/profile.py

tests/handlers/test_profile.py

anoadragon453

A few more things, but we're getting there!

anoadragon453 · 2026-03-03T13:03:43Z

synapse/handlers/profile.py

+        if last_room_id:
+            unhandled_room_ids = []
+            for room_id in room_ids:
+                if room_id > last_room_id:
+                    unhandled_room_ids.append(room_id)
+            room_ids = unhandled_room_ids


Looking at this again, it's starting to be pretty difficult to read. We could simplify this greatly using the built-in bisect module's bisect_right method to find the index of where to insert an item in a sorted list:

from bisect import bisect_right # ... if last_room_id: room_ids = room_ids[bisect_right(room_ids, last_room_id):]

synapse/handlers/profile.py

tests/handlers/test_profile.py

synapse/handlers/profile.py

anoadragon453

I think this is pretty much ready to go, thanks for your patience @MatMaul!

Just a couple small things before merging.

tests/handlers/test_profile.py

anoadragon453 · 2026-03-04T17:55:39Z

synapse/handlers/profile.py

+            # Filter out room IDs that have already been handled
+            # by finding the first room ID greater than the last handled room ID
+            # and slicing the list from that point onwards.
+            first_unhandled_room_idx = 0
+            for idx, room_id in enumerate(room_ids):
+                if room_id > last_room_id:
+                    first_unhandled_room_idx = idx
+                    break
+            room_ids = room_ids[first_unhandled_room_idx:]


@MatMaul What did you think about this (potentially even simpler) implementation? #19311 (comment)

I think you missed my comment here 🙂 #19311 (comment)

Actually it seems like bisect doc is saying that it is using comparison and not equal so we should be fine.

Can you double check what I interpreted from the doc ?

Thanks !

I've pushed an update since pretty sure of it from the doc, thanks for the trick!

Oops, yes I did miss your comment! Apologies.

But yes, even if we leave a room, we're computing a new index based on the room ID, which disappearing values shouldn't impact.

Co-authored-by: Andrew Morgan <[email protected]>

…-task

anoadragon453

This now LGTM. Exciting quality-of-life feature, thanks @MatMaul!

This PR contains the following updates: | Package | Update | Change | |---|---|---| | [element-hq/synapse](https://github.com/element-hq/synapse) | minor | `v1.149.1` → `v1.150.0` | --- ### Release Notes <details> <summary>element-hq/synapse (element-hq/synapse)</summary> ### [`v1.150.0`](https://github.com/element-hq/synapse/releases/tag/v1.150.0) [Compare Source](element-hq/synapse@v1.149.1...v1.150.0) ### Synapse 1.150.0 (2026-03-24) No significant changes since 1.150.0rc1. ### Synapse 1.150.0rc1 (2026-03-17) #### Features - Add experimental support for the [MSC4370](matrix-org/matrix-spec-proposals#4370) Federation API `GET /extremities` endpoint. ([#19314](element-hq/synapse#19314)) - [MSC4140: Cancellable delayed events](matrix-org/matrix-spec-proposals#4140): When persisting a delayed event to the timeline, include its `delay_id` in the event's `unsigned` section in `/sync` responses to the event sender. ([#19479](element-hq/synapse#19479)) - Expose [MSC4354 Sticky Events](matrix-org/matrix-spec-proposals#4354) over the legacy (v3) /sync API. ([#19487](element-hq/synapse#19487)) - When Matrix Authentication Service (MAS) integration is enabled, allow MAS to set the user locked status in Synapse. ([#19554](element-hq/synapse#19554)) #### Bugfixes - Fix `Build and push complement image` CI job pointing to non-existent image. ([#19523](element-hq/synapse#19523)) - Fix a bug introduced in v1.26.0 that caused deactivated, erased users to not be removed from the user directory. ([#19542](element-hq/synapse#19542)) #### Improved Documentation - In the Admin API documentation, always express path parameters as `/<param>` instead of as `/$param`. ([#19307](element-hq/synapse#19307)) - Update docs to clarify `outbound_federation_restricted_to` can also be used with the [Secure Border Gateway (SBG)](https://element.io/en/server-suite/secure-border-gateways). ([#19517](element-hq/synapse#19517)) - Unify Complement developer docs. ([#19518](element-hq/synapse#19518)) #### Internal Changes - Put membership updates in a background resumable task when changing the avatar or the display name. ([#19311](element-hq/synapse#19311)) - Add in-repo Complement test to sanity check Synapse version matches git checkout (testing what we think we are). ([#19476](element-hq/synapse#19476)) - Migrate `dev` dependencies to [PEP 735](https://peps.python.org/pep-0735/) dependency groups. ([#19490](element-hq/synapse#19490)) - Remove the optional `systemd-python` dependency and the `systemd` extra on the `synapse` package. ([#19491](element-hq/synapse#19491)) - Avoid re-computing the event ID when cloning events. ([#19527](element-hq/synapse#19527)) - Allow caching of the `/versions` and `/auth_metadata` public endpoints. ([#19530](element-hq/synapse#19530)) - Add a few labels to the number groupings in the `Processed request` logs. ([#19548](element-hq/synapse#19548)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://gitea.alexlebens.dev/alexlebens/infrastructure/pulls/5040 Co-authored-by: Renovate Bot <[email protected]> Co-committed-by: Renovate Bot <[email protected]>

MatMaul force-pushed the update-join-states-task branch 3 times, most recently from d475f86 to 29032a7 Compare February 6, 2026 16:22

MatMaul marked this pull request as ready for review February 6, 2026 16:25

MatMaul requested a review from a team as a code owner February 6, 2026 16:25

MatMaul force-pushed the update-join-states-task branch 2 times, most recently from 06a82f8 to 580db36 Compare February 17, 2026 19:52

Backgrounds membership updates when changing the avatar or the displa…

368cb19

…y name

MatMaul force-pushed the update-join-states-task branch from 580db36 to 368cb19 Compare February 17, 2026 21:26

anoadragon453 requested changes Feb 25, 2026

View reviewed changes

MatMaul and others added 3 commits March 3, 2026 11:42

Update synapse/handlers/profile.py

ec632bb

Co-authored-by: Andrew Morgan <[email protected]>

Merge remote-tracking branch 'origin/develop' into update-join-states…

aa048be

…-task

Iterate and break on reverse list of rooms

21a715a

MatMaul commented Mar 3, 2026

View reviewed changes

synapse/handlers/profile.py Outdated Show resolved Hide resolved

MatMaul added 2 commits March 3, 2026 12:10

Use standard lock instead of RW one

be794b2

Assert tasks_to_cancel cardinality

e792f9a

MatMaul requested a review from anoadragon453 March 3, 2026 11:21

anoadragon453 requested a review from Copilot March 3, 2026 12:56

Copilot started reviewing on behalf of anoadragon453 March 3, 2026 12:57 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

anoadragon453 requested changes Mar 3, 2026

View reviewed changes

MatMaul added 3 commits March 3, 2026 20:29

typo

f94e546

Slice to the end instead of reversed

b8dc114

Fix task cursor not updated properly and test for it

59a02cf

MatMaul requested a review from anoadragon453 March 4, 2026 08:35

anoadragon453 reviewed Mar 4, 2026

View reviewed changes

Update tests/handlers/test_profile.py

fe141d0

Co-authored-by: Andrew Morgan <[email protected]>

MatMaul requested a review from anoadragon453 March 5, 2026 09:58

lint

630c305

MatMaul added 3 commits March 5, 2026 11:11

Merge remote-tracking branch 'origin/develop' into update-join-states…

542c159

…-task

use bisect

c4d3e4c

format

f6bd5d4

anoadragon453 approved these changes Mar 5, 2026

View reviewed changes

anoadragon453 merged commit 699a898 into element-hq:develop Mar 5, 2026
41 checks passed

anoadragon453 mentioned this pull request Mar 5, 2026

Prevent updating display name causing time outs on large accounts #17074

Closed

3 tasks

MadLittleMods added A-Membership A-Profiles labels Mar 5, 2026

neilisfragile mentioned this pull request Mar 30, 2026

Reorder rooms by read receipts to update most important rooms first #19613

Open

3 tasks

Conversation

MatMaul commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Checklist

Uh oh!

anoadragon453 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anoadragon453 Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MatMaul commented Mar 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anoadragon453 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anoadragon453 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatMaul Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anoadragon453 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MatMaul commented Dec 16, 2025 •

edited

Loading

anoadragon453 Mar 3, 2026 •

edited

Loading

anoadragon453 left a comment •

edited

Loading

MatMaul Mar 5, 2026 •

edited

Loading