Skip to content

[core] Fix comments and a corner case in #32302 - #32323

Merged
fishbone merged 2 commits into
ray-project:masterfrom
fishbone:fix-comments
Feb 8, 2023
Merged

[core] Fix comments and a corner case in #32302#32323
fishbone merged 2 commits into
ray-project:masterfrom
fishbone:fix-comments

Conversation

@fishbone

@fishbone fishbone commented Feb 8, 2023

Copy link
Copy Markdown
Contributor

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

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

Comment thread src/ray/rpc/grpc_server.cc Outdated
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++) {

Copy link
Copy Markdown
Contributor

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 ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oops

@fishbone fishbone added P0 Issues that should be fixed in short order Ray 2.3 release-blocker P0 Issue that blocks the release labels Feb 8, 2023
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";

Copy link
Copy Markdown
Member

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 😄

Copy link
Copy Markdown
Contributor Author

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.

@fishbone
fishbone merged commit 3bb73d3 into ray-project:master Feb 8, 2023
fishbone added a commit to fishbone/ray that referenced this pull request Feb 8, 2023
…ct#32323)

This is a corner case where buffer could be 0 and a comments needs to be fixed in the previous PR.
edoakes pushed a commit to edoakes/ray that referenced this pull request Mar 22, 2023
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P0 Issues that should be fixed in short order release-blocker P0 Issue that blocks the release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants