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

Fix federation backfill bugs #3261

Merged
merged 3 commits into from
May 24, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 22 additions & 9 deletions synapse/handlers/federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -751,9 +751,19 @@ def maybe_backfill(self, room_id, current_depth):
curr_state = yield self.state_handler.get_current_state(room_id)

def get_domains_from_state(state):
"""Get joined domains from state

Args:
state (dict[tuple, FrozenEvent]): State map from type/state
key to event.

Returns:
list[tuple[str, int]]: Returns a list of servers with the
lowest depth of their joins. Sorted by lowest depth first.
"""
joined_users = [
(state_key, int(event.depth))
for (e_type, state_key), event in state.items()
for (e_type, state_key), event in state.iteritems()
if e_type == EventTypes.Member
and event.membership == Membership.JOIN
]
Expand All @@ -770,7 +780,7 @@ def get_domains_from_state(state):
except Exception:
pass

return sorted(joined_domains.items(), key=lambda d: d[1])
return sorted(joined_domains.iteritems(), key=lambda d: d[1])

curr_domains = get_domains_from_state(curr_state)

Expand All @@ -787,7 +797,7 @@ def try_backfill(domains):
yield self.backfill(
dom, room_id,
limit=100,
extremities=[e for e in extremities.keys()]
extremities=extremities,
)
# If this succeeded then we probably already have the
# appropriate stuff.
Expand Down Expand Up @@ -833,7 +843,7 @@ def try_backfill(domains):
tried_domains = set(likely_domains)
tried_domains.add(self.server_name)

event_ids = list(extremities.keys())
event_ids = list(extremities.iterkeys())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this the same as list(extremities)? Happy for this to be explicit but it's the opposite to what you've done at line 800


logger.debug("calling resolve_state_groups in _maybe_backfill")
resolve = logcontext.preserve_fn(
Expand All @@ -843,31 +853,34 @@ def try_backfill(domains):
[resolve(room_id, [e]) for e in event_ids],
consumeErrors=True,
))

# dict[str, dict[tuple, str]], a map from event_id to state map of
# event_ids.
states = dict(zip(event_ids, [s.state for s in states]))

state_map = yield self.store.get_events(
[e_id for ids in states.values() for e_id in ids],
[e_id for ids in states.itervalues() for e_id in ids.itervalues()],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this changing from for e_id in ids to for e_id in ids.itervalues() ?

assuming you've just unpicked what states is in here, could you add some comments about it? I really shouldn't have to go and spelunk in resolve_state_groups_for_events for it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have added a comment and typing info

get_prev_content=False
)
states = {
key: {
k: state_map[e_id]
for k, e_id in state_dict.items()
for k, e_id in state_dict.iteritems()
if e_id in state_map
} for key, state_dict in states.items()
} for key, state_dict in states.iteritems()
}

for e_id, _ in sorted_extremeties_tuple:
likely_domains = get_domains_from_state(states[e_id])

success = yield try_backfill([
dom for dom in likely_domains
dom for dom, _ in likely_domains
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks bogus?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_domains_from_state returns a list[tuple[str, int]]

if dom not in tried_domains
])
if success:
defer.returnValue(True)

tried_domains.update(likely_domains)
tried_domains.update(dom for dom, _ in likely_domains)

defer.returnValue(False)

Expand Down