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

Update remove client in erizoJS #1638

Merged
merged 3 commits into from
Oct 5, 2020

Conversation

lodoyun
Copy link
Contributor

@lodoyun lodoyun commented Sep 30, 2020

Description

This PR changes the way removeClient works in erizoJS.
Before, when a Client left, erizoController would issue the commands to erizoJS to remove all published streams and subscriptions from that particular Client and then it would request the removal of the client itself. This approach lent itself to a number of race conditions.
With this PR, erizoController will only have to request the removal of the Client and only remove locally the data structures associated with that client (published and subscribed streams). ErizoJS will now process this removeClient as a command to remove the subscriptions and the published streams in the same operation.

The new NodeManager makes it easier for erizoJS to find published streams owned by a Client.

  • It needs and includes Unit Tests

Changes in Client or Server public APIs

[] It includes documentation for these changes in /doc.

@lodoyun lodoyun changed the title Update remove client in erizo js Update remove client in erizoJS Sep 30, 2020
Copy link
Contributor

@jcague jcague left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! The only nit-picking comment is about why we name NodeManager instead of PublisherManager, which I think would be less verbose if we had methods like add(), remove(), etc.

@lodoyun lodoyun merged commit 56f494c into lynckia:master Oct 5, 2020
@lodoyun lodoyun deleted the add/updateRemoveClientInErizoJS branch October 5, 2020 14:00
Arri98 pushed a commit to Arri98/licode that referenced this pull request Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants