Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pull less state out if we fail to backfill #16788

Merged
merged 5 commits into from
Jan 10, 2024
Merged
Changes from 1 commit
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
Next Next commit
Pull less state out if we fail to backfill
Sometimes we fail to fetch events during backfill due to missing state,
and we often end up querying the same bad events periodically (as people
backpaginate). In such cases its likely we will continue to fail to get
the state, and therefore we should try *before* loading the state that
we have from the DB (as otherwise it's wasted DB and memory).
  • Loading branch information
erikjohnston committed Jan 8, 2024
commit de5ec991eb49da8814ef1f8ffef95c8887c529d1
18 changes: 13 additions & 5 deletions synapse/handlers/federation_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -1147,10 +1147,7 @@ async def _compute_event_context_with_maybe_missing_prevs(
)
Copy link
Contributor

Choose a reason for hiding this comment

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

^ you didn't delete the old assignment of ours (at least, it seems to be dead).

Copy link
Member Author

Choose a reason for hiding this comment

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

🤦


# state_maps is a list of mappings from (type, state_key) to event_id
state_maps: List[StateMap[str]] = list(ours.values())

# we don't need this any more, let's delete it.
del ours
state_maps: List[StateMap[str]] = []

# Ask the remote server for the states we don't
# know about
Expand All @@ -1169,6 +1166,17 @@ async def _compute_event_context_with_maybe_missing_prevs(

state_maps.append(remote_state_map)

# Get the state of the events we know about. We do this *after*
# trying to fetch missing state over federation as that might fail
# and then we can skip loading the local state.
ours = await self._state_storage_controller.get_state_groups_ids(
room_id, seen, await_full_state=False
)
state_maps.extend(ours.values())

# we don't need this any more, let's delete it.
del ours

room_version = await self._store.get_room_version_id(room_id)
state_map = await self._state_resolution_handler.resolve_events_with_store(
room_id,
Expand Down Expand Up @@ -1299,7 +1307,7 @@ async def _get_state_ids_after_missing_prev_event(
destination=destination, room_id=room_id, event_ids=missing_event_ids
)

# We now need to fill out the state map, which involves fetching the
# We now need to fill out the state map, which involves loading the local state.he
erikjohnston marked this conversation as resolved.
Show resolved Hide resolved
# type and state key for each event ID in the state.
state_map = {}

Expand Down
Loading