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

Enable using Unix sockets to communicate with containers #48

Merged
merged 8 commits into from
Feb 20, 2025

Conversation

aFuerst
Copy link
Contributor

@aFuerst aFuerst commented Feb 18, 2025

  • Create Unix socket server that runs inside containers.
  • Abstract container communication from container creation.
  • Each container now has a special host folder for its "stuff"
  • Using socket has ~6x reduction (~2-3 ms => 0.5 ms) in E2E platform overhead

How it works:

  • Worker passes a environment var to the container indicating the socket path it must open.
  • That folder is mounted from host container folder to that path
  • Worker listens on that to be created.
  • Uses a 2-message system to invoke.
    • First is a SendMessage with the first a u64 Command::Invoke and the len in bytes of the str arguments
    • Then the arguments are sent and deserialized by the worker
  • Same system of sending message metadata first used to return results

TODOs:

  • Add details to benchmark CSVs
  • Push GPU containers that use Unix
  • p99 results of some sort

This change is pretty much done, but I found a related problem. The worker runs into a scaling issue (or something) at high levels of client calls. Such that when running ~40 clients with high frequency invocations, using the scaling part of load gen and the simple hello image, the worker seems to stop responding. Not related to this issue I think, as it happens to the old HTTP client too

communication protocol over socket between container and worker

container communication abstracted

simplified types
example comparing performance of unix vs http
@aFuerst aFuerst requested a review from prateek-s February 18, 2025 15:02
@aFuerst aFuerst self-assigned this Feb 18, 2025
prateek-s
prateek-s previously approved these changes Feb 18, 2025
Copy link
Contributor

@prateek-s prateek-s left a comment

Choose a reason for hiding this comment

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

ok!

@aFuerst aFuerst marked this pull request as ready for review February 20, 2025 13:09
@aFuerst aFuerst merged commit 907c160 into COS-IN:master Feb 20, 2025
4 checks passed
@aFuerst aFuerst deleted the unix_socket branch February 20, 2025 13:11
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.

2 participants