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

Synapse returned event in backfill that was already returned during sync #7164

Closed
bwindels opened this issue Mar 28, 2020 · 9 comments
Closed
Labels
z-bug (Deprecated Label)

Comments

@bwindels
Copy link
Contributor

bwindels commented Mar 28, 2020

Steps to reproduce

The same event is thus returned to be at 2 locations in the timeline.

The event came from another server, at a time that the federation was lagging considerably.

@bwindels bwindels added the z-bug (Deprecated Label) label Mar 28, 2020
@bwindels bwindels changed the title Synapse returned event in backfill that was already returned during earlier sync Synapse returned event in backfill that was already returned during sync Mar 28, 2020
@bwindels
Copy link
Contributor Author

I don't have the request url for the sync that returned the event initially. I did try to do a new initial sync and the event wasn't there anymore between $1585352285101062xfUvo:matrix.org and $1585352478298tKJwR:lant.uk and appears indeed before $158535013295770ZmONI:matrix.org as the backfill above returns.

Both my riot-web and brawl are showing the event between $1585352285101062xfUvo:matrix.org and $1585352478298tKJwR:lant.uk after incrementally syncing though, hence my suspicion that it was indeed returned during a sync at that location.

So possibly synapse changed it mind about where the event should appear in the timeline after having synced it to some clients?

@richvdh
Copy link
Member

richvdh commented Mar 30, 2020

This is somewhat expected. Sync returns things in the order they arrive at the server; backfill returns them in the order determined by the event graph.

@bwindels
Copy link
Contributor Author

bwindels commented Mar 30, 2020

Hmm, would be good to document this, as it's a pitfall for client implementers. Is this more of a protocol implication or rather a synapse implementation detail?

Also, any idea which location clients should choose for the event? The sync location or the backfill location?

@richvdh
Copy link
Member

richvdh commented Mar 30, 2020

well, good questions. I'm not sure if the spec actually mandates any particular ordering for either /sync or /messages, though maybe it should. The general idea is that, if you're following a room in real-time (ie, /sync), you probably want to see the messages as they arrive at your server, rather than skipping any that arrived late; whereas if you're looking at a historical section of timeline (ie, /messages), you want to see the best representation of the state of the room as others were seeing it at the time.

Obviously though that is a simplification and it's not really the case that one size fits all.

Also, any idea which location clients should choose for the event? The sync location or the backfill location?

It's tempting to say "both", but I don't really know. It's kinda a UX decision imho.

@bwindels
Copy link
Contributor Author

Right, that makes sense. So does that mean clients can assume the same ordering between /messages and /context?

The main pitfall we might want to document though is that the same event can be returned twice by the server. Clients that keep history can't assume when processing a /messages response that they're running into the first event of the adjacent chunk of the timeline when running into an event_id they already know about. Instead they need to look at the first/last event_id in adjacent chunks they expect to run into to properly detect overlap, and ignore any other already known events.

They wouldn't be able to do this when back-filling near events previously received from a /context response though, as they don't know where the chunk from a /context response is situated in the timeline. Hence the above question; can ordering be assumed the same between /messages and /context?

@richvdh
Copy link
Member

richvdh commented Mar 30, 2020

well again, I don't think it's specced, but in practice that's probably a safe assumption to make.

@bwindels
Copy link
Contributor Author

Can close this then as it's mainly a spec documentation issue, if any.

@KitsuneRal
Copy link
Member

KitsuneRal commented Mar 31, 2020

Ftr, the client-server spec says on /context that events_before and events_after should be in (reverse) chronological order, which seems to be a lie. Otherwise, there's a very clear warning in the section on syncing which says exactly what's said above - that the two orders may lead to duplicate events - but does not specify the "right" order.

@richvdh
Copy link
Member

richvdh commented Mar 31, 2020

Ftr, the client-server spec says on /context that events_before and events_after should be in (reverse) chronological order, which seems to be a lie.

istr there's an open issue in matrix-doc about this

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
z-bug (Deprecated Label)
Projects
None yet
Development

No branches or pull requests

3 participants