Skip to content

ai/live: Another way to improve control channel cleanup. #3502

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

Merged
merged 4 commits into from
Apr 17, 2025

Conversation

j0sh
Copy link
Collaborator

@j0sh j0sh commented Apr 12, 2025

This achieves a similar effect as #3483 except perhaps more localized with fewer ad hoc cleanup calls - we just call cleanupLive once per session (although cleanupLive would perhaps be better named "stopControl" or thereabouts)

This allows us to ensure that orchestrator selection completes before beginning this control channel cleanup process, which mitigates any risk of ingest and control (the two primary input sources to the runner) falling out of sync due to the same cleanup function being called from various places.

Note that the gateway WHIP handler did not have this race condition because cleanup is correctly sequenced : we wait for selection to complete, wait for ingest to complete, then clean up the control channel. This PR basically introduces a similar sequencing.

This achieves a similar effect as #3483
except perhaps more localized with fewer ad hoc cleanup calls.

This allows us to ensure that ingest completes before beginning
this control channel cleanup process, which mitigates any risk
of these falling out of sync due to the same cleanup function
being called from various places.
@j0sh j0sh requested review from victorges, leszko and mjh1 April 12, 2025 00:34
@github-actions github-actions bot added go Pull requests that update Go code AI Issues and PR related to the AI-video branch. labels Apr 12, 2025
Copy link

codecov bot commented Apr 12, 2025

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 30.64248%. Comparing base (055484e) to head (c87d679).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
server/ai_mediaserver.go 0.00000% 6 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##              master       #3502         +/-   ##
===================================================
- Coverage   30.64298%   30.64248%   -0.00050%     
===================================================
  Files            151         151                 
  Lines          45087       45091          +4     
===================================================
+ Hits           13816       13817          +1     
- Misses         30463       30466          +3     
  Partials         808         808                 
Files with missing lines Coverage Δ
server/ai_live_video.go 0.00000% <ø> (ø)
server/ai_mediaserver.go 5.22565% <0.00000%> (-0.03122%) ⬇️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

Files with missing lines Coverage Δ
server/ai_live_video.go 0.00000% <ø> (ø)
server/ai_mediaserver.go 5.22565% <0.00000%> (-0.03122%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@j0sh j0sh enabled auto-merge (squash) April 15, 2025 23:38
@j0sh j0sh disabled auto-merge April 15, 2025 23:49
@j0sh j0sh enabled auto-merge (squash) April 15, 2025 23:50
@j0sh j0sh merged commit 0227428 into master Apr 17, 2025
12 of 16 checks passed
@j0sh j0sh deleted the ja/control-race-2 branch April 17, 2025 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AI Issues and PR related to the AI-video branch. go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants