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

Send a Segment Upload Multiplier of 3 to local broadcaster #172

Merged
merged 1 commit into from Nov 11, 2022

Conversation

thomshutt
Copy link
Contributor

@thomshutt thomshutt commented Nov 11, 2022

This is to be more lenient than the Live workflow when waiting for transcoders.

Broadcaster change in livepeer/go-livepeer#2650

@codecov
Copy link

codecov bot commented Nov 11, 2022

Codecov Report

Merging #172 (2641727) into main (182412c) will decrease coverage by 0.05996%.
The diff coverage is 0.00000%.

Impacted file tree graph

@@                 Coverage Diff                 @@
##                main        #172         +/-   ##
===================================================
- Coverage   47.03255%   46.97259%   -0.05996%     
===================================================
  Files             23          23                 
  Lines           1567        1569          +2     
===================================================
  Hits             737         737                 
- Misses           736         738          +2     
  Partials          94          94                 
Impacted Files Coverage Δ
clients/broadcaster.go 0.00000% <ø> (ø)
clients/broadcaster_local.go 0.00000% <0.00000%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 182412c...2641727. Read the comment docs.

Impacted Files Coverage Δ
clients/broadcaster.go 0.00000% <ø> (ø)
clients/broadcaster_local.go 0.00000% <0.00000%> (ø)

@thomshutt thomshutt merged commit 91f6d2b into main Nov 11, 2022
@thomshutt thomshutt deleted the transcoder-multiplier branch November 11, 2022 21:25
@@ -30,7 +30,9 @@ func NewLocalBroadcasterClient(broadcasterURL string) (LocalBroadcasterClient, e
}

func (c LocalBroadcasterClient) TranscodeSegment(segment io.Reader, sequenceNumber int64, profiles []EncodedProfile, durationMillis int64, manifestID string) (TranscodeResult, error) {
conf := LivepeerTranscodeConfiguration{}
conf := LivepeerTranscodeConfiguration{
SegUploadTimeoutMultiplier: 3,
Copy link
Member

Choose a reason for hiding this comment

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

Just noting that this means that the timeout for upload from B -> O will always be 3x the seg duration and the overall HTTP timeout between B and O for a single segment (including upload + transcode response) is max(4 x seg duration, 8 seconds).

So, given seg duration = N, we have the upload timeout = 3N and the HTTP timeout = 4N.

In the worst case, where the upload takes 3N, that leaves N for the transcode response (i.e. O completes transcode and returns the URLs for the renditions to B) before we hit the HTTP timeout.

I'm not sure if that is the right ratio between timeouts, but this is probably fine for now as we can tweak as is needed

References:

https://github.com/livepeer/go-livepeer/blob/master/server/segment_rpc.go#L473
https://github.com/livepeer/go-livepeer/blob/2f306f6afdb21535612bb82867c5417c4643fe94/common/util.go#L38

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, good point @yondonfu - my thinking was that if this somewhat improves things or at least leads to them timing out later in the request then we can look at multiplying the overall timeout as well

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.

None yet

2 participants