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

changelog.d/14606.misc

@@ -0,0 +1 @@
+Faster joins: don't stale when another user joins during a fast join resync.

synapse/api/errors.py

@@ -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,
+        )

synapse/federation/federation_server.py

@@ -41,6 +41,7 @@
     FederationError,
     IncompatibleRoomVersionError,
     NotFoundError,
+    PartialStateConflictError,
     SynapseError,
     UnsupportedRoomVersionError,
 )
@@ -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

synapse/handlers/event_auth.py

@@ -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.
@@ -214,15 +214,13 @@ 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.
 
         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

synapse/handlers/federation.py

@@ -49,6 +49,7 @@
     HttpResponseException,
     LimitExceededError,
     NotFoundError,
+    PartialStateConflictError,
     RequestSendFailed,
     SynapseError,
 )
@@ -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, get_domain_from_id
 from synapse.types.state import StateFilter
@@ -562,6 +562,10 @@ async def do_invite_join(
             joinee: The User ID of the joining user.
 
             content: The event content to use for the join event.
+
+        Raises:
+            PartialStateConflictError if homeserver was already in the room and it is
+                no longer partial stated.
         """
         # TODO: We should be able to call this on workers, but the upgrading of
         # room stuff after join currently doesn't work on workers.
@@ -587,8 +591,6 @@ async def do_invite_join(
 
         self._federation_event_handler.room_queues[room_id] = []
 
-        await self._clean_room_for_join(room_id)
-
         try:
             # Try the host we successfully got a response to /make_join/
             # request first.
@@ -614,6 +616,8 @@ async def do_invite_join(
 
             logger.debug("do_invite_join event: %s", event)
 
+            await self._clean_room_for_join(room_id)
+
             # if this is the first time we've joined this room, it's time to add
             # a row to `rooms` with the correct room version. If there's already a
             # row there, we should override it, since it may have been populated
@@ -629,7 +633,8 @@ async def do_invite_join(
                 state_events=state,
             )
 
-            if ret.partial_state:
+            already_partially_joined = await self.store.is_partial_state_room(room_id)
+            if ret.partial_state and not already_partially_joined:
                 # Mark the room as having partial state.
                 # The background process is responsible for unmarking this flag,
                 # even if the join fails.
@@ -654,15 +659,14 @@ async def do_invite_join(
                 )
             except PartialStateConflictError as e:
                 # The homeserver was already in the room and it is no longer partial
-                # stated. We ought to be doing a local join instead. Turn the error into
-                # a 429, as a hint to the client to try again.
+                # stated. We ought to be doing a local join instead.
                 # TODO(faster_joins): `_should_perform_remote_join` suggests that we may
                 #   do a remote join for restricted rooms even if we have full state.
                 logger.error(
                     "Room %s was un-partial stated while processing remote join.",
                     room_id,
                 )
-                raise LimitExceededError(msg=e.msg, errcode=e.errcode, retry_after_ms=0)
+                raise e
             else:
                 # Record the join event id for future use (when we finish the full
                 # join). We have to do this after persisting the event to keep foreign
@@ -676,7 +680,7 @@ async def do_invite_join(
                 # state for the room.
                 # If the join failed, the background process is responsible for
                 # cleaning up — including unmarking the room as a partial state room.
-                if ret.partial_state:
+                if ret.partial_state and not already_partially_joined:
                     # Kick off the process of asynchronously fetching the state for this
                     # room.
                     run_as_background_process(

synapse/handlers/federation_event.py

@@ -47,6 +47,7 @@
     FederationError,
     FederationPullAttemptBackoffError,
     HttpResponseException,
+    PartialStateConflictError,
     RequestSendFailed,
     SynapseError,
 )
@@ -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,
@@ -438,16 +438,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

synapse/handlers/message.py

@@ -38,6 +38,7 @@
     Codes,
     ConsentNotGivenError,
     NotFoundError,
+    PartialStateConflictError,
     ShadowBanError,
     SynapseError,
     UnstableSpecAuthError,
@@ -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,

synapse/handlers/room.py

@@ -51,6 +51,7 @@
     Codes,
     LimitExceededError,
     NotFoundError,
+    PartialStateConflictError,
     StoreError,
     SynapseError,
 )
@@ -62,7 +63,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,