-
Notifications
You must be signed in to change notification settings - Fork 178
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
add Transcoding session ended log message #2685
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2685 +/- ##
===================================================
+ Coverage 56.33537% 56.34518% +0.00981%
===================================================
Files 88 88
Lines 19107 19109 +2
===================================================
+ Hits 10764 10767 +3
+ Misses 7758 7757 -1
Partials 585 585
Continue to review full report at Codecov.
|
@@ -36,7 +36,7 @@ import ( | |||
|
|||
const maxSegmentChannels = 4 | |||
|
|||
var transcodeLoopTimeout = 1 * time.Minute | |||
var transcodeLoopTimeout = 70 * time.Second |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the idea here that we were timing out at the same time as the broadcaster does and so timed out before we had a chance for the B to end the session?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's it yep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a comment that this timeout is set to be greater than B timeout? I'm not sure if we want to have it as tight as possible, +10 sec probably should work fine.
core/orchestrator.go
Outdated
@@ -991,7 +991,9 @@ func (rtm *RemoteTranscoderManager) selectTranscoder(sessionId string, caps *Cap | |||
|
|||
// ends transcoding session and releases resources | |||
func (node *LivepeerNode) EndTranscodingSession(sessionId string) { | |||
node.endTranscodingSession(sessionId, context.TODO()) | |||
logCtx := context.TODO() | |||
clog.V(common.DEBUG).Infof(logCtx, "Transcoding session ended for sessionID=%v", sessionId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe ended by the Broadcaster
to make it explicit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after adding to CHANGELOG_PENDING.md.
What does this pull request do? Explain your changes. (required)
Adding a log line to make it clear on the O end when B ends a session
Specific updates (required)
How did you test each of these updates (required)
I ran B/O/T locally, pushed an rtmp stream, waited 15 seconds or so then stopped the rtmp input, observed the new log message as shown below:
Does this pull request close any open issues?
Fixes #2641
Checklist:
make
runs successfully./test.sh
pass