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

Faster room joins: avoid blocking when pulling events with missing prevs #13355

Merged
merged 10 commits into from
Jul 26, 2022
Merged

Faster room joins: avoid blocking when pulling events with missing prevs #13355

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ad5d75e
Skip soft fail checks for rooms with partial state
Jul 20, 2022
ff8f6cf
Make the `partial_state` parameter to `compute_event_context` stricter
Jul 8, 2022
f7c9a5a
Add `partial_state` parameter to `_process_received_pdu`
Jul 8, 2022
bca5a17
Add `await_full_state` parameter to `get_state_groups_ids`, so that i…
Jul 8, 2022
1d48d23
Allow `_resolve_state_at_missing_prevs` to return partial state, othe…
Jul 8, 2022
6517210
Retry persisting events from federation when we lose the partial stat…
Jul 8, 2022
b879d8c
Add newsfile
Jul 21, 2022
7d55d88
Raise an error if we still have partial state on the 2nd try
Jul 22, 2022
7870d44
Add comment for update_state_for_partial_state_event optimisation
Jul 22, 2022
765614f
Update synapse/handlers/federation_event.py
squahtx Jul 25, 2022
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
8 changes: 6 additions & 2 deletions synapse/storage/controllers/state.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,15 @@ async def get_state_group_delta(
return state_group_delta.prev_group, state_group_delta.delta_ids

async def get_state_groups_ids(
Copy link
Member

Choose a reason for hiding this comment

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

as a random point for future reference: there is a convention that the subject line of a git commit message should be limited to 50 characters. That helps avoid this sort of thing:

image

There's an art to writing such a pithy summary, of course. In this case I would probably just say:

`get_state_groups_ids`: add `await_full_state` parameter

... and put "so that it can return partial state" in the body of the commit message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

interesting, I wasn't aware that the subject line had a shorter recommended length.

Up until now I've been using a limit of 72 characters in merge commits (and ignoring it in commits that get squashed, like the one above). 50 characters certainly makes things tricky!

Copy link
Member

Choose a reason for hiding this comment

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

ignoring it in commits that get squashed, like the one above

Yeah, the problem comes when we're doing a commit-by-commit review: it's much nicer if there's a pithy summary for each commit. It's certainly not worth stressing overmuch over though.

50 characters certainly makes things tricky!

Indeed. github actually seems to truncate at 66 chars, so that seems a more reasonable compromise.

self, _room_id: str, event_ids: Collection[str]
self, _room_id: str, event_ids: Collection[str], await_full_state: bool = True
) -> Dict[int, MutableStateMap[str]]:
"""Get the event IDs of all the state for the state groups for the given events

Args:
_room_id: id of the room for these events
event_ids: ids of the events
await_full_state: if `True`, will block if we do not yet have complete
state at these events.

Returns:
dict of state_group_id -> (dict of (type, state_key) -> event id)
Expand All @@ -100,7 +102,9 @@ async def get_state_groups_ids(
if not event_ids:
return {}

event_to_groups = await self.get_state_group_for_events(event_ids)
event_to_groups = await self.get_state_group_for_events(
event_ids, await_full_state=await_full_state
)

groups = set(event_to_groups.values())
group_to_state = await self.stores.state._get_state_for_groups(groups)
Expand Down