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.

Sign up for GitHub

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

Jump to bottom

Create session_latest symlink for Ray sessions #5580

Merged
merged 2 commits into from
Sep 1, 2019
Merged

Create session_latest symlink for Ray sessions #5580

merged 2 commits into from
Sep 1, 2019

Conversation

edoakes
Copy link
Contributor

@edoakes edoakes commented Aug 29, 2019 •
edited by robertnishihara
Loading

What do these changes do?

When Ray starts up, creates or overwrites a symlink at session_latest to the latest session directory. If this fails, it will only log a warning, not crash.

Related issue number

Fixes #5578

Linter

  • I've run scripts/format.sh to lint the changes in this PR.

@edoakes edoakes requested review from pcmoritz and ericl August 29, 2019 22:33
@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/16645/
Test PASSed.

ericl
ericl reviewed Aug 30, 2019
View reviewed changes
python/ray/utils.py
"""
logger = logging.getLogger("ray")
directory_path = os.path.expanduser(directory_path)
symlink_path = os.path.expanduser(symlink_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI the canonical way to do this is to create the symlink as a tmp file, and then do an atomic move.

ericl
ericl reviewed Aug 30, 2019
View reviewed changes
python/ray/node.py

# Logger for this module. It should be configured at the entry point
# into the program using Ray. Ray configures it by default automatically
# using logging.basicConfig in its entry/init points.
logger = logging.getLogger(__name__)

PY3 = sys.version_info.major >= 3
SESSION_LATEST = "session_latest"
Copy link
Contributor

Choose a reason for hiding this comment

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

how about just latest?

@robertnishihara
Copy link
Collaborator

Seems like a great idea, since we're always copying and pasting the session strings...

Do we care about the case where the user simulates multiple nodes on a single machine, e.g.,

import ray
from ray.tests.cluster_utils import Cluster

c = Cluster()

c.add_node(include_webui=True)

for _ in range(10):
    c.add_node()

ray.init(redis_address=c.redis_address)

In that case, will it just point to the last one out of the 10 created?

cc @suquark who has worked a lot on this code and may have tried something similar in the past.

@edoakes
Copy link
Contributor Author

edoakes commented Aug 30, 2019

Yes it will, but I think that's fine. The right way to support that is probably by giving each of them a separate top-level ray directory.

@robertnishihara
Copy link
Collaborator

@edoakes are you still making changes here or should this be merged?

@edoakes
Copy link
Contributor Author

edoakes commented Sep 1, 2019

Go ahead and merge.

@robertnishihara robertnishihara merged commit 0cc0abf into ray-project:master Sep 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericl ericl ericl left review comments

@robertnishihara robertnishihara robertnishihara approved these changes

@pcmoritz pcmoritz Awaiting requested review from pcmoritz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Finding the latest session log files is annoying
4 participants
@edoakes @AmplabJenkins @robertnishihara @ericl