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

Faster joins: Handle a second join while syncing state for the first one #12801

Closed
Tracked by #14030
squahtx opened this issue May 19, 2022 · 4 comments · Fixed by #14606
Closed
Tracked by #14030

Faster joins: Handle a second join while syncing state for the first one #12801

squahtx opened this issue May 19, 2022 · 4 comments · Fixed by #14606
Assignees
Labels
A-Federated-Join joins over federation generally suck T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@squahtx
Copy link
Contributor

squahtx commented May 19, 2022

We want to avoid launching a second partial state syncing job for the same room.

@squahtx squahtx self-assigned this May 19, 2022
@babolivier babolivier added the T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. label May 19, 2022
@MadLittleMods MadLittleMods added the A-Federated-Join joins over federation generally suck label Jun 3, 2022
@squahtx
Copy link
Contributor Author

squahtx commented Jun 6, 2022

Currently, the second join will block until Synapse finishes syncing state. While not incorrect, it's not a great experience and we should be able to allow some membership changes before we've finished syncing state.

I've left a WIP complement test at https://github.com/matrix-org/complement/tree/squah/faster_room_joins_handle_second_join_while_resyncing


Synapse blocks in update_membership_locked:

  File "/usr/local/lib/python3.9/site-packages/synapse/http/server.py", line 366, in _async_render_wrapper
    callback_return = await self._async_render(request)
  File "/usr/local/lib/python3.9/site-packages/synapse/http/server.py", line 572, in _async_render                                                                                                                                               callback_return = await raw_callback_return
  File "/usr/local/lib/python3.9/site-packages/synapse/rest/client/room.py", line 335, in on_POST
    await self.room_member_handler.update_membership(                                                                                                                                                                                          File "/usr/local/lib/python3.9/site-packages/synapse/handlers/room_member.py", line 524, in update_membership
    result = await self.update_membership_locked(
  File "/usr/local/lib/python3.9/site-packages/synapse/handlers/room_member.py", line 735, in update_membership_locked
    current_state_ids = await self.state_handler.get_current_state_ids(                                                                                                                                                                        File "/usr/local/lib/python3.9/site-packages/synapse/state/__init__.py", line 215, in get_current_state_ids
    ret = await self.resolve_state_groups_for_events(room_id, latest_event_ids)
  File "/usr/local/lib/python3.9/site-packages/synapse/util/metrics.py", line 113, in measured_func
    r = await func(self, *args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/synapse/state/__init__.py", line 423, in resolve_state_groups_for_events
    state_groups = await self._state_storage_controller.get_state_group_for_events(
  File "/usr/local/lib/python3.9/site-packages/synapse/storage/controllers/state.py", line 338, in get_state_group_for_events
    await self._partial_state_events_tracker.await_full_state(event_ids)
  File "/usr/local/lib/python3.9/site-packages/synapse/storage/util/partial_state_events_tracker.py", line 78, in await_full_state
    logger.info(
2022-06-06 17:14:28,273 - synapse.storage.util.partial_state_events_tracker - 78 - INFO - GET-8 - Awaiting un-partial-stating of events ['$-2iMGMrGcncRdZu8ZdY8GwzR2u0WPiFBfCwPLO7O_3Y']

update_membership_locked is a large-ish method that handles all membership changes and not just joins. So we have to consider the impact of using partial state for each of the membership transitions.

@richvdh
Copy link
Member

richvdh commented Jun 9, 2022

I think we can punt this to Q3; it's not causing breakage, rather poor UX.

@richvdh richvdh added T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. and removed T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. labels Jul 22, 2022
@richvdh
Copy link
Member

richvdh commented Oct 5, 2022

we think this will depend on #12997, so this is blocked until that is done.

@squahtx squahtx added the z-blocked (Deprecated Label) label Nov 10, 2022
@MatMaul MatMaul self-assigned this Nov 30, 2022
@squahtx squahtx removed the z-blocked (Deprecated Label) label Nov 30, 2022
@MatMaul
Copy link
Contributor

MatMaul commented Dec 2, 2022

For now I am thinking about doing another round of make_join-send_join when receiving a new client join when in partial state, cf here for the discussion.

squahtx pushed a commit that referenced this issue Feb 10, 2023
Fixes #12801.
Complement tests are at
matrix-org/complement#567.

Avoid blocking on full state when handling a subsequent join into a
partial state room.

Also always perform a remote join into partial state rooms, since we do
not know whether the joining user has been banned and want to avoid
leaking history to banned users.

Signed-off-by: Mathieu Velten <[email protected]>
Co-authored-by: Sean Quah <[email protected]>
Co-authored-by: David Robertson <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Federated-Join joins over federation generally suck T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants