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

Commit

Permalink
Faster joins: don't stall when a user joins during a fast join (#14606)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
3 people committed Feb 10, 2023
1 parent d0c713c commit 6cddf24
Show file tree
Hide file tree
Showing 12 changed files with 196 additions and 94 deletions.
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 @@ -48,6 +48,7 @@
FederationError,
IncompatibleRoomVersionError,
NotFoundError,
PartialStateConflictError,
SynapseError,
UnsupportedRoomVersionError,
)
Expand Down Expand Up @@ -81,7 +82,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):
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 @@ -49,6 +49,7 @@
FederationPullAttemptBackoffError,
HttpResponseException,
NotFoundError,
PartialStateConflictError,
RequestSendFailed,
SynapseError,
)
Expand All @@ -68,7 +69,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 @@ -441,16 +441,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 @@ -526,11 +527,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}
)

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 @@ -43,6 +43,7 @@
Codes,
LimitExceededError,
NotFoundError,
PartialStateConflictError,
StoreError,
SynapseError,
)
Expand All @@ -54,7 +55,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

0 comments on commit 6cddf24

Please sign in to comment.