Skip to content

[Core] fix keepalive in grpc client - #33986

Merged
scv119 merged 1 commit into
ray-project:masterfrom
scv119:keep-alive
Apr 1, 2023
Merged

[Core] fix keepalive in grpc client#33986
scv119 merged 1 commit into
ray-project:masterfrom
scv119:keep-alive

Conversation

@scv119

@scv119 scv119 commented Mar 31, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Why are these changes needed?

The keepalive grpc arguments is applied on a channel object that not used, thus it's not affective on worker client. This PR fixed it.

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 added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • 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 :(

@scv119
scv119 marked this pull request as ready for review March 31, 2023 23:54
@scv119 scv119 linked an issue Mar 31, 2023 that may be closed by this pull request
[core] Corrupted cluster state due to restarting actors associated with finished jobs prevents autoscaler from scaling down #33984
Closed
rickyyx
rickyyx reviewed Apr 1, 2023
View reviewed changes

@rickyyx rickyyx left a comment

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.

rkooo567
rkooo567 approved these changes Apr 1, 2023
View reviewed changes

@rkooo567 rkooo567 left a comment

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.

Oh wow… good catch

@scv119
scv119 merged commit 96be4e0 into ray-project:master Apr 1, 2023
@ollie-iterators ollie-iterators mentioned this pull request Apr 3, 2023
Closed
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
@scv119 @elliottower
[Core] fix keepalive in grpc client ray-project#33986
9d938de 
The keepalive grpc arguments is applied on a channel object that not used, thus it's not affective on worker client. This PR fixed it.

Signed-off-by: elliottower <elliot@elliottower.com>
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request May 4, 2023
@scv119 @ProjectsByJackHe
[Core] fix keepalive in grpc client ray-project#33986
de770bd 
The keepalive grpc arguments is applied on a channel object that not used, thus it's not affective on worker client. This PR fixed it.

Signed-off-by: Jack He <jackhe2345@gmail.com>
@dzikrisyairozi

dzikrisyairozi commented May 17, 2023

Copy link
Copy Markdown

how do we recreate this bugs?

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

Reviewers

@rickyyx rickyyx rickyyx approved these changes
@rkooo567 rkooo567 rkooo567 approved these changes

Assignees

@rickyyx rickyyx

@rkooo567 rkooo567

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[core] Corrupted cluster state due to restarting actors associated with finished jobs prevents autoscaler from scaling down

4 participants

@scv119 @dzikrisyairozi @rickyyx @rkooo567