Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CS API refers to undefined terms stream ordering and topological ordering #1334

Open
DMRobertson opened this issue Nov 14, 2022 · 2 comments
Labels
clarification An area where the expected behaviour is understood, but the spec could do with being more explicit

Comments

@DMRobertson
Copy link
Contributor

Link to problem area:

Issue

From https://spec.matrix.org/v1.4/client-server-api/#get_matrixclientv3sync definition of m.heroes under "Room Summary" defining m.heroes:

The users which can be used to generate a room name if the room does not have one. Required if the room’s m.room.name or m.room.canonical_alias state events are unset or empty.

This should be the first 5 members of the room, ordered by stream ordering, which are joined or invited. The list must never include the client’s own user ID. When no joined or invited members are available, this should consist of the banned and left users. More than 5 members may be provided, however less than 5 should only be provided when there are less than 5 members to represent.

(emphasis mine.)

It is not clear what "stream ordering" means. (AFAIK is a concept that Synapse defines for its own use?)

https://spec.matrix.org/v1.4/appendices/ makes no mention either.

Similarly much of the relations work speaks of "topological ordering" that isn't defined either.

Related:

@DMRobertson DMRobertson added the clarification An area where the expected behaviour is understood, but the spec could do with being more explicit label Nov 14, 2022
@DMRobertson
Copy link
Contributor Author

This should be the first 5 members of the room, ordered by stream ordering, which are joined or invited.

Judging from https://github.com/matrix-org/synapse/blob/66a785733458d0b5801097caff53624e202a91b4/synapse/handlers/sync.py#L831, it seems Synapse doesn't do the ordering properly.

MadLittleMods added a commit to element-hq/synapse that referenced this issue Jul 17, 2024
The spec specifically mentions `stream_ordering` but that's a Synapse specific concept. In any case, the essence of the spec is basically the first 5 members of the room which `stream_ordering` accomplishes.

Split off from #17419 (comment)

## Spec compliance

> This should be the first 5 members of the room, **ordered by stream ordering**, which are joined or invited. The list must never include the client’s own user ID. When no joined or invited members are available, this should consist of the banned and left users.
>
> *-- https://spec.matrix.org/v1.10/client-server-api/#_matrixclientv3sync_roomsummary*

Related to matrix-org/matrix-spec#1334
@MadLittleMods
Copy link
Contributor

MadLittleMods commented Aug 5, 2024

After crafting #1917, I think I found how we can derive the following definitions from this existing piece of spec:

Events are ordered in this API according to the arrival time of the event on the homeserver. This can conflict with other APIs which order events based on their partial ordering in the event graph. This can result in duplicate events being received (once per distinct API called). Clients SHOULD de-duplicate events based on the event ID when this happens.

Therefore:

  • stream_ordering: Ordered by the "arrival time of the event on the homeserver"
  • topological_ordering: Ordered by the "partial ordering in the event graph". This corresponds to the baked in depth field but ideally homeservers would use some graph linearization strategy. Perhaps some online topological ordering (Katriel–Bodlaender algorithm) where depth/topological_ordering is dynamically updated whenever new events are inserted into the DAG.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification An area where the expected behaviour is understood, but the spec could do with being more explicit
Projects
None yet
Development

No branches or pull requests

2 participants