Backfilling on `/messages` causes odd behavior

puckipedia commented 2 years ago

Background information

Dendrite version or git SHA: 633ca06eb9f7652a6c4be04b3ffe8950419a8ee3 (0.8.3rc1)
Monolith or Polylith?: Monolith
SQLite3 or Postgres?: Postgres 14
Running in Docker?: No
go version: 1.16.7
Client used (if applicable): Fractal, but replicates with curl

Description

What is the problem: When using a filter on the /messages endpoint that doesn't match all messages (e.g. ?from=...&dir=b&limit=40&filter={"types":["nope"]}) Dendrite skips over a large chunk of events, as it assumes that retrieving any amount of messages less than the limit is incorrect, and then adds the wrongly backfilled events, unfiltered, into the response.
Who is affected: Uncertain.
How is this bug manifesting: Scrolling upwards in rooms works inconsistently (Fractal has large jumps of >1 year in event age)
When did this first appear: Appeared in the first room I joined Dendrite into, a few days ago, on 0.8.2

Steps to reproduce

Join Dendrite to a room with (multiple?) unfetchable backwards extremities (in my case, this was #flatpak:matrix.org, but it presumably applies to any rooms with non-fully-readable room history)
Send a request for message scrollback from a range that has been backfilled previously, with a filter that doesn't apply to all messages
Notice that Dendrite is sending a backfill request that is unneeded when no filter is applied, and has a completely incorrect end topological pointer.
Notice that the response contains the backfilled events, even though they should not show up in the room's history at this point in the topology

ShadowJonathan commented 2 years ago

Does this only appear when a json filter is attached, or does this also happen without any filter?

puckipedia commented 2 years ago

I believe this bug can apply in both cases, but they do so somewhat differently, and i'm not sure both fit in this one issue. When a filter is applied, this occurs consistently for me, but I've seen a very similar issue if doing a messages request near a backward extremity, fetching previous messages ends up with a non-sorted (and sometimes containing looping topological tokens) result due to all the backfilled events being added to the messages response, even if those events have the wrong topological depth. If it's useful, I could open a separate issue?

bones-was-here commented 2 years ago

Not sure if related, but in old versions it was often possible to scroll all the way back to the creation of a room in Element Web. That doesn't work in newer versions (at least any of 0.8.x). It will scroll back to a certain point, usually a few days or weeks of messages, depending how busy the room is. Beyond that it will either: keep jumping "randomly" to a newer point in the history, or will simply stop loading with no explanation or loading spinner, or will tell me I can't see messages before that point even when I definitely should have access to them.

S7evinK commented 2 years ago

@puckipedia could you try 0.8.9? https://github.com/matrix-org/dendrite/pull/2548 should fix some backfilling issues with /messages in combination with /context which caused strange behavior (in Element Web at least).

bones-was-here commented 2 years ago

I tested scrolling back in 0.8.9 with firefox autoscroll, needed to do a clear cache and reload first though, which caused a #2503.

It works much better than in previous 0.8.x versions, in some rooms I can reach room creation where I couldn't before.
But the issue where, at a certain point, Element will jump to a newer point, can still happen. I think it gets further back before it does that but it still isn't possible to reach room creation in rooms where that happens.

bones-was-here commented 2 years ago

It seems that in 0.9.5 the jumping issue in element web history is fixed, I can scroll back to the room creation in rooms where it wasn't possible before.

matrix-org / dendrite