From 969743b113efbcd9b1ed5e6794a37c581cb591e5 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 19 Jan 2022 18:38:07 +0000 Subject: [PATCH 1/2] Comments and typing for `_update_outliers_txn` A couple of surprises for me here, so thought I'd document them --- changelog.d/11776.misc | 1 + synapse/storage/databases/main/events.py | 33 ++++++++++++++++-------- 2 files changed, 23 insertions(+), 11 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 000000000000..572ccda84741 --- /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 727800232207..9c20248bbde1 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -1254,20 +1254,22 @@ def _update_room_depths_txn( 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 @@ def _update_outliers_txn(self, txn, events_and_contexts): [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,8 +1289,8 @@ def _update_outliers_txn(self, txn, events_and_contexts): 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] @@ -1294,6 +1298,13 @@ def _update_outliers_txn(self, txn, events_and_contexts): # 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 # 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: From 2071f81dea4ca676da649e0e0a564492f9885788 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Wed, 19 Jan 2022 19:19:15 +0000 Subject: [PATCH 2/2] fix missing word --- synapse/storage/databases/main/events.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 9c20248bbde1..1ae1ebe10879 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -1296,7 +1296,7 @@ def _update_outliers_txn( 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