[core] Fix gcs logging #48952
[core] Fix gcs logging #48952
Conversation
Signed-off-by: hjiang <[email protected]>
Signed-off-by: hjiang <[email protected]>
dentiny
force-pushed
the
hjiang/gcs-service-logging
branch
from
79144b4
to
7c1c266
Compare
| @@ -38,17 +41,40 @@ DEFINE_string(session_name, | |||
| "session_name: The session name (ClusterID) of the cluster."); | |||
| DEFINE_string(ray_commit, "", "The commit hash of Ray."); | |||
|
|
|||
| namespace { | |||
| // GCS server output filename. | |||
| constexpr std::string_view kGcsServerLog = "gcs_server.out"; | |||
There was a problem hiding this comment.
get_log_file_handles can create names like gcs_server.2.out if gcs_server.out and gcs_server.1.out both exists. Do we have such thing in spdlog?
There was a problem hiding this comment.
All rotated logs are suffixed with id, similar to what you're described.
| ? std::numeric_limits<int64_t>::max() | ||
| : FLAGS_log_rotation_size; | ||
| RAY_CHECK_EQ(setenv( | ||
| "RAY_ROTATION_MAX_BYTES", std::to_string(log_rotation_max_size), /*overwrite=*/1)); |
There was a problem hiding this comment.
can you explain diff of RAY_ROTATION_MAX_BYTES vs FLAGS_log_rotation_size ? If we already have the former, then we only need to fix existing behavior? I see gcs_server_main.cc already call ray::RayLog::StartRayLog and why does the log rotations in it do not work?
There was a problem hiding this comment.
If we don't do anything in this PR, instead only do 2 things:
- remove python stdout/stderr redirection
- change ray_log_shutdown_raii from
/*log_dir=*/""to/*log_dir=*/FLAGS_log_dir
will the rotations automatically work?
There was a problem hiding this comment.
Yes but the file name will be changed. We want to keep the existing gcs_server.out filename for backward compatibility.
There was a problem hiding this comment.
will the rotations automatically work?
To answer your question, passing the log directory works for log rotation.
But one motivation would be backward compatibility, namely keep the gcs_server.out filename.
Signed-off-by: hjiang <[email protected]>
Signed-off-by: hjiang <[email protected]>
Signed-off-by: hjiang <[email protected]>
Signed-off-by: hjiang <[email protected]>
Same motivation as #48931, but different implementation.
TLDR for the problem: