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

Sliding Sync: when using $LAZY always send down all members for non-gappy syncs #17929

Open
Tracked by #2580
erikjohnston opened this issue Nov 13, 2024 · 4 comments · May be fixed by #17947
Open
Tracked by #2580

Sliding Sync: when using $LAZY always send down all members for non-gappy syncs #17929

erikjohnston opened this issue Nov 13, 2024 · 4 comments · May be fixed by #17947
Assignees
Labels

Comments

@erikjohnston
Copy link
Member

Currently, $LAZY only sends down membership events for senders of events in the timeline (if the membership hasn't previously been sent down). In particular, that means if a user's state changes and it is not in the timeline, then the client won't know the membership state has changed / been invalidated.

To make things easier for clients, lets change $LAZY to also send down all membership changes in non-gappy syncs (i.e. when limited: false). This allows clients to cache the membership list for as long as it doesn't get a gappy sync, but still ensures for large gaps the server doesn't need to send down all membership changes.

@ganfra
Copy link
Member

ganfra commented Nov 15, 2024

Probably related issue on EXA : element-hq/element-x-android#3790

@MadLittleMods
Copy link
Contributor

MadLittleMods commented Nov 19, 2024

if a user's state changes and it is not in the timeline

@erikjohnston When does this happen? If someone changes their display name for example, the membership state event will show up in the timeline. And if the membership change happened outside of the range of the timeline_limit, then we are talking about a limited: true sync where this new logic doesn't apply anyway.

Is this targeting a scenario where some state comes over federation from some time ago which updates the current state but isn't in the timeline? (not sure this is even a thing because it will still get a recent stream_ordering which should make it come down /sync)

Probably related issue on EXA : element-hq/element-x-android#3790

@ganfra What's your exact theory for how this problem contributes to that issue?

@jmartinesp
Copy link
Member

jmartinesp commented Nov 21, 2024

To clarify this a bit: the original reported issue was:

  1. There is a request to join (a knock membership event) from Alice. This appears in the required_state and is taken into account by the client, and Alice's membership is updated.
  2. Bob accepts/declines the request to join (so he invites/kicks Alice from the room). This membership change appears in the timeline, but not in the required_state, probably because the sender of those events (Bob) is not the member whose membership changed (Alice).
  3. Some other client of Bob doesn't receive any new state event in required_state, so it still thinks Alice's membership is knock, instead of invite/leave.

Technically, this could be also solved by processing the state events in the timeline, but since those are not guaranteed to be up to date it could easily lead to inconsistencies in the clients.

@MadLittleMods
Copy link
Contributor

@jmartinesp Perfect description! That makes a lot more sense 👌

So basically, we just need to fill in this FIXME

# Everyone in the timeline is relevant
#
# FIXME: We probably also care about invite, ban, kick, targets, etc
# but the spec only mentions "senders".

I've made the changes with some tests in #17947 🚀

Technically, this could be also solved by processing the state events in the timeline, but since those are not guaranteed to be up to date it could easily lead to inconsistencies in the clients.

Correct choice 👍 Only rely on what the homeserver is telling you through required_state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants