Don't store prev state IDs and current state IDs

Author: erikjohnston

synapse/events/snapshot.py

@@ -57,6 +57,9 @@ class EventContext:
             If ``state_group`` is None (ie, the event is an outlier),
             ``state_group_before_event`` will always also be ``None``.
 
+        delta_before_after: If `state_group` and `state_group_before_event` are not None
+            then this is the delta of the state between the two groups.
+
         prev_group: If it is known, ``state_group``'s prev_group. Note that this being
             None does not necessarily mean that ``state_group`` does not have
             a prev_group!
@@ -75,30 +78,6 @@ class EventContext:
         app_service: If this event is being sent by a (local) application service, that
             app service.
 
-        _current_state_ids: The room state map, including this event - ie, the state
-            in ``state_group``.
-
-            (type, state_key) -> event_id
-
-            For an outlier, this is {}
-
-            Note that this is a private attribute: it should be accessed via
-            ``get_current_state_ids``. _AsyncEventContext impl calculates this
-            on-demand: it will be None until that happens.
-
-        _prev_state_ids: The room state map, excluding this event - ie, the state
-            in ``state_group_before_event``. For a non-state
-            event, this will be the same as _current_state_events.
-
-            Note that it is a completely different thing to prev_group!
-
-            (type, state_key) -> event_id
-
-            For an outlier, this is {}
-
-            As with _current_state_ids, this is a private attribute. It should be
-            accessed via get_prev_state_ids.
-
         partial_state: if True, we may be storing this event with a temporary,
             incomplete state.
     """
@@ -107,22 +86,19 @@ class EventContext:
     rejected: Union[bool, str] = False
     _state_group: Optional[int] = None
     state_group_before_event: Optional[int] = None
+    _delta_before_after: Optional[StateMap[str]] = None
     prev_group: Optional[int] = None
     delta_ids: Optional[StateMap[str]] = None
     app_service: Optional[ApplicationService] = None
 
-    _current_state_ids: Optional[StateMap[str]] = None
-    _prev_state_ids: Optional[StateMap[str]] = None
-
     partial_state: bool = False
 
     @staticmethod
     def with_state(
         storage: "Storage",
         state_group: Optional[int],
         state_group_before_event: Optional[int],
-        current_state_ids: Optional[StateMap[str]],
-        prev_state_ids: Optional[StateMap[str]],
+        delta_before_after: Optional[StateMap[str]],
         partial_state: bool,
         prev_group: Optional[int] = None,
         delta_ids: Optional[StateMap[str]] = None,
@@ -131,6 +107,7 @@ def with_state(
             storage=storage,
             state_group=state_group,
             state_group_before_event=state_group_before_event,
+            delta_before_after=delta_before_after,
             prev_group=prev_group,
             delta_ids=delta_ids,
             partial_state=partial_state,
@@ -141,11 +118,7 @@ def for_outlier(
         storage: "Storage",
     ) -> "EventContext":
         """Return an EventContext instance suitable for persisting an outlier event"""
-        return EventContext(
-            storage=storage,
-            current_state_ids={},
-            prev_state_ids={},
-        )
+        return EventContext(storage=storage)
 
     async def serialize(self, event: EventBase, store: "DataStore") -> JsonDict:
         """Converts self to a type that can be serialized as JSON, and then
@@ -163,6 +136,7 @@ async def serialize(self, event: EventBase, store: "DataStore") -> JsonDict:
             "state_group_before_event": self.state_group_before_event,
             "rejected": self.rejected,
             "prev_group": self.prev_group,
+            "delta_before_after": _encode_state_dict(self._delta_before_after),
             "delta_ids": _encode_state_dict(self.delta_ids),
             "app_service_id": self.app_service.id if self.app_service else None,
             "partial_state": self.partial_state,
@@ -187,6 +161,7 @@ def deserialize(storage: "Storage", input: JsonDict) -> "EventContext":
             state_group=input["state_group"],
             state_group_before_event=input["state_group_before_event"],
             prev_group=input["prev_group"],
+            delta_before_after=_decode_state_dict(input["delta_before_after"]),
             delta_ids=_decode_state_dict(input["delta_ids"]),
             rejected=input["rejected"],
             partial_state=input.get("partial_state", False),
@@ -234,10 +209,15 @@ async def get_current_state_ids(self) -> Optional[StateMap[str]]:
         if self.rejected:
             raise RuntimeError("Attempt to access state_ids of rejected event")
 
-        if self._state_group is None:
-            return None
+        assert self._delta_before_after is not None
+
+        prev_state_ids = await self.get_prev_state_ids()
+
+        if self._delta_before_after:
+            prev_state_ids = dict(prev_state_ids)
+            prev_state_ids.update(self._delta_before_after)
 
-        return await self._storage.state.get_state_ids_for_group(self._state_group)
+        return prev_state_ids
 
     async def get_prev_state_ids(self) -> StateMap[str]:
         """
@@ -252,7 +232,7 @@ async def get_prev_state_ids(self) -> StateMap[str]:
             Maps a (type, state_key) to the event ID of the state event matching
             this tuple.
         """
-        assert self.state_group_before_event
+        assert self.state_group_before_event is not None
         return await self._storage.state.get_state_ids_for_group(
             self.state_group_before_event
         )

synapse/handlers/federation_event.py

@@ -1877,8 +1877,7 @@ async def _update_context_for_auth_events(
             storage=self._storage,
             state_group=state_group,
             state_group_before_event=context.state_group_before_event,
-            current_state_ids=current_state_ids,
-            prev_state_ids=prev_state_ids,
+            delta_before_after=state_updates,
             prev_group=prev_group,
             delta_ids=state_updates,
             partial_state=context.partial_state,

synapse/handlers/message.py

@@ -757,6 +757,10 @@ async def deduplicate_state_event(
             The previous version of the event is returned, if it is found in the
             event context. Otherwise, None is returned.
         """
+        if event.internal_metadata.is_outlier():
+            # This can happen due to out of band memberships
+            return None
+
         prev_state_ids = await context.get_prev_state_ids()
         prev_event_id = prev_state_ids.get((event.type, event.state_key))
         if not prev_event_id:

synapse/push/action_generator.py

@@ -40,5 +40,9 @@ def __init__(self, hs: "HomeServer"):
     async def handle_push_actions_for_event(
         self, event: EventBase, context: EventContext
     ) -> None:
+        if event.internal_metadata.is_outlier():
+            # This can happen due to out of band memberships
+            return
+
         with Measure(self.clock, "action_for_event_by_user"):
             await self.bulk_evaluator.action_for_event_by_user(event, context)

synapse/state/__init__.py

@@ -365,8 +365,7 @@ async def compute_event_context(
                 storage=self._storage,
                 state_group_before_event=state_group_before_event,
                 state_group=state_group_before_event,
-                current_state_ids=state_ids_before_event,
-                prev_state_ids=state_ids_before_event,
+                delta_before_after={},
                 prev_group=state_group_before_event_prev_group,
                 delta_ids=deltas_to_state_group_before_event,
                 partial_state=partial_state,
@@ -398,8 +397,7 @@ async def compute_event_context(
             storage=self._storage,
             state_group=state_group_after_event,
             state_group_before_event=state_group_before_event,
-            current_state_ids=state_ids_after_event,
-            prev_state_ids=state_ids_before_event,
+            delta_before_after=delta_ids,
             prev_group=state_group_before_event,
             delta_ids=delta_ids,
             partial_state=partial_state,