Skip to content
This repository was archived by the owner on Sep 24, 2025. It is now read-only.

double cleanup speed#1

Draft
mikep-vistapath wants to merge 1 commit intoros2from
ros2-dev
Draft

double cleanup speed#1
mikep-vistapath wants to merge 1 commit intoros2from
ros2-dev

Conversation

@mikep-vistapath
Copy link
Copy Markdown

No description provided.

@mikep-vistapath mikep-vistapath requested a review from jguggenh June 9, 2020 20:12
Copy link
Copy Markdown

@jguggenh jguggenh left a comment

Choose a reason for hiding this comment

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

interesting. can you test this further? if the cleanup speed was supposed to be 500ms, there is no way it was functioning. id guess that however its determining "inactive" streams is not working, or at least wasn't working a month ago.

to test, just start everything up and then run a case with a ton of cassettes. for example, run two cassettes, save both, run two more, save both, run two more etc.

i think the reason this is working better is not that this change solves the web video server problem but rather all the work cornel has done to limit the streams he is accessing.

@jguggenh
Copy link
Copy Markdown

jguggenh commented Jun 9, 2020

however, if we are now moving over to firefox, i think the number of streams we can simultaneously host increases (6 -> 14? something like that). in that case, maybe its a moot point for now whether the web video server is successfully closing inactive streams.

@mikep-vistapath
Copy link
Copy Markdown
Author

Yeah, with Firefox we may not need any of our changes. After banging on this for a while on my device, it seemed like streams got picked up faster by doubling the cleanup frequency. But I rarely get infinite spinners so I'm not 100% certain that this would fix things for others.

@mikep-vistapath mikep-vistapath marked this pull request as draft June 9, 2020 20:58
@jguggenh jguggenh marked this pull request as ready for review June 9, 2020 21:08
@jguggenh jguggenh marked this pull request as draft June 9, 2020 21:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants