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

Test LPMS refactoring on staging #305

Closed
Quintendos opened this issue Dec 14, 2022 · 7 comments
Closed

Test LPMS refactoring on staging #305

Quintendos opened this issue Dec 14, 2022 · 7 comments
Assignees

Comments

@Quintendos
Copy link

[1] livepeer/lpms#347
[2] livepeer/lpms#348

(blocks transcoding work Diederik)

@Quintendos
Copy link
Author

Quintendos commented Dec 14, 2022

@AlexHevc brought up a really good point this morning that Michal's LPMS refactorings [1] [2] still haven't been rolled out and that this should probably be a precondition for the work @roxlu is doing.

The current state is that [1] is merged but behind a feature flag and so we should enable that flag on Staging and test

I think that after that, the steps are:

  1. Gradually turn on the flag for our Prod Os and monitor
  2. Remove the flag and cut a go-livepeer release for community Os
  3. @AlexHevc and @roxlu to analyse [2] and either merge, test and roll out or cherry-pick bits they want to keep

@mjh1
Copy link
Contributor

mjh1 commented Dec 14, 2022

@thomshutt is this the correct place to add this env variable? orchestrator section..? https://github.com/livepeer/livepeer-infra/commit/e411d494bb171cb9d998c4138ac71d40d2dc7338

@thomshutt
Copy link
Contributor

I think so, you should check if we have logging to confirm it's working though (and add it if we don't)

@mjh1
Copy link
Contributor

mjh1 commented Jan 5, 2023

I think so, you should check if we have logging to confirm it's working though (and add it if we don't)

@thomshutt do you think this will be ok: https://github.com/livepeer/lpms/pull/362/files
Obviously not great having a log line for every transcode but the log line would go away eventually when we remove the old implementation. Easiest alternative I can think of is just reading the same environment variable on startup in go-livepeer and logging it there 🤔

@thomshutt
Copy link
Contributor

@mjh1 Yeah, that's fine - I just approved. In the grand scheme of the amount of logging we do, one line per transcode is fine.

@mjh1
Copy link
Contributor

mjh1 commented Jan 10, 2023

This was rolled out to staging today. Initial tests aren't showing any issues 👍

Confirmed with the log line that it's definitely in action:

$ kubectl logs staging-livepeer-orchestrator-1 | grep "LPMS new transcode enabled"
I0110 10:20:41.029146       1 ffmpeg.go:970] LPMS new transcode enabled

@thomshutt
Copy link
Contributor

Holding off until we find the cause of extra frames that @AlexKordic saw being added in some cases

iameli pushed a commit that referenced this issue Feb 7, 2023
Files changed:
M	manifest.yaml

Co-authored-by: livepeer-docker <livepeer-docker@users.noreply.github.com>
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

No branches or pull requests

3 participants