Skip to content

[Core] Log monitor multidriver#8953

Merged
ericl merged 72 commits intoray-project:masterfrom
wuisawesome:log-monitor-multidriver
Jun 25, 2020
Merged

[Core] Log monitor multidriver#8953
ericl merged 72 commits intoray-project:masterfrom
wuisawesome:log-monitor-multidriver

Conversation

@wuisawesome
Copy link
Copy Markdown
Contributor

@wuisawesome wuisawesome commented Jun 15, 2020 •
edited
Loading

Why are these changes needed?

Note: This is based off of #8885, not master.

This PR ensures that stdout/err for tasks are directed at the proper drivers and not broadcasted to all drivers.

Related issue number

Closes #4029

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)

@wuisawesome
.
ce5775d
@wuisawesome
.
c57b228
@wuisawesome
Correct now
60f6f4f
@wuisawesome
No interactivity errors
ff77d8d
@wuisawesome
Merge branch 'master' of https://github.com/ray-project/ray into ssh-…
922e9c3 
…command-format
@wuisawesome
Merge branch 'master' into command-runner-interactivity
359d250
@wuisawesome
Merge branch 'master' into command-runner-interactivity
3e9c8bf
@wuisawesome
Merge branch 'master' of https://github.com/ray-project/ray into comm…
227aac8 
…and-runner-interactivity
@wuisawesome
format
29ab5c0
@wuisawesome
Filtering
f894dc1
@wuisawesome
lint
b49d8f1
@wuisawesome
.
69ada2d
@wuisawesome
No more filtering
a897f53
@wuisawesome
Removed interactivity
14b000f
@wuisawesome
.
ea36dd9
@wuisawesome
.
f425c4a
@wuisawesome
.
0be4d1c
@wuisawesome
Merge branch 'command-runner-interactivity' into multidriver-logging
db9f3ff
@wuisawesome
.
201397c
@wuisawesome
.
93ef3c6
@wuisawesome
.
cda3509
@wuisawesome
Merge branch 'master' of https://github.com/ray-project/ray into mult…
ea27d37 
…idriver-logging
@wuisawesome
Redirection works
90c83eb
@wuisawesome
formatting
c26400d
@wuisawesome
something broken?
048dd1f
@wuisawesome
.
ecaf0cf
@wuisawesome
Works
1b61b69
@wuisawesome
formatting
54524bb
@wuisawesome
redirect output
2f49721
@wuisawesome
formatting
6e43bc3
@AmplabJenkins
Copy link
Copy Markdown

AmplabJenkins commented Jun 16, 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/27192/
Test FAILed.

@wuisawesome wuisawesome changed the title [WIP][Core] Log monitor multidriver Log monitor multidriver Jun 24, 2020
@wuisawesome wuisawesome changed the title Log monitor multidriver [Core\ Log monitor multidriver Jun 24, 2020
@wuisawesome wuisawesome changed the title [Core\ Log monitor multidriver [Core] Log monitor multidriver Jun 24, 2020
@wuisawesome wuisawesome requested a review from ericl June 24, 2020 19:48
ericl
ericl reviewed Jun 24, 2020
View reviewed changes
python/ray/log_monitor.py
json.dumps({
"ip": self.ip,
"pid": file_info.worker_pid,
"job": file_info.job_id,
Copy link
Copy Markdown
Contributor

@ericl ericl Jun 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Where is this actually checked? I don't see the code in this PR.

ericl
ericl reviewed Jun 24, 2020
View reviewed changes
python/ray/tests/test_multi_node.py
ray.get(f.remote())


def test_multi_driver_logging(ray_start_regular):
Copy link
Copy Markdown
Contributor

@ericl ericl Jun 24, 2020

Choose a reason for hiding this comment

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

Did you check that this test fails before filtering by job id?

@AmplabJenkins
Copy link
Copy Markdown

AmplabJenkins commented Jun 24, 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/27527/
Test FAILed.

@AmplabJenkins
Copy link
Copy Markdown

AmplabJenkins commented Jun 24, 2020

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

ericl
ericl approved these changes Jun 24, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@ericl ericl left a comment

Choose a reason for hiding this comment

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

Tried and it works!

@AmplabJenkins
Copy link
Copy Markdown

AmplabJenkins commented Jun 24, 2020

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

@AmplabJenkins
Copy link
Copy Markdown

AmplabJenkins commented Jun 25, 2020

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

@AmplabJenkins
Copy link
Copy Markdown

AmplabJenkins commented Jun 25, 2020

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

@ericl ericl merged commit 46962f5 into ray-project:master Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericl ericl ericl approved these changes

Assignees

@ericl ericl

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Stdout/stderr from all drivers are streamed to all other drivers.

3 participants

@wuisawesome @AmplabJenkins @ericl