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

Faster joins: non-lazy_load_members /syncs should not block #12989

Closed
1 task done
Tracked by #14030
squahtx opened this issue Jun 8, 2022 · 6 comments · Fixed by #14870
Closed
1 task done
Tracked by #14030

Faster joins: non-lazy_load_members /syncs should not block #12989

squahtx opened this issue Jun 8, 2022 · 6 comments · Fixed by #14870
Assignees
Labels
A-Federated-Join joins over federation generally suck A-Sync defects related to /sync T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@squahtx
Copy link
Contributor

squahtx commented Jun 8, 2022

Instead they should pretend that the room has not been joined yet.

/syncs with lazy_load_members turned on should continue to return the room.


Blocked by:

@squahtx squahtx added A-Federated-Join joins over federation generally suck T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Jun 8, 2022
@richvdh
Copy link
Member

richvdh commented Jun 8, 2022

One particular challenge here is being able to figure out which rooms have completed their state resyncs since a given /sync token.

@richvdh
Copy link
Member

richvdh commented Jun 8, 2022

For the record (contrary to what I may have said earlier): this is a fair way down the order of things that I agreed with dan, so maybe not the best battlefront to open right now. Having non-lazy_load_members /syncs block while a faster join is in progress is fine for now.

@erikjohnston
Copy link
Member

One particular challenge here is being able to figure out which rooms have completed their state resyncs since a given /sync token.

I guess the simplest thing is to add a table that stores the stream ordering of when a room completed joining? Then the sync code can check that table for new rooms.

Note that we already have features for excluding rooms from /sync, so we have prior art there.

@erikjohnston
Copy link
Member

I guess the simplest thing is to add a table that stores the stream ordering of when a room completed joining? Then the sync code can check that table for new rooms.

I guess the question is more, which stream should we use? We could reuse the events stream, but that would add another complexity to an already complex stream. We could make a new one, but that would mean adding another field to the sync tokens.

@richvdh
Copy link
Member

richvdh commented Oct 10, 2022

I guess the question is more, which stream should we use? We could reuse the events stream, but that would add another complexity to an already complex stream. We could make a new one, but that would mean adding another field to the sync tokens.

Yeah. I'm unenthusiastic about complexifying the events stream any further, even if that does mean adding yet another field to the sync tokens. (Sliding sync will mean that clients no longer need to worry about raw sync tokens, right??)

@reivilibre
Copy link
Contributor

reivilibre commented Nov 11, 2022

Just a note of an edge case before I forget: if you see a room become un-partial-stated in a sync's window, it's important to still check that it isn't partial-stated at the end. The room could have been left and rejoined in the meantime since becoming originally un-partial-stated.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Federated-Join joins over federation generally suck A-Sync defects related to /sync T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
6 participants