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

Fix global_state not disconnected after ray.shutdown #4354

Merged
merged 6 commits into from
Mar 18, 2019

Conversation

jovany-wang
Copy link
Contributor

@jovany-wang jovany-wang commented Mar 13, 2019

What do these changes do?

Considering this script:

import ray
ray.init()
ray.shutdown()
ray.global_state.object_table()

It throws a ConnectionRefusedError error. Worse, _check_connected () will pass.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12839/
Test FAILed.

@jovany-wang jovany-wang changed the title Make global state disconnects from GCS gracefully Fix global_state not disconnected after ray.shutdown Mar 14, 2019
@robertnishihara robertnishihara self-assigned this Mar 14, 2019
@jovany-wang jovany-wang deleted the fix-global-state branch March 15, 2019 03:34
@jovany-wang jovany-wang restored the fix-global-state branch March 15, 2019 03:35
@jovany-wang jovany-wang reopened this Mar 15, 2019
@@ -2088,6 +2088,9 @@ def disconnect():
if hasattr(worker, "plasma_client"):
worker.plasma_client.disconnect()

# Disconnect global state from GCS.
global_state.disconnect()
Copy link
Contributor

Choose a reason for hiding this comment

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

better put this in shutdown, because global_state isn't part of the worker

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12894/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12977/
Test FAILed.



def test_shutdown_disconnect_global_state():
ray.init()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ray.init()
ray.init(num_cpus=0)

@@ -2054,7 +2057,7 @@ def connect(info,


def disconnect():
"""Disconnect this worker from the scheduler and object store."""
"""Disconnect this worker from the raylet and object store"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"""Disconnect this worker from the raylet and object store"""
"""Disconnect this worker from the raylet and object store."""

@robertnishihara
Copy link
Collaborator

I'm surprised that Travis ran the Java tests.. it shouldn't have done that for this PR.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12986/
Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12988/
Test PASSed.

@robertnishihara robertnishihara merged commit 3b141b2 into ray-project:master Mar 18, 2019
@robertnishihara
Copy link
Collaborator

Thanks @jovany-wang!

@raulchen raulchen deleted the fix-global-state branch March 27, 2019 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants