Fix backfill storing incorrect state for events #4718
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #4718 +/- ##
===========================================
+ Coverage 75.15% 75.25% +0.09%
===========================================
Files 340 340
Lines 34823 34829 +6
Branches 5704 5704
===========================================
+ Hits 26171 26209 +38
+ Misses 7042 7019 -23
+ Partials 1610 1601 -9 |
| @@ -786,13 +801,18 @@ def backfill(self, dest, room_id, limit, extremities): | |||
| }) | |||
|
|
|||
| for e_id in events_to_state: | |||
| # For paranoia we ensure that these events are marked as | |||
There was a problem hiding this comment.
this seems to be in conflict with the comment at line 775 that says "... state get persisted as outliers". What am I missing?
There was a problem hiding this comment.
This needs more comments, obviously. The keys of events_to_state are the new backwards extremities, as those are the events that we fetched state for
There was a problem hiding this comment.
it would seem a whole lot less opaque to iterate over edges rather than the keys of events_to_state, but ok.
synapse/handlers/federation.py
Outdated
| # For paranoia we ensure that these events are marked as | ||
| # non-outliers | ||
| ev = event_map[e_id] | ||
| ev.internal_metadata.outlier = False |
There was a problem hiding this comment.
this sort of thing always alarms me slightly; if outlier was set doesn't it mean that something else has gone wrong? shouldn't we just raise an AssertionError or something?
There was a problem hiding this comment.
Assert is actually probably what we want here, yes
|
[sad pep8 is sad] |
changelog.d/4718.bugfix
Outdated
| @@ -0,0 +1 @@ | |||
| Fix paginating over federation persisting incorrect state | |||
Broadly, we were going through marking all fetched "auth events" as outliers, even if they appeared in the main backfill chunk of events. This led to state getting very confused when we tried to persist events that referenced outliers.