This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Avoid blocking lazy-loading /sync
s during partial joins
#13477
Merged
squahtx
merged 12 commits into
develop
from
squah/faster_room_joins_unblock_lazy_loading_sync
Aug 18, 2022
Merged
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
31a2e5e
Add option for `get_state_ids_for_event(s)` to return partial state
0a539b9
Add option to `get_state_at/after_event` to return partial state
ad8bab8
Do not wait for full state in a few cases in `_load_filtered_recents`
10013ea
Do not wait for full state in a few cases in `_get_rooms_changed`
5274c87
Do not wait for full state in `compute_state_delta`
04bee9e
Dig up memberships for lazy-loading syncs in partial state rooms
ad8a2b3
Add newsfile
f728bac
fixup: make `await_full_state` a boolean again
24821a2
fixup: explain why `await_full_state` is off when lazy loading members
8f8c5bd
fixup: Don't include own membership in `members_to_fetch`
7fe2654
fixup: factor out logic to find missing memberships in auth events
1a33404
fixup: Split members_to_fetch into a set and a dictionary
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -918,8 +918,14 @@ async def compute_state_delta( | |
|
||
with Measure(self.clock, "compute_state_delta"): | ||
# The memberships needed for events in the timeline. | ||
# A dictionary with user IDs as keys and the first event in the timeline | ||
# requiring each member as values. | ||
# Only calculated when `lazy_load_members` is on. | ||
members_to_fetch = None | ||
members_to_fetch: Optional[Dict[str, Optional[EventBase]]] = None | ||
|
||
# The contribution to the room state from state events in the timeline. | ||
# Only contains the last event for any given state key. | ||
timeline_state: StateMap[str] | ||
|
||
lazy_load_members = sync_config.filter_collection.lazy_load_members() | ||
include_redundant_members = ( | ||
|
@@ -930,29 +936,38 @@ async def compute_state_delta( | |
# We only request state for the members needed to display the | ||
# timeline: | ||
|
||
members_to_fetch = { | ||
event.sender # FIXME: we also care about invite targets etc. | ||
for event in batch.events | ||
} | ||
timeline_state = {} | ||
|
||
members_to_fetch = {} | ||
for event in batch.events: | ||
# We need the event's sender, unless their membership was in a | ||
# previous timeline event. | ||
if ( | ||
EventTypes.Member, | ||
event.sender, | ||
) not in timeline_state and event.sender not in members_to_fetch: | ||
members_to_fetch[event.sender] = event | ||
# FIXME: we also care about invite targets etc. | ||
|
||
if event.is_state(): | ||
timeline_state[(event.type, event.state_key)] = event.event_id | ||
|
||
if full_state: | ||
# always make sure we LL ourselves so we know we're in the room | ||
# (if we are) to fix https://github.com/vector-im/riot-web/issues/7209 | ||
# We only need apply this on full state syncs given we disabled | ||
# LL for incr syncs in #3840. | ||
members_to_fetch.add(sync_config.user.to_string()) | ||
members_to_fetch[sync_config.user.to_string()] = None | ||
|
||
state_filter = StateFilter.from_lazy_load_member_list(members_to_fetch) | ||
else: | ||
state_filter = StateFilter.all() | ||
timeline_state = { | ||
(event.type, event.state_key): event.event_id | ||
for event in batch.events | ||
if event.is_state() | ||
} | ||
|
||
# The contribution to the room state from state events in the timeline. | ||
# Only contains the last event for any given state key. | ||
timeline_state = { | ||
(event.type, event.state_key): event.event_id | ||
for event in batch.events | ||
if event.is_state() | ||
} | ||
state_filter = StateFilter.all() | ||
|
||
# Now calculate the state to return in the sync response for the room. | ||
# This is more or less the change in state between the end of the previous | ||
|
@@ -1087,7 +1102,76 @@ async def compute_state_delta( | |
await_full_state=False, | ||
) | ||
|
||
# FIXME: `state_ids` may be missing memberships for partial state rooms. | ||
# If we only have partial state for the room, `state_ids` may be missing the | ||
# memberships we wanted. We attempt to find some by digging through the auth | ||
# events of timeline events. | ||
if lazy_load_members: | ||
assert members_to_fetch is not None | ||
|
||
is_partial_state = await self.store.is_partial_state_room(room_id) | ||
if is_partial_state: | ||
additional_state_ids: MutableStateMap[str] = {} | ||
|
||
# Tracks the missing members for logging purposes. | ||
missing_members = {} | ||
|
||
# Pick out the auth events of timeline events whose sender | ||
# memberships are missing. | ||
auth_event_ids: Set[str] = set() | ||
for member, first_referencing_event in members_to_fetch.items(): | ||
if ( | ||
first_referencing_event is None | ||
richvdh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
or (EventTypes.Member, member) in state_ids | ||
): | ||
continue | ||
|
||
missing_members[member] = first_referencing_event | ||
auth_event_ids.update(first_referencing_event.auth_event_ids()) | ||
|
||
auth_events = await self.store.get_events(auth_event_ids) | ||
|
||
# Run through the events with missing sender memberships once more, | ||
# picking out the memberships from the pile of auth events we have | ||
# just fetched. | ||
for member, first_referencing_event in members_to_fetch.items(): | ||
if ( | ||
first_referencing_event is None | ||
or (EventTypes.Member, member) in state_ids | ||
): | ||
continue | ||
|
||
# Dig through the auth events to find the sender's membership. | ||
for auth_event_id in first_referencing_event.auth_event_ids(): | ||
# We only store events once we have all their auth events, | ||
# so the auth event must be in the pile we have just | ||
# fetched. | ||
auth_event = auth_events[auth_event_id] | ||
|
||
if ( | ||
auth_event.type == EventTypes.Member | ||
and auth_event.state_key == event.sender | ||
): | ||
missing_members.pop(member) | ||
additional_state_ids[ | ||
(EventTypes.Member, event.sender) | ||
] = auth_event.event_id | ||
break | ||
|
||
# Now merge in the state we have scrounged up. | ||
state_ids = {**state_ids, **additional_state_ids} | ||
|
||
if missing_members: | ||
# There really shouldn't be any missing memberships now. | ||
# For an event to appear in the timeline, we must have its auth | ||
# events, which must include its sender's membership. | ||
logger.error( | ||
"Failed to find memberships for %s in partial state room " | ||
"%s in the auth events of %s.", | ||
list(missing_members.keys()), | ||
room_id, | ||
list(missing_members.values()), | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we move all this out to a separate function which returns There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I gave that a go in the latest commit. |
||
|
||
# At this point, if `lazy_load_members` is enabled, `state_ids` includes | ||
# the memberships of all event senders in the timeline. This is because we | ||
# may not have sent the memberships in a previous sync. | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change calculates
members_to_fetch
more accurately, by excluding event senders whose membership appears previously in the timeline.