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

Clean up Ray processes after cluster util exits #3278

Merged
merged 6 commits into from
Nov 13, 2018

Conversation

richardliaw
Copy link
Contributor

Adds atexit hook to Cluster instantiation.

Closes #3277.

@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/9204/
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/9208/
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/9210/
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/9211/
Test PASSed.

assert not any(n.any_processes_alive() for n in [node, node2])


def test_shutdown():
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this intended to test? It seems redundant with the above test. Also, it doesn't hit the atexit code path right? Because the Python interpreter is not exiting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's to test that the shutdown endpoint works as expected.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, I see. It's not testing the atexit stuff though, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No; it's just an additional test piggy-backing on this approval..

@robertnishihara
Copy link
Collaborator

If you want to test this functionality, I think you need to do something like

ray.utils.run_string_as_driver(
"""
import ...
g = Cluster(initialize_head=False)
node = g.add_node()
node2 = g.add_node()
"""
)

# Then in python, check that the raylets and plasma stores and such are gone.

@ericl ericl merged commit 97f4237 into ray-project:master Nov 13, 2018
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.

When using ray.test.cluster_utils, many Ray processes are not cleaned up when the script exits.
4 participants