Stream logs to driver by default. #3892

robertnishihara · 2019-01-29T23:13:48Z

This fixes #2173.

In this PR:

By default, worker stdout/stderr on all machines (including the single machine case) are written to files and streamed to the driver via file on disk -> log_monitor -> redis -> driver.
If the log monitor dies, an exception is pushed to the driver.
We now shutdown driver threads when we call ray.shutdown(). This is most relevant for our tests.

Possible TODO:

Make sure we don't ignore files once a large number of workers have been created.
Think through relevant thread safety issues.
Format print statements to include node and process ID of process that logged the statement, maybe function name as well.
Code cleanups (hard coded strings).
Isolation between drivers, ideally only print logging statements for the relevant driver.
Think through the API (naming, and in particular, the interaction with redirect_output and redirect_worker_output).
Tests
Push error to all drivers if log monitor fails.
Test log monitor failure.

AmplabJenkins · 2019-01-30T01:32:02Z

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

robertnishihara · 2019-02-01T06:38:05Z

@richardliaw @pcmoritz @ericl the main thing that's missing here is isolation between drivers. That is, by default, if you connect multiple drivers to the same cluster, then they will all see each other's stdout/stderr.

I don't see a good way to address this at the moment. Two possibilities are

Use separate workers for separate drivers.
Include extra annotations in the log files so that the log monitor can figure out which log statements belong to which drivers.

I think it's ok to merge without this feature. Though in the multi-tenant setting it will be necessary to use ray.init(log_to_driver=False). What do you think? I'd like to leave log_to_driver=True as the default so that things just work naturally out of the box. That way, things will behave intuitively the first time you start running on a cluster. Then when you need more complex workloads, you can figure out how to turn off the logging. The opposite approach makes the simple behavior hard and the complex setting easy.

robertnishihara · 2019-02-01T06:41:55Z

This PR lets you do

import ray
import sys

ray.init(redis_address=...)

@ray.remote
def f():
    print("hello!")
    print("hi", file=sys.stderr)
    sys.stdout.flush()
    sys.stderr.flush()

[f.remote() for _ in range(10)]

You will see something like

worker (stdout) (pid=33570, host=Roberts-MBP)
hello!
worker (stdout) (pid=33571, host=Roberts-MBP)
hello!
worker (stderr) (pid=33568, host=Roberts-MBP)
hi
worker (stderr) (pid=33570, host=Roberts-MBP)
hi
worker (stdout) (pid=33567, host=Roberts-MBP)
hello!
worker (stderr) (pid=33564, host=Roberts-MBP)
hi
worker (stdout) (pid=33564, host=Roberts-MBP)
hello!
worker (stderr) (pid=33567, host=Roberts-MBP)
hi
worker (stdout) (pid=33568, host=Roberts-MBP)
hello!
worker (stderr) (pid=33565, host=Roberts-MBP)
hi
worker (stdout) (pid=33566, host=Roberts-MBP)
hello!
worker (stderr) (pid=33571, host=Roberts-MBP)
hi
worker (stderr) (pid=33566, host=Roberts-MBP)
hi
worker (stdout) (pid=33565, host=Roberts-MBP)
hello!
worker (stderr) (pid=33563, host=Roberts-MBP)
hi
worker (stdout) (pid=33563, host=Roberts-MBP)
hello!
worker (stdout) (pid=33567, host=Roberts-MBP)
hello!
worker (stderr) (pid=33567, host=Roberts-MBP)
hi
worker (stderr) (pid=33565, host=Roberts-MBP)
hi
worker (stdout) (pid=33565, host=Roberts-MBP)
hello!

Currently all of this goes to stderr, though maybe worker stdout should also go to stdout? Im not sure. If a worker prints a lot at once, then more lines will be batched together.

Note that the sys.stdout.flush() and sys.stderr.flush() are necessary. There seems to be a lot of buffering otherwise, and it trying to make the files unbuffered doesn't work out of the box..

AmplabJenkins · 2019-02-01T08:16:43Z

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

AmplabJenkins · 2019-02-01T09:43:50Z

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

AmplabJenkins · 2019-02-01T09:59:57Z

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

AmplabJenkins · 2019-02-01T21:56:04Z

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

AmplabJenkins · 2019-02-01T23:26:57Z

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

AmplabJenkins · 2019-02-02T04:13:34Z

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

AmplabJenkins · 2019-02-02T22:06:53Z

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

AmplabJenkins · 2019-02-02T22:23:26Z

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

robertnishihara · 2019-02-04T20:35:39Z

python/ray/worker.py

https://github.com/andymccurdy/redis-py says that Redis clients can safely be shared between threads (however pubsub clients cannot).

We should leave a comment here about this thread-safe behavior.

AmplabJenkins · 2019-02-04T22:13:56Z

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

AmplabJenkins · 2019-02-04T22:21:49Z

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

AmplabJenkins · 2019-02-04T23:15:41Z

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

AmplabJenkins · 2019-02-04T23:30:03Z

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

robertnishihara · 2019-02-04T23:47:05Z

python/ray/utils.py

@richardliaw please take a look, this no longer seems necessary.

oh because dup2 -- yeah this is probably fine; this was basically to pass one of the tests in run_test so as long as this passes it's fine

robertnishihara · 2019-02-04T23:48:06Z

python/ray/worker.py

@richardliaw I would use the logger module, but the output seemed uglier.

AmplabJenkins · 2019-02-05T00:07:17Z

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

AmplabJenkins · 2019-02-05T00:35:06Z

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

AmplabJenkins · 2019-02-05T02:35:19Z

Merged build finished. Test FAILed.

This reverts commit b3846b89040bcd8e583b2e18cb513cb040e71d95.

…ject manager.

AmplabJenkins · 2019-02-07T21:34:16Z

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

AmplabJenkins · 2019-02-07T23:27:58Z

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

AmplabJenkins · 2019-02-08T00:38:12Z

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

AmplabJenkins · 2019-02-08T00:50:42Z

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

This reverts commit ef527f8.

robertnishihara assigned richardliaw Jan 29, 2019

robertnishihara mentioned this pull request Jan 31, 2019

redis-server and raylet memory usage grows constantly. #3900

Closed

robertnishihara force-pushed the logtodriver branch from 7142eeb to 5626107 Compare February 1, 2019 06:31

robertnishihara changed the title ~~[WIP] Prototype streaming logs to driver.~~ Stream logs to driver by default. Feb 1, 2019

robertnishihara force-pushed the logtodriver branch from 2e8d96d to dec7ce7 Compare February 1, 2019 07:24

robertnishihara force-pushed the logtodriver branch from dec7ce7 to 6d712d6 Compare February 1, 2019 19:44

robertnishihara force-pushed the logtodriver branch from c2312b8 to d1f37e4 Compare February 2, 2019 19:28

robertnishihara force-pushed the logtodriver branch from d1f37e4 to a972dfb Compare February 4, 2019 19:29

robertnishihara commented Feb 4, 2019

View reviewed changes

robertnishihara requested a review from guoyuhong February 4, 2019 23:43

robertnishihara commented Feb 4, 2019

View reviewed changes

python/ray/worker.py Outdated

Copy link

Collaborator Author

robertnishihara Feb 4, 2019

Choose a reason for hiding this comment

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

@richardliaw I would use the logger module, but the output seemed uglier.

robertnishihara added 13 commits February 7, 2019 13:04

Add warning for too many log messages.

5a1a64d

Only stop threads if worker is connected.

a2e8c87

Only stop threads if they exist.

8535645

Remove unnecessary try/excepts.

76ca0e7

Fix

f8278d2

Only add new logging handler once.

35f5b81

Increase timeout.

dc02bbe

Fix tempfile test.

d1ed2be

Fix logging in cluster_utils.

26021ae

Revert "Increase timeout."

57518d2

This reverts commit b3846b89040bcd8e583b2e18cb513cb040e71d95.

Retry longer when connecting to plasma store from node manager and ob…

dfbbfb6

…ject manager.

Close pubsub channels to avoid leaking file descriptors.

1f31342

Limit log monitor open files to 200.

036165f

robertnishihara force-pushed the logtodriver branch from 6fd54d8 to 9c9c031 Compare February 7, 2019 21:14

Increase plasma connect retries.

6fa0474

robertnishihara force-pushed the logtodriver branch from 9c9c031 to 6fa0474 Compare February 7, 2019 21:40

Add comment.

f5a348b

pcmoritz approved these changes Feb 8, 2019

View reviewed changes

pcmoritz merged commit ef527f8 into ray-project:master Feb 8, 2019

robertnishihara deleted the logtodriver branch February 8, 2019 03:55

This was referenced Feb 8, 2019

Limit size of stdout/stderr log files in primary Redis shard. #3874

Closed

Deprecate redirect_output and redirect_worker_output arguments to ray.init. #3989

Closed

guoyuhong mentioned this pull request Feb 10, 2019

Fix CI test failures #4007

Merged

robertnishihara mentioned this pull request Feb 12, 2019

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

Closed

pcmoritz added a commit to pcmoritz/ray-1 that referenced this pull request Mar 4, 2019

Revert "Stream logs to driver by default. (ray-project#3892)"

f4d6324

This reverts commit ef527f8.

simon-mo mentioned this pull request Apr 15, 2019

No log_to_driver option in cli mode #4632

Closed

Stream logs to driver by default. #3892

Stream logs to driver by default. #3892

Uh oh!

Conversation

robertnishihara commented Jan 29, 2019 • edited by guoyuhong Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AmplabJenkins commented Jan 30, 2019

Uh oh!

robertnishihara commented Feb 1, 2019

Uh oh!

robertnishihara commented Feb 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AmplabJenkins commented Feb 1, 2019

Uh oh!

AmplabJenkins commented Feb 1, 2019

Uh oh!

AmplabJenkins commented Feb 1, 2019

Uh oh!

AmplabJenkins commented Feb 1, 2019

Uh oh!

AmplabJenkins commented Feb 1, 2019

Uh oh!

AmplabJenkins commented Feb 2, 2019

Uh oh!

AmplabJenkins commented Feb 2, 2019

Uh oh!

AmplabJenkins commented Feb 2, 2019

Uh oh!

robertnishihara Feb 4, 2019

Choose a reason for hiding this comment

Uh oh!

suquark Feb 6, 2019

Choose a reason for hiding this comment

Uh oh!

robertnishihara Feb 7, 2019

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Feb 4, 2019

Uh oh!

AmplabJenkins commented Feb 4, 2019

Uh oh!

AmplabJenkins commented Feb 4, 2019

Uh oh!

AmplabJenkins commented Feb 4, 2019

Uh oh!

robertnishihara Feb 4, 2019

Choose a reason for hiding this comment

Uh oh!

richardliaw Feb 6, 2019

Choose a reason for hiding this comment

Uh oh!

robertnishihara Feb 4, 2019

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Feb 5, 2019

Uh oh!

AmplabJenkins commented Feb 5, 2019

Uh oh!

AmplabJenkins commented Feb 5, 2019

Uh oh!

AmplabJenkins commented Feb 7, 2019

Uh oh!

AmplabJenkins commented Feb 7, 2019

Uh oh!

AmplabJenkins commented Feb 8, 2019

Uh oh!

AmplabJenkins commented Feb 8, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

robertnishihara commented Jan 29, 2019 •

edited by guoyuhong

Loading

robertnishihara commented Feb 1, 2019 •

edited

Loading