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

Prevent race in participant update. #308

Merged
merged 1 commit into from
Sep 9, 2023
Merged

Prevent race in participant update. #308

merged 1 commit into from
Sep 9, 2023

Conversation

boks1971
Copy link
Contributor

@boks1971 boks1971 commented Sep 9, 2023

In load test, participant updates were racing causing subscription failures. The sequence is

  • Join response received
  • Read worker started
  • Add remote participants from join response
  • But, before that could happen, because the read worker is active, there was a participant update which contained track information. That path added the remote participant before join response handler could add it.
  • Eventually join response path comes around and adds the remote participant, but that version of remote participant info does not have the tracks yet. And it ends up overwriting the participant info. And it also drops tracks that are not there in the update.
  • As the order got reversed, tracks added by participant update path gets wiped out by join response path.
  • And subscription fails.

Adding a simple fix to reject participant info updates that are older version.

It will probably be better to process join response fully? Maybe, start the read worker only after processing join response?

NOTE: When the remote participant gets created via the participant update path, the local participant itself is not set up fully.

In load test, participant updates were racing causing subscription
failures. The sequence is
- Join response received
- Read worker started
- Add remote participants from join response
- But, before that could happen, because the read worker is active,
  there was a participant update which contained track information. That
  path added the remote participant before join response handler
  could add it.
- Eventually join response path comes around and adds the remote
  participant, but that version of remote participant info does not have
  the tracks yet. And it ends up overwriting the participant info. And
  it also drops tracks that are not there in the update.
- As the order got reversed, tracks added by participant update path
  gets wiped out by join response path.
- And subscription fails.

Adding a simple fix to reject participant info updates that are older
version.

It will probably be better to process join response fully?
Maybe, start the read worker only after processing join response?

NOTE: When the remote participant gets created via the participant
update path, the local participant itself is not set up fully.
@boks1971
Copy link
Contributor Author

boks1971 commented Sep 9, 2023

It will probably be better to process join response fully? Maybe, start the read worker only after processing join response?

Irrespective of the above, I think the change in this PR is good as there is another path also where a remote participant is added before ParticipantInfo. That is in handleMediaTrack path. In this case, there are two ParticipantInfos trampling on each other. In the media track case, a minimal ParticipantInfo is synthesised. That will have version 0. So, the version check here should still work.

@boks1971 boks1971 merged commit a30cb3f into main Sep 9, 2023
1 check passed
@boks1971 boks1971 deleted the raja_update_race branch September 9, 2023 17:57
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