Skip to content

Fix Google log directory#9025

Merged
edoakes merged 1 commit intoray-project:masterfrom
mehrdadn:glog-dir
Jun 19, 2020
Merged

Fix Google log directory#9025
edoakes merged 1 commit intoray-project:masterfrom
mehrdadn:glog-dir

Conversation

@mehrdadn
Copy link
Copy Markdown
Contributor

@mehrdadn mehrdadn commented Jun 19, 2020 •
edited
Loading

Why are these changes needed?

Currently, some log files end up in the user temp directory rather than inside the Ray session log directory, due to an apparent misconfiguration of glog. Fix PR fixes this issue to ensure all Ray-related logs go to the Ray session log directory.

Related issue number

#631

Checks

  • 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/latest/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failure rates at https://ray-travis-tracker.herokuapp.com/.
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested (please justify below)

@AmplabJenkins
Copy link
Copy Markdown

AmplabJenkins commented Jun 19, 2020

Can one of the admins verify this patch?

@mehrdadn mehrdadn marked this pull request as ready for review June 19, 2020 02:52
@edoakes
Copy link
Copy Markdown
Collaborator

edoakes commented Jun 19, 2020

Oh, interesting - I've seen this happen before but never often enough for me to look into it. What exactly is the situation where it occurs? I will have a closer look at the code changes in the morning.

@mehrdadn
Copy link
Copy Markdown
Contributor Author

mehrdadn commented Jun 19, 2020

It actually happens all the time for me, just python -c "import ray; ray.init(); ray.shutdown()" is enough.

The reason is that google::SetLogDestination needs to be called for every log level, but for some reason currently it's only called for the threshold level itself, so higher log levels end up going into the default log file.

@AmplabJenkins
Copy link
Copy Markdown

AmplabJenkins commented Jun 19, 2020

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

edoakes
edoakes approved these changes Jun 19, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@edoakes edoakes left a comment

Choose a reason for hiding this comment

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

LGTM. Seems like behavior must not be standard across platforms when it isn't set this way, but this seems like an improvement.

@edoakes edoakes merged commit f43cad6 into ray-project:master Jun 19, 2020
@mehrdadn mehrdadn deleted the glog-dir branch June 19, 2020 18:45
@mehrdadn mehrdadn mentioned this pull request Jun 21, 2020
Merged
6 tasks
@mehrdadn mehrdadn mentioned this pull request Jul 11, 2020
Merged
6 tasks
@mehrdadn mehrdadn mentioned this pull request Sep 25, 2020
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@edoakes edoakes edoakes approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mehrdadn @AmplabJenkins @edoakes @web-flow