Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Fix mainline ordering in state res v2 (#8971)
Browse files Browse the repository at this point in the history
This had two effects 1) it'd give the wrong answer and b) would iterate
*all* power levels in the auth chain of each event. The latter of which
can be *very* expensive for certain types of IRC bridge rooms that have
large numbers of power level changes.
  • Loading branch information
erikjohnston authored Dec 18, 2020
1 parent 28877fa commit 5e7d75d
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 2 deletions.
1 change: 1 addition & 0 deletions changelog.d/8971.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix small bug in v2 state resolution algorithm, which could also cause performance issues for rooms with large numbers of power levels.
2 changes: 1 addition & 1 deletion synapse/state/v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ async def _get_mainline_depth_for_event(
# We do an iterative search, replacing `event with the power level in its
# auth events (if any)
while tmp_event:
depth = mainline_map.get(event.event_id)
depth = mainline_map.get(tmp_event.event_id)
if depth is not None:
return depth

Expand Down
57 changes: 56 additions & 1 deletion tests/state/test_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def to_event(self, auth_events, prev_events):
event_dict = {
"auth_events": [(a, {}) for a in auth_events],
"prev_events": [(p, {}) for p in prev_events],
"event_id": self.node_id,
"event_id": self.event_id,
"sender": self.sender,
"type": self.type,
"content": self.content,
Expand Down Expand Up @@ -381,6 +381,61 @@ def test_topic(self):

self.do_check(events, edges, expected_state_ids)

def test_mainline_sort(self):
"""Tests that the mainline ordering works correctly.
"""

events = [
FakeEvent(
id="T1", sender=ALICE, type=EventTypes.Topic, state_key="", content={}
),
FakeEvent(
id="PA1",
sender=ALICE,
type=EventTypes.PowerLevels,
state_key="",
content={"users": {ALICE: 100, BOB: 50}},
),
FakeEvent(
id="T2", sender=ALICE, type=EventTypes.Topic, state_key="", content={}
),
FakeEvent(
id="PA2",
sender=ALICE,
type=EventTypes.PowerLevels,
state_key="",
content={
"users": {ALICE: 100, BOB: 50},
"events": {EventTypes.PowerLevels: 100},
},
),
FakeEvent(
id="PB",
sender=BOB,
type=EventTypes.PowerLevels,
state_key="",
content={"users": {ALICE: 100, BOB: 50}},
),
FakeEvent(
id="T3", sender=BOB, type=EventTypes.Topic, state_key="", content={}
),
FakeEvent(
id="T4", sender=ALICE, type=EventTypes.Topic, state_key="", content={}
),
]

edges = [
["END", "T3", "PA2", "T2", "PA1", "T1", "START"],
["END", "T4", "PB", "PA1"],
]

# We expect T3 to be picked as the other topics are pointing at older
# power levels. Note that without mainline ordering we'd pick T4 due to
# it being sent *after* T3.
expected_state_ids = ["T3", "PA2"]

self.do_check(events, edges, expected_state_ids)

def do_check(self, events, edges, expected_state_ids):
"""Take a list of events and edges and calculate the state of the
graph at END, and asserts it matches `expected_state_ids`
Expand Down

0 comments on commit 5e7d75d

Please sign in to comment.