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

Backfill in the background if we're doing it "just because" #15702

Closed
MadLittleMods opened this issue Jun 1, 2023 · 0 comments · Fixed by #15710
Closed

Backfill in the background if we're doing it "just because" #15702

MadLittleMods opened this issue Jun 1, 2023 · 0 comments · Fixed by #15710
Assignees
Labels
A-Federation A-Messages-Endpoint /messages client API endpoint (`RoomMessageListRestServlet`) (which also triggers /backfill) A-Performance Performance, both client-facing and admin-facing O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@MadLittleMods
Copy link
Contributor

With Synapse currently, if you're paginating in a section of timeline back in time without any backward extremities, Synapse will still try to backfill any backward extremities that come later in the room (where you're not even viewing). This is fine from a eventual consistency perspective but is unlikely that the extra /backfill will reveal any new messages to return in the /messages request that triggered it in the first place.

Instead of doing this in the foreground which causes unnecessary delay for the user, we can just do this in the background.

# 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 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."
)

I completely acknowledge that a backward extremity later in the DAG, could connect to an unknown branch in the DAG that has events at the depth we're scrolling through but how likely is this to happen? This would be an interesting metric to track (how many events from /backfill actually end up in /messages)

@MadLittleMods MadLittleMods added A-Federation A-Performance Performance, both client-facing and admin-facing A-Messages-Endpoint /messages client API endpoint (`RoomMessageListRestServlet`) (which also triggers /backfill) labels Jun 1, 2023
@MadLittleMods MadLittleMods changed the title Backfill in the background if we're just doing it "just because" Backfill in the background if we're doing it "just because" Jun 1, 2023
@MadLittleMods MadLittleMods added the T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. label Jun 1, 2023
@MadLittleMods MadLittleMods self-assigned this Jun 5, 2023
@H-Shay H-Shay added O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. labels Jun 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Federation A-Messages-Endpoint /messages client API endpoint (`RoomMessageListRestServlet`) (which also triggers /backfill) A-Performance Performance, both client-facing and admin-facing O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants