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

Faster joins: don't stall when a user joins during a fast join #14606

Merged
merged 23 commits into from
Feb 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
384ccf2
Faster joins: don't stale when an user joins during a fast join
Dec 2, 2022
615d303
Merge branch 'develop' into mv/join-during-fast-join
Dec 29, 2022
7c1e8cf
Only delete fw extrems on join if room state is not partial
MatMaul Dec 29, 2022
0c3bc68
Only call _clean_room_for_join if send_join was successful
MatMaul Dec 29, 2022
4f6c342
move already_partially_joined closer to its use to avoid races
MatMaul Dec 29, 2022
6aa72e1
Fallback to local join if a room is unpartial stated during remote join
Jan 2, 2023
d98994c
Move PartialStateConflictError to api.errors because of circular dep
Jan 2, 2023
02d285e
Merge branch 'develop' into mv/join-during-fast-join
Jan 2, 2023
9919405
Merge branch 'develop' into mv/join-during-fast-join
Jan 9, 2023
a24d07a
Small mistake
Jan 9, 2023
20d5d34
Merge branch 'develop' into mv/join-during-fast-join
Jan 27, 2023
9a934c1
fix up for #14882
Jan 27, 2023
5826574
fixup imports
Jan 27, 2023
1df7060
fixup exception docstring
Jan 27, 2023
bfba7bb
fixup: remove more defunct PartialStateConflictError handling
Jan 27, 2023
0ecd9ba
Avoid clobbering existing local memberships on a second partial join
Jan 31, 2023
a23a64f
fix trial tests
Jan 31, 2023
95a529c
fix typo in changelog
Feb 3, 2023
438d592
fixup docstring to explain what a None prev_membership means
Feb 4, 2023
cfb71f0
fixup: is_host_in_room was already accurate given partial state
Feb 4, 2023
4fcf84e
make partial statedness of state maps explicit
Feb 4, 2023
4da4eda
fix typos and line wrapping
Feb 4, 2023
e97b44d
fixup: comment wording
Feb 10, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/14606.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Faster joins: don't stall when another user joins during a fast join resync.
22 changes: 22 additions & 0 deletions synapse/api/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -751,3 +751,25 @@ class ModuleFailedException(Exception):
Raised when a module API callback fails, for example because it raised an
exception.
"""


class PartialStateConflictError(SynapseError):
"""An internal error raised when attempting to persist an event with partial state
after the room containing the event has been un-partial stated.

This error should be handled by recomputing the event context and trying again.

This error has an HTTP status code so that it can be transported over replication.
It should not be exposed to clients.
"""

@staticmethod
def message() -> str:
return "Cannot persist partial state event in un-partial stated room"

def __init__(self) -> None:
super().__init__(
HTTPStatus.CONFLICT,
msg=PartialStateConflictError.message(),
errcode=Codes.UNKNOWN,
)
2 changes: 1 addition & 1 deletion synapse/federation/federation_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
FederationError,
IncompatibleRoomVersionError,
NotFoundError,
PartialStateConflictError,
SynapseError,
UnsupportedRoomVersionError,
)
Expand Down Expand Up @@ -72,7 +73,6 @@
ReplicationFederationSendEduRestServlet,
ReplicationGetQueryRestServlet,
)
from synapse.storage.databases.main.events import PartialStateConflictError
from synapse.storage.databases.main.lock import Lock
from synapse.storage.databases.main.roommember import extract_heroes_from_room_summary
from synapse.storage.roommember import MemberSummary
Expand Down
16 changes: 8 additions & 8 deletions synapse/handlers/event_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ async def check_restricted_join_rules(
state_ids: StateMap[str],
room_version: RoomVersion,
user_id: str,
prev_member_event: Optional[EventBase],
prev_membership: Optional[str],
) -> None:
"""
Check whether a user can join a room without an invite due to restricted join rules.
Expand All @@ -214,15 +214,14 @@ async def check_restricted_join_rules(
state_ids: The state of the room as it currently is.
room_version: The room version of the room being joined.
user_id: The user joining the room.
prev_member_event: The current membership event for this user.
prev_membership: The current membership state for this user. `None` if the
user has never joined the room (equivalent to "leave").

Raises:
AuthError if the user cannot join the room.
"""
# If the member is invited or currently joined, then nothing to do.
if prev_member_event and (
prev_member_event.membership in (Membership.JOIN, Membership.INVITE)
):
if prev_membership in (Membership.JOIN, Membership.INVITE):
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
return

# This is not a room with a restricted join rule, so we don't need to do the
Expand Down Expand Up @@ -255,13 +254,14 @@ async def check_restricted_join_rules(
)

async def has_restricted_join_rules(
self, state_ids: StateMap[str], room_version: RoomVersion
self, partial_state_ids: StateMap[str], room_version: RoomVersion
) -> bool:
"""
Return if the room has the proper join rules set for access via rooms.

Args:
state_ids: The state of the room as it currently is.
state_ids: The state of the room as it currently is. May be full or partial
state.
room_version: The room version of the room to query.

Returns:
Expand All @@ -272,7 +272,7 @@ async def has_restricted_join_rules(
return False

# If there's no join rule, then it defaults to invite (so this doesn't apply).
join_rules_event_id = state_ids.get((EventTypes.JoinRules, ""), None)
join_rules_event_id = partial_state_ids.get((EventTypes.JoinRules, ""), None)
if not join_rules_event_id:
return False

Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
FederationPullAttemptBackoffError,
HttpResponseException,
NotFoundError,
PartialStateConflictError,
RequestSendFailed,
SynapseError,
)
Expand All @@ -58,7 +59,6 @@
ReplicationCleanRoomRestServlet,
ReplicationStoreRoomOnOutlierMembershipRestServlet,
)
from synapse.storage.databases.main.events import PartialStateConflictError
from synapse.storage.databases.main.events_worker import EventRedactBehaviour
from synapse.types import JsonDict, StrCollection, get_domain_from_id
from synapse.types.state import StateFilter
Expand Down
59 changes: 53 additions & 6 deletions synapse/handlers/federation_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
FederationError,
FederationPullAttemptBackoffError,
HttpResponseException,
PartialStateConflictError,
RequestSendFailed,
SynapseError,
)
Expand Down Expand Up @@ -74,7 +75,6 @@
ReplicationFederationSendEventsRestServlet,
)
from synapse.state import StateResolutionStore
from synapse.storage.databases.main.events import PartialStateConflictError
from synapse.storage.databases.main.events_worker import EventRedactBehaviour
from synapse.types import (
PersistedEventPosition,
Expand Down Expand Up @@ -439,16 +439,17 @@ async def check_join_restrictions(
# Check if the user is already in the room or invited to the room.
user_id = event.state_key
prev_member_event_id = prev_state_ids.get((EventTypes.Member, user_id), None)
prev_member_event = None
prev_membership = None
if prev_member_event_id:
prev_member_event = await self._store.get_event(prev_member_event_id)
prev_membership = prev_member_event.membership

# Check if the member should be allowed access via membership in a space.
await self._event_auth_handler.check_restricted_join_rules(
prev_state_ids,
event.room_version,
user_id,
prev_member_event,
prev_membership,
)

@trace
Expand Down Expand Up @@ -524,11 +525,57 @@ async def process_remote_join(
"Peristing join-via-remote %s (partial_state: %s)", event, partial_state
)
with nested_logging_context(suffix=event.event_id):
if partial_state:
# When handling a second partial state join into a partial state room,
# the returned state will exclude the membership from the first join. To
# preserve prior memberships, we try to compute the partial state before
# the event ourselves if we know about any of the prev events.
#
# When we don't know about any of the prev events, it's fine to just use
# the returned state, since the new join will create a new forward
# extremity, and leave the forward extremity containing our prior
# memberships alone.
prev_event_ids = set(event.prev_event_ids())
seen_event_ids = await self._store.have_events_in_timeline(
prev_event_ids
)
missing_event_ids = prev_event_ids - seen_event_ids

state_maps_to_resolve: List[StateMap[str]] = []

# Fetch the state after the prev events that we know about.
state_maps_to_resolve.extend(
(
await self._state_storage_controller.get_state_groups_ids(
room_id, seen_event_ids, await_full_state=False
)
).values()
)

# When there are prev events we do not have the state for, we state
# resolve with the state returned by the remote homeserver.
if missing_event_ids or len(state_maps_to_resolve) == 0:
state_maps_to_resolve.append(
{(e.type, e.state_key): e.event_id for e in state}
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
)

state_ids_before_event = (
await self._state_resolution_handler.resolve_events_with_store(
event.room_id,
room_version.identifier,
state_maps_to_resolve,
event_map=None,
state_res_store=StateResolutionStore(self._store),
)
)
else:
state_ids_before_event = {
(e.type, e.state_key): e.event_id for e in state
}

context = await self._state_handler.compute_event_context(
event,
state_ids_before_event={
(e.type, e.state_key): e.event_id for e in state
},
state_ids_before_event=state_ids_before_event,
partial_state=partial_state,
)

Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
Codes,
ConsentNotGivenError,
NotFoundError,
PartialStateConflictError,
ShadowBanError,
SynapseError,
UnstableSpecAuthError,
Expand All @@ -57,7 +58,6 @@
from synapse.metrics.background_process_metrics import run_as_background_process
from synapse.replication.http.send_event import ReplicationSendEventRestServlet
from synapse.replication.http.send_events import ReplicationSendEventsRestServlet
from synapse.storage.databases.main.events import PartialStateConflictError
from synapse.storage.databases.main.events_worker import EventRedactBehaviour
from synapse.types import (
MutableStateMap,
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
Codes,
LimitExceededError,
NotFoundError,
PartialStateConflictError,
StoreError,
SynapseError,
)
Expand All @@ -53,7 +54,6 @@
from synapse.handlers.relations import BundledAggregations
from synapse.module_api import NOT_SPAM
from synapse.rest.admin._base import assert_user_is_admin
from synapse.storage.databases.main.events import PartialStateConflictError
from synapse.streams import EventSource
from synapse.types import (
JsonDict,
Expand Down
Loading