From 5572e6cc4b0f7f3a907773f29d82671202812a6d Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Wed, 19 Jan 2022 19:45:36 +0000
Subject: [PATCH] Comments and typing for `_update_outliers_txn` (#11776)

A couple of surprises for me here, so thought I'd document them
---
 changelog.d/11776.misc                   |  1 +
 synapse/storage/databases/main/events.py | 35 ++++++++++++++++--------
 2 files changed, 24 insertions(+), 12 deletions(-)
 create mode 100644 changelog.d/11776.misc

diff --git a/changelog.d/11776.misc b/changelog.d/11776.misc
new file mode 100644
index 000000000..572ccda84
--- /dev/null
+++ b/changelog.d/11776.misc
@@ -0,0 +1 @@
+Add some comments and type annotations for `_update_outliers_txn`.
diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py
index 727800232..1ae1ebe10 100644
--- a/synapse/storage/databases/main/events.py
+++ b/synapse/storage/databases/main/events.py
@@ -1254,20 +1254,22 @@ class PersistEventsStore:
         for room_id, depth in depth_updates.items():
             self._update_min_depth_for_room_txn(txn, room_id, depth)
 
-    def _update_outliers_txn(self, txn, events_and_contexts):
+    def _update_outliers_txn(
+        self,
+        txn: LoggingTransaction,
+        events_and_contexts: List[Tuple[EventBase, EventContext]],
+    ) -> List[Tuple[EventBase, EventContext]]:
         """Update any outliers with new event info.
 
-        This turns outliers into ex-outliers (unless the new event was
-        rejected).
+        This turns outliers into ex-outliers (unless the new event was rejected), and
+        also removes any other events we have already seen from the list.
 
         Args:
-            txn (twisted.enterprise.adbapi.Connection): db connection
-            events_and_contexts (list[(EventBase, EventContext)]): events
-                we are persisting
+            txn: db connection
+            events_and_contexts: events we are persisting
 
         Returns:
-            list[(EventBase, EventContext)] new list, without events which
-            are already in the events table.
+            new list, without events which are already in the events table.
         """
         txn.execute(
             "SELECT event_id, outlier FROM events WHERE event_id in (%s)"
@@ -1275,7 +1277,9 @@ class PersistEventsStore:
             [event.event_id for event, _ in events_and_contexts],
         )
 
-        have_persisted = {event_id: outlier for event_id, outlier in txn}
+        have_persisted: Dict[str, bool] = {
+            event_id: outlier for event_id, outlier in txn
+        }
 
         to_remove = set()
         for event, context in events_and_contexts:
@@ -1285,15 +1289,22 @@ class PersistEventsStore:
             to_remove.add(event)
 
             if context.rejected:
-                # If the event is rejected then we don't care if the event
-                # was an outlier or not.
+                # If the incoming event is rejected then we don't care if the event
+                # was an outlier or not - what we have is at least as good.
                 continue
 
             outlier_persisted = have_persisted[event.event_id]
             if not event.internal_metadata.is_outlier() and outlier_persisted:
                 # We received a copy of an event that we had already stored as
-                # an outlier in the database. We now have some state at that
+                # an outlier in the database. We now have some state at that event
                 # so we need to update the state_groups table with that state.
+                #
+                # Note that we do not update the stream_ordering of the event in this
+                # scenario. XXX: does this cause bugs? It will mean we won't send such
+                # events down /sync. In general they will be historical events, so that
+                # doesn't matter too much, but that is not always the case.
+
+                logger.info("Updating state for ex-outlier event %s", event.event_id)
 
                 # insert into event_to_state_groups.
                 try: