Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] Use boost iostream in pipe logger for cross-platform #50044

Merged
merged 31 commits into from
Feb 1, 2025
Merged
Changes from 1 commit
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
9f7ded8
fix test comment
dentiny Jan 23, 2025
8bdd94a
use boost iostream for cross-platform
dentiny Jan 23, 2025
ad89e4e
remove unnecessary cross-platform
dentiny Jan 24, 2025
dfc4ddc
fix windows build
dentiny Jan 24, 2025
df8c7ce
rename
dentiny Jan 24, 2025
94db87c
another rename
dentiny Jan 24, 2025
4fb9002
doc
dentiny Jan 24, 2025
025bad1
fix naming
dentiny Jan 25, 2025
5d74185
fix data flush
dentiny Jan 25, 2025
e6c6ce1
Merge branch 'master' into hjiang/boost-windows-jan-23
dentiny Jan 25, 2025
48ce959
no need to flush
dentiny Jan 25, 2025
64ff960
disable tsan
dentiny Jan 25, 2025
7f661f4
add check
dentiny Jan 27, 2025
342e401
Add another check
dentiny Jan 27, 2025
860120f
early exit for eof state
dentiny Jan 27, 2025
2847484
remove no tsan
dentiny Jan 27, 2025
f79043b
remove unreachable EOF branch
dentiny Jan 28, 2025
e624686
disable TSAN
dentiny Jan 28, 2025
3a78143
windows test
dentiny Jan 29, 2025
9aa55db
more TSAN comment
dentiny Jan 29, 2025
6468e62
fix windows
dentiny Jan 29, 2025
39b512a
remove
dentiny Jan 29, 2025
2316eab
unset logger to destruct
dentiny Jan 30, 2025
d72927b
Merge branch 'hjiang/boost-windows-jan-23' of github.com:dentiny/ray …
dentiny Jan 30, 2025
7d410fd
todo
dentiny Jan 30, 2025
ccd438f
Merge branch 'master' into hjiang/boost-windows-jan-23
dentiny Jan 31, 2025
d1c3c00
exit hook to cleanup
dentiny Jan 31, 2025
f359d4c
hook
dentiny Jan 31, 2025
39b3498
Merge branch 'hjiang/boost-windows-jan-23' of github.com:dentiny/ray …
dentiny Jan 31, 2025
cb3d01d
cleanup
dentiny Jan 31, 2025
740a886
Merge branch 'master' into hjiang/boost-windows-jan-23
dentiny Feb 1, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
remove unnecessary cross-platform
Signed-off-by: dentiny <[email protected]>
dentiny committed Jan 24, 2025
commit ad89e4ec980a7e4b69e1fdc318927678acbe04b2
65 changes: 17 additions & 48 deletions src/ray/util/pipe_logger.cc
Original file line number Diff line number Diff line change
@@ -147,7 +147,6 @@ bool ShouldUsePipeStream(const StreamRedirectionOption &stream_redirect_opt) {
stream_redirect_opt.tee_to_stderr;
}

#if defined(__APPLE__) || defined(__linux__)
RedirectionFileHandle OpenFileForRedirection(const std::string &file_path) {
boost::iostreams::file_descriptor_sink sink{file_path, std::ios_base::out};
auto handle = sink.handle();
@@ -162,35 +161,6 @@ RedirectionFileHandle OpenFileForRedirection(const std::string &file_path) {
return RedirectionFileHandle{
handle, std::move(ostream), std::move(flush_fn), std::move(close_fn)};
}
#elif defined(_WIN32)
#include <windows.h>
RedirectionFileHandle OpenFileForRedirection(const std::string &file_path) {
HANDLE file_handle = CreateFile(file_path.c_str(),
GENERIC_WRITE,
0, // No sharing
NULL, // Default security attributes
CREATE_ALWAYS, // Always create a new file
FILE_ATTRIBUTE_NORMAL, // Normal file attributes
NULL // No template file
);
RAY_CHECK(file_handle != INVALID_HANDLE_VALUE)
<< "Fails to open file " << file_path << " with error "
<< std::to_string(GetLastError());

auto flush_fn = [file_handle]() {
RAY_CHECK(FlushFileBuffers(file_handle))
<< "Failed to flush data to disk with error: " << std::to_string(GetLastError());
};
auto close_fn = [file_handle]() {
RAY_CHECK(FlushFileBuffers(file_handle))
<< "Failed to flush data to disk with error: " << std::to_string(GetLastError());
RAY_CHECK(CloseHandle(file_handle))
<< "Failed to close file with error: " << std::to_string(GetLastError());
};
return RedirectionFileHandle{file_handle, std::move(flush_fn), std::move(close_fn)};
}
#endif

} // namespace

RedirectionFileHandle CreateRedirectionFileHandle(
@@ -210,7 +180,7 @@ RedirectionFileHandle CreateRedirectionFileHandle(
// Invoked after flush and close finished.
auto on_close_completion = [promise = promise]() { promise->set_value(); };

StreamSink std_stream_fd{};
StreamSink std_stream_sink{};

#if defined(__APPLE__) || defined(__linux__)
if (stream_redirect_opt.tee_to_stdout) {
@@ -219,7 +189,7 @@ RedirectionFileHandle CreateRedirectionFileHandle(

boost::iostreams::file_descriptor_sink sink{
duped_stdout_fd, /*file_descriptor_flags=*/boost::iostreams::close_handle};
std_stream_fd.stdout_sink = std::make_shared<
std_stream_sink.stdout_sink = std::make_shared<
boost::iostreams::stream<boost::iostreams::file_descriptor_sink>>(
std::move(sink));
}
@@ -229,13 +199,12 @@ RedirectionFileHandle CreateRedirectionFileHandle(

boost::iostreams::file_descriptor_sink sink{
duped_stderr_fd, /*file_descriptor_flags=*/boost::iostreams::close_handle};
std_stream_fd.stderr_sink = std::make_shared<
std_stream_sink.stderr_sink = std::make_shared<
boost::iostreams::stream<boost::iostreams::file_descriptor_sink>>(
std::move(sink));
}

int pipefd[2] = {0};
// TODO(hjiang): We shoud have our own syscall macro.
RAY_CHECK_EQ(pipe(pipefd), 0);
int read_fd = pipefd[0];
int write_fd = pipefd[1];
@@ -246,34 +215,34 @@ RedirectionFileHandle CreateRedirectionFileHandle(

#elif defined(_WIN32)
if (tream_redirect_opt.tee_to_stdout) {
int duped_stderr_fd = -1;
HANDLE duped_stderr_handle;
BOOL result = DuplicateHandle(GetCurrentProcess(),
GetStdHandle(STD_OUTPUT_HANDLE),
GetCurrentProcess(),
&duped_stderr_fd,
&duped_stderr_handle,
0,
FALSE,
DUPLICATE_SAME_ACCESS);
RAY_CHECK(result) << "Fails to duplicate stdout handle";

boost::iostreams::file_descriptor_sink sink{duped_stderr_fd, std::ios_base::out};
std_stream_fd.stdout = std::make_shared<
boost::iostreams::file_descriptor_sink sink{duped_stderr_handle, std::ios_base::out};
std_stream_sink.stdout_sink = std::make_shared<
boost::iostreams::stream<boost::iostreams::file_descriptor_sink>>(
std::move(sink));
}
if (tream_redirect_opt.tee_to_stderr) {
int duped_stderr_fd = -1;
HANDLE duped_stderr_handle;
BOOL result = DuplicateHandle(GetCurrentProcess(),
GetStdHandle(STD_ERROR_HANDLE),
GetCurrentProcess(),
&duped_stderr_fd,
&duped_stderr_handle,
0,
FALSE,
DUPLICATE_SAME_ACCESS);
RAY_CHECK(result) << "Fails to duplicate stderr handle";

boost::iostreams::file_descriptor_sink sink{duped_stderr_fd, std::ios_base::out};
std_stream_fd.stderr_sink = std::make_shared<
boost::iostreams::file_descriptor_sink sink{duped_stderr_handle, std::ios_base::out};
std_stream_sink.stderr_sink = std::make_shared<
boost::iostreams::stream<boost::iostreams::file_descriptor_sink>>(
std::move(sink));
}
@@ -307,12 +276,12 @@ RedirectionFileHandle CreateRedirectionFileHandle(
// newliner, if any.
auto write_fn = [logger,
stream_redirect_opt = stream_redirect_opt,
std_stream_fd = std_stream_fd](std::string content) {
std_stream_sink = std_stream_sink](std::string content) {
if (stream_redirect_opt.tee_to_stdout) {
std_stream_fd.stdout_sink->write(content.data(), content.length());
std_stream_sink.stdout_sink->write(content.data(), content.length());
}
if (stream_redirect_opt.tee_to_stderr) {
std_stream_fd.stderr_sink->write(content.data(), content.length());
std_stream_sink.stderr_sink->write(content.data(), content.length());
}
if (logger != nullptr) {
// spdlog adds newliner for every content, no need to maintan the application-passed
@@ -325,15 +294,15 @@ RedirectionFileHandle CreateRedirectionFileHandle(
};
auto flush_fn = [logger,
stream_redirect_opt = stream_redirect_opt,
std_stream_fd = std_stream_fd]() {
std_stream_sink = std_stream_sink]() {
if (logger != nullptr) {
logger->flush();
}
if (stream_redirect_opt.tee_to_stdout) {
std_stream_fd.stdout_sink->flush();
std_stream_sink.stdout_sink->flush();
}
if (stream_redirect_opt.tee_to_stderr) {
std_stream_fd.stderr_sink->flush();
std_stream_sink.stderr_sink->flush();
}
};