From 3886cdb22cfa608302d4768d17e9f8be6a3614e7 Mon Sep 17 00:00:00 2001 From: Lonnie Liu <95255098+aslonnie@users.noreply.github.com> Date: Mon, 3 Feb 2025 21:00:17 -0800 Subject: [PATCH] Revert "[core] Compatibile function for file descriptor" (#50213) Reverts ray-project/ray#50170 --- src/ray/util/BUILD | 5 -- src/ray/util/compat.cc | 84 ------------------------ src/ray/util/compat.h | 30 --------- src/ray/util/stream_redirection_utils.cc | 16 +++-- src/ray/util/tests/BUILD | 12 ---- src/ray/util/tests/compat_test.cc | 52 --------------- 6 files changed, 12 insertions(+), 187 deletions(-) delete mode 100644 src/ray/util/compat.cc delete mode 100644 src/ray/util/tests/compat_test.cc diff --git a/src/ray/util/BUILD b/src/ray/util/BUILD index e8bebb3332f2..7074971c0db7 100644 --- a/src/ray/util/BUILD +++ b/src/ray/util/BUILD @@ -242,11 +242,6 @@ ray_cc_library( ray_cc_library( name = "compat", hdrs = ["compat.h"], - srcs = ["compat.cc"], - deps = [ - ":logging", - "//src/ray/common:status", - ], ) ray_cc_library( diff --git a/src/ray/util/compat.cc b/src/ray/util/compat.cc deleted file mode 100644 index 183b217cd86b..000000000000 --- a/src/ray/util/compat.cc +++ /dev/null @@ -1,84 +0,0 @@ -// Copyright 2025 The Ray Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "ray/util/compat.h" - -#include - -#include "ray/util/logging.h" - -#if defined(__APPLE__) || defined(__linux__) -#include -#elif defined(_WIN32) -#include -#endif - -namespace ray { - -#if defined(__APPLE__) || defined(__linux__) -Status CompleteWrite(MEMFD_TYPE_NON_UNIQUE fd, const char *data, size_t len) { - const ssize_t ret = write(fd, data, len); - if (ret == -1) { - return Status::IOError("") << "Fails to write to file because " << strerror(errno); - } - if (ret != static_cast(len)) { - return Status::IOError("") << "Fails to write all requested bytes, requests to write " - << len << " bytes, but actually write " << ret << " bytes"; - } - return Status::OK(); -} -Status Flush(MEMFD_TYPE_NON_UNIQUE fd) { - const int ret = fdatasync(fd); - RAY_CHECK(ret != -1 || errno != EIO) << "Fails to flush to file " << strerror(errno); - if (ret == -1) { - return Status::IOError("") << "Fails to flush file because " << strerror(errno); - } - return Status::OK(); -} -Status Close(MEMFD_TYPE_NON_UNIQUE fd) { - const int ret = close(fd); - if (ret != 0) { - return Status::IOError("") << "Fails to flush file because " << strerror(errno); - } - return Status::OK(); -} -#elif defined(_WIN32) -Status CompleteWrite(MEMFD_TYPE_NON_UNIQUE fd, const char *data, size_t len) { - DWORD bytes_written; - BOOL success = WriteFile(fd, data, (DWORD)len, &bytes_written, NULL); - if (!success) { - return Status::IOError("") << "Fails to write to file"; - } - if ((DWORD)len != bytes_written) { - return Status::IOError("") << "Fails to write all requested bytes, requests to write " - << len << " bytes, but actually write " << bytes_written - << " bytes"; - } - return Status::OK(); -} -Status Flush(MEMFD_TYPE_NON_UNIQUE fd) { - if (!FlushFileBuffers(fd)) { - return Status::IOError("") << "Fails to flush file"; - } - return Status::OK(); -} -Status Close(MEMFD_TYPE_NON_UNIQUE fd) { - if (!CloseHandle(fd)) { - return Status::IOError("") << "Fails to close file handle"; - } - return Status::OK(); -} -#endif - -} // namespace ray diff --git a/src/ray/util/compat.h b/src/ray/util/compat.h index dd9d426048e2..371192331084 100644 --- a/src/ray/util/compat.h +++ b/src/ray/util/compat.h @@ -31,8 +31,6 @@ #pragma once -#include "ray/common/status.h" - // Workaround for multithreading on XCode 9, see // https://issues.apache.org/jira/browse/ARROW-1622 and // https://github.com/tensorflow/tensorflow/issues/13220#issuecomment-331579775 @@ -49,10 +47,6 @@ mach_port_t pthread_mach_thread_np(pthread_t); #endif /* _MACH_PORT_T */ #endif /* __APPLE__ */ -#if defined(__APPLE__) || defined(__linux__) -#include -#endif - #ifdef _WIN32 #ifndef _WINDOWS_ #ifndef WIN32_LEAN_AND_MEAN // Sorry for the inconvenience. Please include any related @@ -78,27 +72,3 @@ mach_port_t pthread_mach_thread_np(pthread_t); // since fd values can get re-used by the operating system. #define MEMFD_TYPE std::pair #define INVALID_UNIQUE_FD_ID 0 - -namespace ray { -#if defined(__APPLE__) || defined(__linux__) -inline int GetStdoutFd() { return STDOUT_FILENO; } -inline int GetStderrFd() { return STDERR_FILENO; } -inline MEMFD_TYPE_NON_UNIQUE GetStdoutHandle() { return STDOUT_FILENO; } -inline MEMFD_TYPE_NON_UNIQUE GetStderrHandle() { return STDERR_FILENO; } -#elif defined(_WIN32) -inline int GetStdoutFd() { return _fileno(stdout); } -inline int GetStderrFd() { return _fileno(stderr); } -inline MEMFD_TYPE_NON_UNIQUE GetStdoutHandle() { return GetStdHandle(STD_OUTPUT_HANDLE); } -inline MEMFD_TYPE_NON_UNIQUE GetStderrHandle() { return GetStdHandle(STD_ERROR_HANDLE); } -#endif - -// Write the whole content into file descriptor, if any error happens, or actual written -// content is less than expected, IO error status will be returned. -Status CompleteWrite(MEMFD_TYPE_NON_UNIQUE fd, const char *data, size_t len); -// Flush the given file descriptor, if EIO happens, error message is logged and process -// exits directly. Reference to fsyncgate: https://wiki.postgresql.org/wiki/Fsync_Errors -Status Flush(MEMFD_TYPE_NON_UNIQUE fd); -// Close the given file descriptor, if any error happens, IO error status will be -// returned. -Status Close(MEMFD_TYPE_NON_UNIQUE fd); -} // namespace ray diff --git a/src/ray/util/stream_redirection_utils.cc b/src/ray/util/stream_redirection_utils.cc index e67d32c9dc7d..7812065f55cd 100644 --- a/src/ray/util/stream_redirection_utils.cc +++ b/src/ray/util/stream_redirection_utils.cc @@ -32,6 +32,14 @@ namespace ray { namespace { +#if defined(__APPLE__) || defined(__linux__) +int GetStdoutHandle() { return STDOUT_FILENO; } +int GetStderrHandle() { return STDERR_FILENO; } +#elif defined(_WIN32) +int GetStdoutHandle() { return _fileno(stdout); } +int GetStderrHandle() { return _fileno(stderr); } +#endif + // TODO(hjiang): Revisit later, should be able to save some heap allocation with // absl::InlinedVector. // @@ -81,12 +89,12 @@ void FlushOnRedirectedStream(int stream_fd) { } // namespace void RedirectStdout(const StreamRedirectionOption &opt) { - RedirectStream(GetStdoutFd(), opt); + RedirectStream(GetStdoutHandle(), opt); } void RedirectStderr(const StreamRedirectionOption &opt) { - RedirectStream(GetStderrFd(), opt); + RedirectStream(GetStderrHandle(), opt); } -void FlushOnRedirectedStdout() { FlushOnRedirectedStream(GetStdoutFd()); } -void FlushOnRedirectedStderr() { FlushOnRedirectedStream(GetStderrFd()); } +void FlushOnRedirectedStdout() { FlushOnRedirectedStream(GetStdoutHandle()); } +void FlushOnRedirectedStderr() { FlushOnRedirectedStream(GetStderrHandle()); } } // namespace ray diff --git a/src/ray/util/tests/BUILD b/src/ray/util/tests/BUILD index b1778be6cefa..6c76d6be8fc8 100644 --- a/src/ray/util/tests/BUILD +++ b/src/ray/util/tests/BUILD @@ -287,15 +287,3 @@ ray_cc_test( size = "small", tags = ["team:core"], ) - -ray_cc_test( - name = "compat_test", - srcs = ["compat_test.cc"], - deps = [ - "//src/ray/util:compat", - "//src/ray/util:filesystem", - "@com_google_googletest//:gtest_main", - ], - size = "small", - tags = ["team:core"], -) diff --git a/src/ray/util/tests/compat_test.cc b/src/ray/util/tests/compat_test.cc deleted file mode 100644 index 8c3646a6e713..000000000000 --- a/src/ray/util/tests/compat_test.cc +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright 2025 The Ray Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "ray/util/compat.h" - -#include - -#include - -#include "ray/common/status_or.h" -#include "ray/util/filesystem.h" - -#if defined(__APPLE__) || defined(__linux__) -#include -#include -#include -#include -#elif defined(_WIN32) -#include -#endif - -namespace ray { - -namespace { - -constexpr std::string_view kContent = "helloworld"; - -TEST(CompatTest, WriteTest) { - MEMFD_TYPE_NON_UNIQUE fd = GetStdoutHandle(); - - testing::internal::CaptureStdout(); - RAY_CHECK_OK(CompleteWrite(fd, kContent.data(), kContent.length())); - RAY_CHECK_OK(Flush(fd)); - const std::string stdout_content = testing::internal::GetCapturedStdout(); - - EXPECT_EQ(stdout_content, kContent); -} - -} // namespace - -} // namespace ray