From de5ec991eb49da8814ef1f8ffef95c8887c529d1 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 8 Jan 2024 14:01:56 +0000 Subject: [PATCH 1/5] 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). --- synapse/handlers/federation_event.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 882be905dbb..e6af1bff055 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1147,10 +1147,7 @@ async def _compute_event_context_with_maybe_missing_prevs( ) # 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 @@ -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, @@ -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 # type and state key for each event ID in the state. state_map = {} From 4f6de3a51c23a29b44b66bcc2ce1f8b7353f3534 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 8 Jan 2024 14:04:49 +0000 Subject: [PATCH 2/5] Newsfile --- changelog.d/16788.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/16788.misc diff --git a/changelog.d/16788.misc b/changelog.d/16788.misc new file mode 100644 index 00000000000..e58a5a7a320 --- /dev/null +++ b/changelog.d/16788.misc @@ -0,0 +1 @@ +Pull less state out of the DB when we retry fetching old events during backfill. From de0c1a010c3387d10592a8e327c69cd6fbf5f877 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 10 Jan 2024 14:15:44 +0000 Subject: [PATCH 3/5] Update synapse/handlers/federation_event.py Co-authored-by: reivilibre --- synapse/handlers/federation_event.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index e6af1bff055..0da697efc84 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1307,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 loading the local state.he + # We now need to fill out the state map, which involves loading the local state. # type and state key for each event ID in the state. state_map = {} From c2d55241a8573794019ed0f0dde71743d1d3dc59 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 10 Jan 2024 14:15:56 +0000 Subject: [PATCH 4/5] Delete the code that is meant to be deleted --- synapse/handlers/federation_event.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 0da697efc84..d60565f6bba 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1141,11 +1141,6 @@ async def _compute_event_context_with_maybe_missing_prevs( partial_state_flags = await self._store.get_partial_state_events(seen) partial_state = any(partial_state_flags.values()) - # Get the state of the events we know about - ours = await self._state_storage_controller.get_state_groups_ids( - room_id, seen, await_full_state=False - ) - # state_maps is a list of mappings from (type, state_key) to event_id state_maps: List[StateMap[str]] = [] From a869faf0d578fc1bf5acfa90903db82b8ebf22d2 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 10 Jan 2024 14:18:02 +0000 Subject: [PATCH 5/5] Remove spurious comment change?! --- synapse/handlers/federation_event.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index d60565f6bba..b05a23c9d15 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1302,7 +1302,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 loading the local state. + # We now need to fill out the state map, which involves fetching the # type and state key for each event ID in the state. state_map = {}