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

Update the limitation of multiple servers binding to the same http/grp… #4991

Merged
merged 3 commits into from
Oct 18, 2022

Conversation

krishung5
Copy link
Contributor

…c port

@krishung5 krishung5 changed the title Update the limitation of multiple server binding to the same http/grp… Update the limitation of multiple servers binding to the same http/grp… Oct 18, 2022
@dyastremsky
Copy link
Contributor

Are we sure this is the best place for this documentation? I think users usually check our documentation (e.g. .MD files) rather than the command line arguments help screen. Especially since it's quite long. If we want users to know about and understand these options, it might make sense to also include them in documentation.

@krishung5
Copy link
Contributor Author

@dyastremsky I might be wrong but I couldn't find any documentation regarding anything about http/grpc port except in main.cc. It seems like we ask users to use tritonserver --help to see those options that are not in the MD files. That's why I only updated main.cc, but I'm willing to add those options to the document if this makes more sense.

src/main.cc Outdated
@@ -401,7 +401,9 @@ std::vector<Option> options_
"The port for the server to listen on for HTTP requests."},
{OPTION_REUSE_HTTP_PORT, "reuse-http-port", Option::ArgBool,
"Allow multiple servers to listen on the same HTTP port when every "
"server has this option set."},
"server has this option set. The same set of models/same model "
Copy link
Member

Choose a reason for hiding this comment

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

"If you plan to use this option as a way to load-balance between different triton servers, the same model repository or set of models must be used for every server."

Note that this feature only supports stateless models.

I think it might be better to remove this sentence since the customers may figure out a way to control which server gets the requests and address this limitation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. Updated the document, thanks for the comment!

@dyastremsky
Copy link
Contributor

@dyastremsky I might be wrong but I couldn't find any documentation regarding anything about http/grpc port except in main.cc. It seems like we ask users to use tritonserver --help to see those options that are not in the MD files. That's why I only updated main.cc, but I'm willing to add those options to the document if this makes more sense.

I see, okay. Sounds like we're being consistent here then.

src/main.cc Outdated
@@ -401,7 +401,9 @@ std::vector<Option> options_
"The port for the server to listen on for HTTP requests."},
{OPTION_REUSE_HTTP_PORT, "reuse-http-port", Option::ArgBool,
"Allow multiple servers to listen on the same HTTP port when every "
"server has this option set."},
"server has this option set. If you plan to use this option as a way to "
"load-balance between different triton servers, the same model "
Copy link
Contributor

Choose a reason for hiding this comment

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

Here and below:

  • Triton should be capitalized.
  • Load balance should not have a dash.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the catch! Updated.

@krishung5 krishung5 merged commit dfa101c into main Oct 18, 2022
@krishung5 krishung5 deleted the krish-port-doc branch October 18, 2022 18:19
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.

3 participants