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

Add support for unix sockets #244

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

roger652
Copy link

This PR allows to run the Triton HTTP client with an unix domain socket.

The Triton server and client can then be started with an unix domain socket path:
e.g. -tritonServerHostAndPort=unix:///run/triton.unix

@debermudez
Copy link
Contributor

debermudez commented Feb 13, 2023

I am looking into this PR to verify. In the mean time, can you please sign the CLA: https://github.com/triton-inference-server/server/blob/main/CONTRIBUTING.md#contributor-license-agreement-cla

@roger652
Copy link
Author

I am looking into this PR to verify. In the mean time, can you please sign the CLA: https://github.com/triton-inference-server/server/blob/main/CONTRIBUTING.md#contributor-license-agreement-cla

Thanks @debermudez. I need to check the CLA with my employer. I will get back to you.

@debermudez
Copy link
Contributor

@roger652 After looking into this more, it does not seem that triton server would support unix sockets even with this PR. Can you please attach repro instructions? I want to repro on my end and see what work is necessary to get this fully functional for HTTP atleast.

@roger652
Copy link
Author

@roger652 After looking into this more, it does not seem that triton server would support unix sockets even with this PR. Can you please attach repro instructions? I want to repro on my end and see what work is necessary to get this fully functional for HTTP atleast.

The Triton server didn't require any changes to make it run on an unix socket.
The underlying evhtp library will use unix domain sockets when the URL starts with unix://

Steps to reproduce:

  1. Start Triton server (example for Jetson Nano)
    tritonserver --backend-config=tensorrt,version=8.2.1 --min-supported-compute-capability=5.3 --http-address=unix:///run/triton.unix
  2. Check the Triton server log output for:
    Started HTTPService at unix:///run/triton.unix:8000
  3. Connect with the Triton client:
    triton::client::InferenceServerHttpClient::Create(&client, "unix:///run/triton.unix");

@debermudez
Copy link
Contributor

@roger652 How are you able to get the server operational without the --allow-metrics=false flag?

At the moment, this seems to be working on triton server with the caveat that you need to disable metrics.
The triton server team has tickets now to look into this and enable that functionality in the future as well as cleaning up the server output around unix sockets.

Additionally, I cannot merge this without the signed CLA. Let me know when you have a moment to sign that document and send it over.

@roger652
Copy link
Author

@roger652 How are you able to get the server operational without the --allow-metrics=false flag?

At the moment, this seems to be working on triton server with the caveat that you need to disable metrics. The triton server team has tickets now to look into this and enable that functionality in the future as well as cleaning up the server output around unix sockets.

I'm still using an older Triton version (r22.05) and I didn't have the mentioned problem.

Additionally, I cannot merge this without the signed CLA. Let me know when you have a moment to sign that document and send it over.

I understand. I hope to get the approval to sign the CLA this week.

@roger652
Copy link
Author

@debermudez FYI the CLA is signed.

@debermudez
Copy link
Contributor

Great thank you @roger652.
There was a small snag on the triton server side that is blocking this from getting in. I am working with that team to get the update done on their side.
In the interim, it would be helpful to know if this is an enhancement you are contributing to the codebase or if this is blocking your work.
Thanks

@roger652
Copy link
Author

Great thank you @roger652. There was a small snag on the triton server side that is blocking this from getting in. I am working with that team to get the update done on their side. In the interim, it would be helpful to know if this is an enhancement you are contributing to the codebase or if this is blocking your work. Thanks

We are using Triton with this change in our product. So far we patch Triton manually before building.
It is not a blocker for us, but it would be great to include it to simplify the build and make it easier to switch to newer versions.

@roger652
Copy link
Author

@debermudez Any updates when we can merge this PR?

@Tabrizian
Copy link
Member

Hi @roger652, this PR is currently blocked on a few other changes that are needed in the server side. We'll merge these changes when the server side changes are complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants