[Datasets] Persist Datasets statistics to log file #30557
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Scott Lee <[email protected]>
scottjlee
requested review from
ericl,
scv119,
clarkzinzow,
jjyao,
jianoaix and
c21
as code owners
Signed-off-by: Scott Lee <[email protected]>
Signed-off-by: Scott Lee <[email protected]>
Signed-off-by: Scott Lee <[email protected]>
Signed-off-by: Scott Lee <[email protected]>
scottjlee
commented
Nov 23, 2022
c21
reviewed
Nov 23, 2022
Signed-off-by: Scott Lee <[email protected]>
Signed-off-by: Scott Lee <[email protected]>
Signed-off-by: Scott Lee <[email protected]>
Signed-off-by: Scott Lee <[email protected]>
Signed-off-by: Scott Lee <[email protected]>
Signed-off-by: Scott Lee <[email protected]>
c21
reviewed
Nov 29, 2022
|
Hi @clarkzinzow and @jianoaix - could you help take a look? Thanks. |
Signed-off-by: Scott Lee <[email protected]>
Signed-off-by: Scott Lee <[email protected]>
|
So currently, this log is written to the worker log which is persisted already as .out, and in case of job failure, we can access the stats from worker log, right? |
scottjlee
requested review from
gjoliver,
avnishn,
ArturNiederfahrenhorst,
smorad,
kouroshHakha,
fishbone,
stephanie-wang,
suquark,
ijrsvt and
alanwguo
as code owners
scottjlee
force-pushed
the
stats-logfile
branch
from
d8c09b2
to
7c4db74
Compare
c21
approved these changes
Dec 5, 2022
Signed-off-by: Scott Lee <[email protected]>
Signed-off-by: Scott Lee <[email protected]>
jianoaix
reviewed
Dec 6, 2022
| """ | ||
| # Logger used to logging to log file (in addition to the root logger, | ||
| # which logs to stdout as normal). We set `logger.propagate` to False | ||
| # to ensure the file logger only logs to the file, and not stdout, by default. |
There was a problem hiding this comment.
This documentation seems confusing to me since the class-level comment says it writes to the file in addition to stdout.
There was a problem hiding this comment.
Reworded the comments and documentation to clarify here and in the class level comment, let me know if things are still confusing here. thanks!
Signed-off-by: Scott Lee <[email protected]>
|
The failed CI test looks irrelevant - https://flakey-tests.ray.io/ .
|
c21
added
the
tests-ok
|
@clarkzinzow, @jianoaix any more comments? Thanks |
jianoaix
approved these changes
Dec 7, 2022
WeichenXu123
pushed a commit
to WeichenXu123/ray
that referenced
this pull request
Dec 19, 2022
Currently, when we print Dataset stats after execution, there is no way to retrieve this information in case of job failure/crash. By persisting the logs to a separate file, we can access the stats which could be helpful for debugging. By default, this is configured to write to /logs/ray-data.log. The new logger, DatasetLogger, is configured to always write logs to the ray-data.log file, and optionally also writes to stdout (this is enabled by default). The motivation behind this is so that users can easily use the specific log file to filter for Dataset logs, while still maintaining console logs for those who use them. Signed-off-by: Weichen Xu <[email protected]>
7 tasks
tamohannes
pushed a commit
to ju2ez/ray
that referenced
this pull request
Jan 25, 2023
Currently, when we print Dataset stats after execution, there is no way to retrieve this information in case of job failure/crash. By persisting the logs to a separate file, we can access the stats which could be helpful for debugging. By default, this is configured to write to /logs/ray-data.log. The new logger, DatasetLogger, is configured to always write logs to the ray-data.log file, and optionally also writes to stdout (this is enabled by default). The motivation behind this is so that users can easily use the specific log file to filter for Dataset logs, while still maintaining console logs for those who use them. Signed-off-by: tmynn <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why are these changes needed?
Currently, when we print Dataset stats after execution, there is no way to retrieve this information in case of job failure/crash. By persisting the logs to a separate file, we can access the stats which could be helpful for debugging. By default, this is configured to write to
/logs/ray-data.log:The new logger,
DatasetLogger, is configured to always write logs to theray-data.logfile, and optionally also writes to stdout (this is enabled by default). The motivation behind this is so that users can easily use the specific log file to filter for Dataset logs, while still maintaining console logs for those who use them.Related issue number
Closes #29575
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.