Skip to content

[Core] Log output from different jobs to different drivers. #8885

Merged
ericl merged 67 commits intoray-project:masterfrom
wuisawesome:multidriver-logging
Jun 24, 2020
Merged

[Core] Log output from different jobs to different drivers. #8885
ericl merged 67 commits intoray-project:masterfrom
wuisawesome:multidriver-logging

Conversation

@wuisawesome
Copy link
Copy Markdown
Contributor

@wuisawesome wuisawesome commented Jun 10, 2020

Why are these changes needed?

This is a first step to handling ensuring that only output from the relevant job is sent to each driver. This PR will make each worker write its own output to a file titled worker-{worker-id}.out (which it already does), but use produced output from jobs will now be written to a file titled worker-{worker-id}-{job-id}.out. stderr will also be redirected accordingly.

Related issue number

First of 2 steps towards closing #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)

No tests here. In some ways this is just a non-trivial refactoring in the sense that it shouldn't effect end behavior, thus it's sufficient to not break anything. When behavior changes (i.e. next PR where output is actually sent to specific drivers only), test cases will be included.

"""
redirect_output = self._ray_params.redirect_output

if redirect_output is None:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we remove this?

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.

Overall looks good

@AmplabJenkins
Copy link
Copy Markdown

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

@AmplabJenkins
Copy link
Copy Markdown

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

@AmplabJenkins
Copy link
Copy Markdown

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

@ericl
Copy link
Copy Markdown
Contributor

ericl commented Jun 23, 2020

test_tempfile still seems to be failing, does it pass locally?

@AmplabJenkins
Copy link
Copy Markdown

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

@AmplabJenkins
Copy link
Copy Markdown

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

@AmplabJenkins
Copy link
Copy Markdown

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

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.

5 participants