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

api/stream: Improve reliability of /setactive API #992

Merged
merged 11 commits into from
Apr 7, 2022

Conversation

victorges
Copy link
Member

What does this pull request do? Explain your changes. (required)
This is to do a couple improvements in the /setactive API, specifically made for improving
reliability of it. This is especially important for our stream nuking APIs, since for those we need
a reliable source of "where is the stream?". In the worst case we can start fetching this info
from the Stream Health API, but I believe a very simple change to the /setactive API should
already make this a lot more reliable, especially against malicious actors like our soccer
streamers who have learned how to explore the bugs in that API.

Apart from that very simple change, inplemented here 8045d8d, also made a bunch of
improvements to the API performance, since I noticed from our metrics that it's already
running dangerously close to the mist-api-connector timeout a lot of the times: [grafana]

Also did a small fix in the /user/suspended API here, just to avoid re-emailing the same user
that has already been suspended.

Specific updates (required)

  • Avoid responding 403 to a /setactive: false request for a suspended stream (only when active=true)
  • Ignore /setactive calls from sessions that are not the last one
  • Optimize API handler execution by moving Webhooks and aux DB updates to background
  • Only send account suspended emails to not already suspended users

-

  • How did you test each of these updates (required)
    yarn test
    also deployed to staging and checked things still work

Does this pull request close any open issues?
It is related to but does not close https://github.com/livepeer/livepeer-infra/issues/830

I'm hoping that this change will make a full backend calling the kube-api unnecessary.
Our stream termination logic from the API should work perfectly.

Checklist:

  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.

@victorges victorges requested a review from a team as a code owner April 6, 2022 22:13
@vercel
Copy link

vercel bot commented Apr 6, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/livepeer/livepeer-com/B4EtVyCzGz7oPdYJaQkmB6Zf8AjB
✅ Preview: https://livepeer-com-git-vg-fixstream-user-suspending-livepeer.vercel.app

Copy link
Member

@hjpotter92 hjpotter92 left a comment

Choose a reason for hiding this comment

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

lgtm!

@victorges victorges merged commit 16174b7 into master Apr 7, 2022
@victorges victorges deleted the vg/fix/stream-user-suspending branch April 7, 2022 15:08
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