Skip to content
This repository has been archived by the owner on Dec 2, 2023. It is now read-only.

fix sending repetitive and inactive peers #592

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aminyazdanpanah
Copy link

Description

This pull request fixes sending repetitive and inactive peers to the new peer.
https://github.com/pion/ion/blob/118b8c419822088b948d98466a9e173b7fe50317/apps/room/server/room_signal.go#L207-L242

These above lines of code send every peer that was saved in the Redis to the new peer. For example, if a user attempted to join the room twice, it will record two peers in the Redis (assuming UIDs are different due to security issues). Besides, if the user leaves the room, it won't delete the record in Redis.

Instead, I removed these lines and created a new function to do this job. In this function, it will send all peers that are saved in addpeer method.

This function sends all active peers to the new peer

https://github.com/aminyazdanpanah/ion/blob/f0677770b63f4fbd6decff92b4db3b62c79740ba/apps/room/server/room_signal.go#L277-299

Hope this clarifies the issue.

@adwpc
Copy link
Collaborator

adwpc commented Jan 24, 2022

@aminyazdanpanah
Thanks for this PR!!
ION is a distributed system(strong consistency), we store track infos in redis so that other singal-Node can use the same infos.
I think we should fix redis issue

@aminyazdanpanah
Copy link
Author

Hi @adwpc

Thanks for reviewing!

Actually, this PR does not have any impact on storing track Infos in Redis. the lines of code that I removed just fetch other peers from Redis and send them to the new peer.

Btw, thank you for spending time on this PR

@adwpc
Copy link
Collaborator

adwpc commented Mar 2, 2022

@aminyazdanpanah Thanks!
we should delete the record in redis if the user leaves the room

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants