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

Make server.js delete old peer connections on the same IP #118

Merged
merged 4 commits into from
Jan 16, 2024

Conversation

amalnanavati
Copy link
Contributor

@amalnanavati amalnanavati commented Jan 15, 2024

Before this PR, server.js created a new peer connection for every new registered publisher/subscriber. This was intentional, to allow multiple devices to simultaneously register to the robot's video stream (e.g., to test latency). However, this also means that throughout the course of the meal, more and more peer connections are created, which hugely increases the CPU utilization of server.js (we saw it go up to 500%).

This PR addresses that by only allowing one publisher and subscriber per topic at a time. (Note: in theory this should allow for a different subscriber per IP, but in practice all the IPs end up being the same -- unclear why -- resulting in this only allowing one publisher/subscriber per topic at once.)

Testing: On lovelace:

  • Start server.js
  • Start/restart start_robot_browser.js several times. Verify that CPU utilization for server.js stays roughly the same.
  • Load the web app on another device. Load and reload a video stream (e.g., refreshing the bite selection page). Verify that CPU utilization for server.js stays roughly the same.
  • Open a new tab on that device. Verify that the old tab stops updating but the new tab keeps updating.
  • [FAIL] Load the web app on a different device. Verify that both media streams are updating.
    • This test does not hold up, see above for why.

@amalnanavati amalnanavati merged commit 961bce4 into main Jan 16, 2024
4 checks passed
@amalnanavati amalnanavati deleted the amaln/server_delete_peers branch January 16, 2024 00:06
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.

1 participant