LL members can get out of sync with server on limited sync response

[ara4n]

I invited uhoreg into a private dev room; he clearly joined, but in practice LL clients show him to still be invited. We see RRs from him though.

[bwindels]

Can't repro in a room on localhost server with 4 joined + 1 invited members. I did notice another bug, where one of the joined members in the room forgot about the invited user. The invite event was in the room timeline, but not present in the room state nor the accumulated sync data. Seems like a Riot bug. Looking into it now, but these two issues couldn't be related you think?

[bwindels]

UPDATE: see comment below first

The problem described above is also hard to reproduce, but I noticed one thing in the sync response after accepting an invite (so when the room appear under "join" for the first time): the state events are not complete. I don't receive the state at the point where I join. Instead I only get what seem to be the 3 first state events (m.room.create, m.room.member for room creator, m.room.power_levels). I would have expected the invite member event for another member to be in there that was invite just before me.
More concretely, the timeline looks like this, and the syncing user is Bruno3:

• Bruno1 joined the room.
• Bruno1 made future room history visible to all room members.
• Bruno1 changed the room name to the happy few.
• Bruno1 invited Bruno2.
• Bruno1 invited Bruno5.
• Bruno1 invited Bruno3.
• Bruno2 joined the room.
• Bruno3 joined the room.

Is this how it is supposed to work @ara4n ?
I tried to reproduce on develop with LL turned off but there state.events was just empty?
I thought you'd get the full room state (apart from joined members with LL turned on) after accepting a room, much like when doing an initial sync for a joined room? Maybe I'm missing something.

[bwindels]

Creating a ticket for the issue described above, as it's probably not related to what happend with uhoreg where join event seems to have been fabricated. #7243

[ara4n]

Right, looks like you might have found another bug with this "sync on joining is wrong".

Separately, I've reproduced the original bug - it looks to be a problem with limited syncs. To reproduce:

• Alice turns on LL
• Alice creates room
• Bob joins room
• Alice pauses her Riot
• Bob invites Charlie
• Charlie joins, and can even say something
• Bob says 30 messages
• Alice unpauses her Riot, and so gets a catchup (limited) /sync
• Alice observes that the /sync has an empty sync block entirely.

This is very sytest-able and just a screwup in the impl, i guess; I didn't explicitly test limited syncs.

[ara4n]

Actually, my issue isn't really synapse's fault. It's correct that the state is missing for the members who joined during the gap, given by definition if they're not in the timeline, and the current contract for LL is that you only get state for members who are in the timeline. Instead, it's an edge case that after a limited (aka gappy) sync, we would have to do a full bg resync of the members list to go and fill in the details of any changes which happened during the gap.

So, to fix this, either we:

• Decide that gappy incremental syncs shouldn't lazy-load members (which is questionable; why are we loading members for things which aren't visible in the timeline?)
• Re-trigger a bg members update after a gappy incremental sync. This has potential performance problems, as gappy syncs happen whenever a client is getting behind (either because of slow performance, slow network, or a hiatus in connectivity - e.g. laptop being slept). Background membership syncs are obviously quite heavy, so it seems like a bit of a pyrrhic victory to swap out an incremental state sync across the gap with a full resync because of this API decision.

I'm not sure what the right soln is, tbh. On one hand, just making synapse include the state during a gappy sync is kinda appealing as it will fix this without changes on the clients, and typically there won't be too much of a delta. On the other hand, most sophisticated clients (e.g. Riot on all platforms) only ever do initial sync at initial launch, and thereafter always do an incremental sync - so if they're offline for weeks and then relaunch, the relaunch could be delayed by needlessly pulling in tonnes of membership events.

Having written it out, i think we should optimise for the smallest possible syncs, given we might have better ways in future for pulling in the membership data selectively (rather than the very blunt hack of currently resyncing the whole membership list in the bg). In other words, responsivity is more important than bandwidth utilisation.

[ara4n]

i've split out the other issue into #7303