Closed gfodor closed 4 years ago
Seems to work fine in load tests
I think this patch is not right. The problem is that join_room
and leave_room
sound very symmetrical, but they are not used symmetrically in the SFU. Here's how things work:
switchboard.join_room
is called on their session. This updates occupants
. This is misleading, because occupants
claims it is "Connections which have joined the room, per room", but actually it's only publisher connections.remove_session
is called on it, which calls leave_room
on it for the room it was in, if any. This removes that particular session from occupants
, if it exists. This is also misleading, since it's a no-op for subscriber connections -- I bet I forgot the truth when I wrote it.This patch's change to leave_room
is wrong, because it removes the entry in users_to_room
regardless of whether the call to leave_room
was a subscriber or publisher connection. So the entry in users_to_room
for a user will disappear the first time anyone that user is subscribed to leaves.
Refactors
get_publisher
to use a new hashmap to reduce the need to walk all sessions when fetching publishers. Instead, we now only walk the sessions in the first room the user joined.get_sessions
could also be refactored but proved challenging for me.Also it seems that
get_publisher
assumes a user is only publishing in one room (ie, has one subscriber offer across all of their joined rooms), butget_sessions
assumes a user can be joined in several rooms. So this PR may be a bad idea, since the new hashmap now assumes a user is going to publish from the first room they join. I wasn't sure of the original assumptions being made in switchboard - in practice users only join one room in hubs for both publishing and subscribing. This PR assumes that the first room a user joins is the one they will be publishing in - if that isn't the case (ie they join two rooms, and publish in the second one), the hashmap will register them in the wrong room and their publisher will not be discoverable. However I believe currently the SFU requires publishing in all joined rooms for other reasons, so maybe this is a moot point. If not a better but more complex approach would be to have the hashmap be registered into if/when the user begins publishing.