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

Mark all input errors from LPMS as non-retriable #1764

Merged
merged 2 commits into from
Feb 16, 2021
Merged

Conversation

jailuthra
Copy link
Contributor

@jailuthra jailuthra commented Feb 16, 2021

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

Mark all input errors from LPMS as non-retriable

Specific updates (required)

  • Update LPMS module
  • Go through all LPMS errors and mark segment as non retriable if it returns an error in the known list.

How did you test each of these updates (required)

Manually tested with a segment with invalid pixel format https://cdn.discordapp.com/attachments/426409080551374849/809890640527097862/1.ts - got the correct behaviour -

I0216 20:10:17.532951  368785 segment_rpc.go:465] Uploaded segment nonce=6549207908557319073 manifestID=test sessionID=ab710a7e seqNo=2 orch=https://0.0.0.0:8935 dur=2.848236ms
E0216 20:10:17.539248  368785 segment_rpc.go:497] Transcode failed for segment nonce=6549207908557319073 manifestID=test sessionID=ab710a7e seqNo=2 orch=https://0.0.0.0:8935 err=Unsupported input pixel format
W0216 20:10:17.539286  368785 broadcast.go:440] Not retrying current segment nonce=6549207908557319073 seqNo=2 due to non-retryable error err=Unsupported input pixel format
E0216 20:10:17.539331  368785 mediaserver.go:877] http push error processing segment url=http://127.0.0.1:8936/live/test/2.ts manifestID=test err=Unsupported input pixel format

Does this pull request close any open issues?

Fixes #1756

Checklist:

  • README and other documentation updated
  • Node runs in OSX and devenv
  • All tests in ./test.sh pass

Copy link
Contributor

@darkdarkdragon darkdarkdragon left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

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

LGTM after merging livepeer/lpms#224 Not sure if go mod needs to be updated in case the commit hash is different on master in LPMS?

@jailuthra
Copy link
Contributor Author

LGTM after merging livepeer/lpms#224 Not sure if go mod needs to be updated in case the commit hash is different on master in LPMS?

Thx! Did a manual rebase+push in LPMS to retain the same commit ID in master.

@jailuthra jailuthra merged commit e76d730 into master Feb 16, 2021
@yondonfu yondonfu deleted the jai/nonretriable branch February 16, 2021 17:07
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.

Mark additional LPMS transcode errors as non-retryable on B
3 participants