-
Notifications
You must be signed in to change notification settings - Fork 6k
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
[core] Fix comments and a corner case in #32302 #32323
Conversation
src/ray/rpc/grpc_server.cc
Outdated
@@ -148,7 +149,7 @@ void GrpcServer::Run() { | |||
if (entry->GetMaxActiveRPCs() != -1) { | |||
buffer_size = entry->GetMaxActiveRPCs(); | |||
} | |||
for (int j = 0; j < (buffer_size / num_threads_); j++) { | |||
for (int j = 0; j < std::min(1, buffer_size / num_threads_); j++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it be std::max
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops
@@ -40,6 +40,7 @@ GrpcServer::GrpcServer(std::string name, | |||
is_closed_(true), | |||
num_threads_(num_threads), | |||
keepalive_time_ms_(keepalive_time_ms) { | |||
RAY_CHECK(num_threads_ > 0) << "Num of threads in gRPC must be greater than 0"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like a good case for an unsigned type 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but potentially it still could be 0 if it's unsigned type.
…ct#32323) This is a corner case where buffer could be 0 and a comments needs to be fixed in the previous PR.
…ct#32323) This is a corner case where buffer could be 0 and a comments needs to be fixed in the previous PR. Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Why are these changes needed?
This is a corner case where buffer could be 0 and a comments needs to be fixed in the previous PR.
Related issue number
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.