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

We sometimes backfill on /messages requests when we shouldn't. #10742

Open
erikjohnston opened this issue Sep 2, 2021 · 0 comments
Open

We sometimes backfill on /messages requests when we shouldn't. #10742

erikjohnston opened this issue Sep 2, 2021 · 0 comments
Labels
A-Federation A-Messages-Endpoint /messages client API endpoint (`RoomMessageListRestServlet`) (which also triggers /backfill) S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@erikjohnston
Copy link
Member

erikjohnston commented Sep 2, 2021

This happens when we use a from token that just has a stream ordering part (which clients do the first time they start back paginating in a room after a /sync request), as the following function to convert a stream token to a topological token can lead to incorrect results:

async def get_current_topological_token(self, room_id: str, stream_key: int) -> int:
"""Gets the topological token in a room after or at the given stream
ordering.
Args:
room_id
stream_key
"""
sql = (
"SELECT coalesce(MIN(topological_ordering), 0) FROM events"
" WHERE room_id = ? AND stream_ordering >= ?"
)
row = await self.db_pool.execute(
"get_current_topological_token", None, sql, room_id, stream_key
)
return row[0][0] if row else 0

This is a particularly big problem for rooms like Matrix HQ, where repeated calls to /messages will always trigger a backfill and then time out, even though the server has enough events to return to the client.

The problem manifests when the server receives an old event, so that it ends up with an event with a recent stream ordering but an old depth, causing the above function to return the old depth.

@erikjohnston erikjohnston added S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Sep 2, 2021
@MadLittleMods MadLittleMods added the A-Messages-Endpoint /messages client API endpoint (`RoomMessageListRestServlet`) (which also triggers /backfill) label Jul 19, 2022
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) S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

2 participants