core: reuse remote transcoder for stream

[reubenr0d]

What does this pull request do? Explain your changes. (required)

Specific updates (required)

• Adds map to keep track of transcoders assigned to sessions
• Updated selectTranscoder to return and set transcoders to sessions
• Removed transcoder from map in case of session end/transcoder errors

How did you test each of these updates (required)

On local offchain setup:

• Setup 1 O, 2 T, 1 B
• Start stream and observed that only one transcoder is processing stream
• Killed that transcoder and stream moves to other transcoder
• Created Unit Tests

Does this pull request close any open issues?

Fixes #1842
Fixes #1712
Fixes #1273

Checklist:

• Read the contribution guide
• make runs successfully
• All tests in ./test.sh pass
• README and other documentation updated
• Pending changelog updated

[kyriediculous]

We also want to remove the currentTranscoder from the streamSessions map when it returns an error here

go-livepeer/core/orchestrator.go

Line 906 in 7a0fd1f

res, err := currentTranscoder.Transcode(md)

[kyriediculous]

We also want to remove the currentTranscoder from the streamSessions map when the connection closes like we do here

go-livepeer/core/orchestrator.go

Line 840 in 7a0fd1f

delete(rtm.liveTranscoders, transcoder.stream)

[kyriediculous]

Looks good ! Just two very small comments.

Feel free to rebase and flatten the commit history once you've addressed those.

Let's use a commit message that indicates which packages were altered and also use that in the title of the PR

e.g. "core: re-use remote transcoder for stream"

[reubenr0d]

I've pushed the changes to the same commit(will use !fixup henceforth), have a look. The Ts are getting removed from the list properly now, but don't seem to adding back, if there is no error. This is the assertion in question. Shouldn't it be 2 after the T succeeds? I'm not quite sure how would it get selected, if the T was not in the list in the first place. Any advice?

[kyriediculous]

I've pushed the changes to the same commit(will use !fixup henceforth), have a look. The Ts are getting removed from the list properly now, but don't seem to adding back, if there is no error. This is the assertion in question. Shouldn't it be 2 after the T succeeds? I'm not quite sure how would it get selected, if the T was not in the list in the first place. Any advice?

@reubenr0d Transcoder 2 gets removed here: https://github.com/reubenr0d/go-livepeer/blob/59b545b57a94f350879307da304a838e99d2c390/core/orch_test.go#L343

[reubenr0d]

Transcoder 2 gets removed here

Right, the comment "assert transcoder gets added back to remoteTranscoders if no transcoding error" threw me off, if it's not supposed to get added back, shall I just remove the last set of assertions after that comment?

[kyriediculous]

@yondonfu Did you have any other remarks regarding the changes you requested ?

[yondonfu]

See #1864 (comment)

[yondonfu]

LGTM!