From e49d58fb50b68819b00c6e95caed87e1a199c02e Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 2 Jun 2023 19:29:11 -0500 Subject: [PATCH 1/3] Backfill in the background if we're doing it "just because" Fix https://github.com/matrix-org/synapse/issues/15702 --- synapse/handlers/federation.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 2eb28d55ac82..a5bbca0c25a1 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -310,7 +310,9 @@ async def _maybe_backfill_inner( logger.debug( "_maybe_backfill_inner: all backfill points are *after* current depth. Trying again with later backfill points." ) - return await self._maybe_backfill_inner( + run_as_background_process( + "_maybe_backfill_inner_anyway_with_max_depth", + self._maybe_backfill_inner, room_id=room_id, # We use `MAX_DEPTH` so that we find all backfill points next # time (all events are below the `MAX_DEPTH`) @@ -321,6 +323,10 @@ async def _maybe_backfill_inner( # overall otherwise the smaller one will throw off the results. processing_start_time=None, ) + # We return `False` because we're backfilling but doing it in the background + # and is just a best effort attempt for eventual consistency's sake. The + # return value of this function isn't used for anything yet anyway. + return False # Even after recursing with `MAX_DEPTH`, we didn't find any # backward extremities to backfill from. From 25c502a14ec31d2c49108e53ed3a3e7224013702 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 5 Jun 2023 00:45:51 -0500 Subject: [PATCH 2/3] Add changelog --- changelog.d/15710.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/15710.feature diff --git a/changelog.d/15710.feature b/changelog.d/15710.feature new file mode 100644 index 000000000000..fe77a2fef6e2 --- /dev/null +++ b/changelog.d/15710.feature @@ -0,0 +1 @@ +Speed up `/messages` by backfilling in the background when there are no backward extremities where we are directly paginating. From 0611c08d154bdea1ba9406b48c0b8c2fb9afeabb Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 7 Jun 2023 13:10:21 -0500 Subject: [PATCH 3/3] Update comments --- synapse/handlers/federation.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index da185e649da2..b7b5e2102036 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -320,9 +320,14 @@ async def _maybe_backfill_inner( str(len(sorted_backfill_points)), ) - # If we have no backfill points lower than the `current_depth` then - # either we can a) bail or b) still attempt to backfill. We opt to try - # backfilling anyway just in case we do get relevant events. + # If we have no backfill points lower than the `current_depth` then either we + # can a) bail or b) still attempt to backfill. We opt to try backfilling anyway + # just in case we do get relevant events. This is good for eventual consistency + # sake but we don't need to block the client for something that is just as + # likely not to return anything relevant so we backfill in the background. The + # only way, this could return something relevant is if we discover a new branch + # of history that extends all the way back to where we are currently paginating + # and it's within the 100 events that are returned from `/backfill`. if not sorted_backfill_points and current_depth != MAX_DEPTH: logger.debug( "_maybe_backfill_inner: all backfill points are *after* current depth. Trying again with later backfill points." @@ -340,9 +345,8 @@ async def _maybe_backfill_inner( # overall otherwise the smaller one will throw off the results. processing_start_time=None, ) - # We return `False` because we're backfilling but doing it in the background - # and is just a best effort attempt for eventual consistency's sake. The - # return value of this function isn't used for anything yet anyway. + # We return `False` because we're backfilling in the background and there is + # no new events immediately for the caller to know about yet. return False # Even after recursing with `MAX_DEPTH`, we didn't find any