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

trickle: Sweep idle channels on the server. #3296

Merged
merged 6 commits into from
Dec 9, 2024
Merged

trickle: Sweep idle channels on the server. #3296

merged 6 commits into from
Dec 9, 2024

Conversation

j0sh
Copy link
Collaborator

@j0sh j0sh commented Dec 5, 2024

Eventually cleans up anything on the trickle server side in case we miss a cleanup somewhere.

This does imply that channels need to be active regularly, so for naturally intermittent channels like control or events we will have to send a keepalive via clients periodically. (keepalives will also be useful for keeping client connections alive.)

Default 5-minute idle timeout, with a 1-minute sweep interval (both configurable). This basically requires us to explicitly start the trickle server (in order to start sweeping), and the Start() function returns a cancel-type function in order to stop things, a'la context cancel.

Also adds an explicit "channel create" endpoint to the trickle server (empty POST to /channel-name without an associated sequence number). Not used right now but it has come in handy while testing elsewhere, eg in python.

Also includes this change to make the startAIServer take a *lphttp so we aren't copying the entire struct every time we pass this in, and can make changes to its fields (eg, assigning the trickle server)

@j0sh j0sh requested review from victorges, leszko and mjh1 December 5, 2024 01:36
@github-actions github-actions bot added the go Pull requests that update Go code label Dec 5, 2024
j0sh added 2 commits December 5, 2024 06:09
This is consistent with the usage of lphttp elsewhere and allows
us to actually make changes to its fields (eg, trickle server)
Copy link

codecov bot commented Dec 5, 2024

Codecov Report

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

Project coverage is 34.14555%. Comparing base (51fd713) to head (8b71572).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
trickle/trickle_server.go 0.00000% 57 Missing ⚠️
server/ai_http.go 0.00000% 10 Missing ⚠️
server/rpc.go 0.00000% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##              master       #3296         +/-   ##
===================================================
- Coverage   34.19096%   34.14555%   -0.04541%     
===================================================
  Files            139         139                 
  Lines          36846       36895         +49     
===================================================
  Hits           12598       12598                 
- Misses         23529       23578         +49     
  Partials         719         719                 
Files with missing lines Coverage Δ
server/rpc.go 67.26190% <0.00000%> (-0.60597%) ⬇️
server/ai_http.go 10.25641% <0.00000%> (ø)
trickle/trickle_server.go 0.00000% <0.00000%> (ø)

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 51fd713...8b71572. Read the comment docs.

Files with missing lines Coverage Δ
server/rpc.go 67.26190% <0.00000%> (-0.60597%) ⬇️
server/ai_http.go 10.25641% <0.00000%> (ø)
trickle/trickle_server.go 0.00000% <0.00000%> (ø)

@j0sh
Copy link
Collaborator Author

j0sh commented Dec 5, 2024

Holding off on merging this until we have keepalives to ensure control API liveness since messages there are intermittent and we don't want those to be swept up

@j0sh
Copy link
Collaborator Author

j0sh commented Dec 5, 2024

Once #3299 and livepeer/ai-worker#313 are in, I'll merge this one

@j0sh j0sh merged commit bc85404 into master Dec 9, 2024
18 checks passed
@j0sh j0sh deleted the ja/sweep-idle branch December 9, 2024 21:54
j0sh added a commit that referenced this pull request Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants