matrix-org / matrix-org/waterfall

Rethink (refactor) `participant` package

Open
#128 0 comments 0 reactions 0 assignees View on GitHub
T-Task X-Refactor
Dominant language
Go
Stars
105
Forks
5
PR merge metrics
No merged PRs in 30d

Description

The current `participant` package is not particularly useful, since `Participant` only provides public fields and does not do any encapsulation. The only reason it was moved there is the `Tracker` implementation. `Tracker` was created to ensure that 3 entities: subscriptions, published tracks, and participants are in sync.

However, after tackling https://github.com/matrix-org/waterfall/issues/120 and https://github.com/matrix-org/waterfall/issues/117, the published track along with its subscribers is extracted into its own package.

This means that the only thing that `Tracker` synchronized are 2 fields: participants and published tracks. It's easy to see that published tracks actually belong to the participant structure (and it was implemented like this at the beginning), however, they were split so that we could do a fast look-up of the published tracks by their IDs, which was required to ensure:

- fast subscribe/unsubscribe: when someone sends us a subscribe/unsubscribe command, they specify the track IDs, but not participant IDs, which means that if we stored published tracks inside participants, then we would need to first go over all participants and then over each participant's tracks to find the published track.
- fast selection of subscriptions when packets arrive to the new conference (this was due to participants sending their packets directly to the conference, this changes with aforementioned issues)

So once it's done, it would be good to review the `participant` and refactor it, so that:
- `Participant` contains `publishedTracks` inside it (it simplifies the clean-ups and managing tracks in general).
- `Participant` has all its fields as private providing a simple interface to interact with it that would ensure that all invariants that the participant has, are intact.
- Get rid of a `Tracker` (optional), and instead just store an additional map that contains `trackID : ownerID` relation. That way we just need to maintain the synchronous state of these two which would allow us to find the required tracks faster (make sure that it does not bring any performance downsides).
- Then part of the logic would be moved back to the `Conference`. This may not necessarily be bad since currently, the responsibility of the `Conference` overlaps with the `Tracker` a bit.

Contributor guide

Open the contributing guide

Research direction

Start by reviewing the participant package, Participant, Tracker, and Conference, then read issues #120 and #117 to understand the completed restructuring. Determine how published tracks and the trackID-to-ownerID relation should be maintained while preserving fast lookups and participant invariants. Done means the refactor has a clear ownership model, private Participant fields, and no unnecessary Tracker responsibility.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
audio-video-rtc
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.