Optimise get rooms for user sync part 2 #11
Conversation
By getting the joined rooms before the current token we avoid any reading history to confirm a user *was* in a room. We can then use any membership change events, which we already fetch during sync, to determine the final list of joined room IDs.
Fizzadar
force-pushed
the
optimise-get-rooms-for-user-sync-part-2
branch
from
a4af801
to
47ebb6d
Compare
| ) | ||
| # User joined a room - we have to then check the room state to ensure we | ||
| # respect any bans if there's a race between the join and ban events. | ||
| if event.membership == Membership.JOIN: |
There was a problem hiding this comment.
I think we need to be careful that we handle a join -> join transition? e.g. a display name update?
I think we could look at the current_state_deltas table to get a more accurate sense of how the state has changed?
There was a problem hiding this comment.
Actually, no we don't need to worry about join -> join transitions. I was thinking of a different un-applicable scenario.
current_state_deltas probably still the right table?
There was a problem hiding this comment.
Was just trying to figure this out but I don't think current_state_deltas has all the information we need - specifically the membership field in event content means we can't determine whether an event was a join or invite.
Pulling in the events would solve this, but at that point it's essentially the same as the current get_membership_changes_for_user call I think, is there a reason this isn't suitable?
| membership_change_events = [] | ||
| if since_token: | ||
| membership_change_events = await self.store.get_membership_changes_for_user( | ||
| user_id, since_token.room_key, now_token.room_key, self.rooms_to_exclude |
There was a problem hiding this comment.
Isn't since_token significantly older than we need? Can we take the current token before we fetch get_rooms_for_user and then only get the changes between the two?
There was a problem hiding this comment.
Yes much older - if we switch to using deltas as above this would be far more efficient.
In the current state of this PR though it makes sense to call get_membership_changes_for_user between the since_token & now because we later fetch this anyway (twice), so passing it around as part of the SyncResultBuilder removes those duplicate lookups.
|
Merged upstream in direct PR. |
Part 2 of: matrix-org#13787
Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.(run the linters)