This is an automated email from the ASF dual-hosted git repository.
dataroaring pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.0 by this push:
new 31aa1a6d8cf [enhance](fs)Improve FS error code (#32427)
31aa1a6d8cf is described below
commit 31aa1a6d8cf7525f8964d733aad19c3efb62f8cd
Author: plat1ko <[email protected]>
AuthorDate: Tue Mar 19 22:19:52 2024 +0800
[enhance](fs)Improve FS error code (#32427)
Co-authored-by: Kang <[email protected]>
---
be/src/common/status.h | 1 +
be/src/io/fs/err_utils.cpp | 36 ++++++++++++++++++
be/src/io/fs/err_utils.h | 5 +++
be/src/io/fs/local_file_reader.cpp | 13 +++----
be/src/io/fs/local_file_system.cpp | 75 ++++++++++++++++----------------------
be/src/io/fs/local_file_writer.cpp | 42 ++++++++++-----------
6 files changed, 99 insertions(+), 73 deletions(-)
diff --git a/be/src/common/status.h b/be/src/common/status.h
index 555ee73443d..32f60e72bae 100644
--- a/be/src/common/status.h
+++ b/be/src/common/status.h
@@ -83,6 +83,7 @@ E(COPY_FILE_ERROR, -121);
E(FILE_ALREADY_EXIST, -122);
E(BAD_CAST, -123);
E(ARITHMETIC_OVERFLOW_ERRROR, -124);
+E(PERMISSION_DENIED, -125);
E(CALL_SEQUENCE_ERROR, -202);
E(BUFFER_OVERFLOW, -204);
E(CONFIG_ERROR, -205);
diff --git a/be/src/io/fs/err_utils.cpp b/be/src/io/fs/err_utils.cpp
index 648a358850f..05817de1852 100644
--- a/be/src/io/fs/err_utils.cpp
+++ b/be/src/io/fs/err_utils.cpp
@@ -27,6 +27,8 @@
#include "io/fs/hdfs.h"
namespace doris {
+using namespace ErrorCode;
+
namespace io {
std::string errno_to_str() {
@@ -73,5 +75,39 @@ std::string glob_err_to_str(int code) {
return fmt::format("({}), {}", code, msg);
}
+Status localfs_error(const std::error_code& ec, std::string_view msg) {
+ if (ec == std::errc::io_error) {
+ return Status::Error<IO_ERROR, false>(msg);
+ } else if (ec == std::errc::no_such_file_or_directory) {
+ return Status::Error<NOT_FOUND, false>(msg);
+ } else if (ec == std::errc::file_exists) {
+ return Status::Error<ALREADY_EXIST, false>(msg);
+ } else if (ec == std::errc::no_space_on_device) {
+ return Status::Error<DISK_REACH_CAPACITY_LIMIT, false>(msg);
+ } else if (ec == std::errc::permission_denied) {
+ return Status::Error<PERMISSION_DENIED, false>(msg);
+ } else {
+ return Status::Error<ErrorCode::INTERNAL_ERROR, false>("{}: {}", msg,
ec.message());
+ }
+}
+
+Status localfs_error(int posix_errno, std::string_view msg) {
+ switch (posix_errno) {
+ case EIO:
+ return Status::Error<IO_ERROR, false>(msg);
+ case ENOENT:
+ return Status::Error<NOT_FOUND, false>(msg);
+ case EEXIST:
+ return Status::Error<ALREADY_EXIST, false>(msg);
+ case ENOSPC:
+ return Status::Error<DISK_REACH_CAPACITY_LIMIT, false>(msg);
+ case EACCES:
+ return Status::Error<PERMISSION_DENIED, false>(msg);
+ default:
+ return Status::Error<ErrorCode::INTERNAL_ERROR, false>("{}: {}", msg,
+
std::strerror(posix_errno));
+ }
+}
+
} // namespace io
} // namespace doris
diff --git a/be/src/io/fs/err_utils.h b/be/src/io/fs/err_utils.h
index 971596fab19..42a455fcf2d 100644
--- a/be/src/io/fs/err_utils.h
+++ b/be/src/io/fs/err_utils.h
@@ -20,6 +20,8 @@
#include <string>
#include <system_error>
+#include "common/status.h"
+
namespace doris {
namespace io {
@@ -28,5 +30,8 @@ std::string errcode_to_str(const std::error_code& ec);
std::string hdfs_error();
std::string glob_err_to_str(int code);
+Status localfs_error(const std::error_code& ec, std::string_view msg);
+Status localfs_error(int posix_errno, std::string_view msg);
+
} // namespace io
} // namespace doris
diff --git a/be/src/io/fs/local_file_reader.cpp
b/be/src/io/fs/local_file_reader.cpp
index d0054a1c970..7a29b39e0fb 100644
--- a/be/src/io/fs/local_file_reader.cpp
+++ b/be/src/io/fs/local_file_reader.cpp
@@ -68,9 +68,7 @@ Status LocalFileReader::close() {
}
#endif
if (-1 == res) {
- std::string err = errno_to_str();
- LOG(WARNING) << fmt::format("failed to close {}: {}",
_path.native(), err);
- return Status::IOError("failed to close {}: {}", _path.native(),
err);
+ return localfs_error(errno, fmt::format("failed to close {}",
_path.native()));
}
_fd = -1;
}
@@ -81,8 +79,9 @@ Status LocalFileReader::read_at_impl(size_t offset, Slice
result, size_t* bytes_
const IOContext* /*io_ctx*/) {
DCHECK(!closed());
if (offset > _file_size) {
- return Status::IOError("offset exceeds file size(offset: {}, file
size: {}, path: {})",
- offset, _file_size, _path.native());
+ return Status::InternalError(
+ "offset exceeds file size(offset: {}, file size: {}, path:
{})", offset, _file_size,
+ _path.native());
}
size_t bytes_req = result.size;
char* to = result.data;
@@ -92,10 +91,10 @@ Status LocalFileReader::read_at_impl(size_t offset, Slice
result, size_t* bytes_
while (bytes_req != 0) {
auto res = ::pread(_fd, to, bytes_req, offset);
if (UNLIKELY(-1 == res && errno != EINTR)) {
- return Status::IOError("cannot read from {}: {}", _path.native(),
std::strerror(errno));
+ return localfs_error(errno, fmt::format("failed to read {}",
_path.native()));
}
if (UNLIKELY(res == 0)) {
- return Status::IOError("cannot read from {}: unexpected EOF",
_path.native());
+ return Status::InternalError("cannot read from {}: unexpected
EOF", _path.native());
}
if (res > 0) {
to += res;
diff --git a/be/src/io/fs/local_file_system.cpp
b/be/src/io/fs/local_file_system.cpp
index f7ac1e77e16..4e0c9aaefae 100644
--- a/be/src/io/fs/local_file_system.cpp
+++ b/be/src/io/fs/local_file_system.cpp
@@ -62,7 +62,7 @@ Status LocalFileSystem::create_file_impl(const Path& file,
FileWriterPtr* writer
const FileWriterOptions* opts) {
int fd = ::open(file.c_str(), O_TRUNC | O_WRONLY | O_CREAT | O_CLOEXEC,
0666);
if (-1 == fd) {
- return Status::IOError("failed to open {}: {}", file.native(),
errno_to_str());
+ return localfs_error(errno, fmt::format("failed to create file {}",
file.native()));
}
bool sync_data = opts != nullptr ? opts->sync_file_data : true;
*writer = std::make_unique<LocalFileWriter>(
@@ -81,7 +81,7 @@ Status LocalFileSystem::open_file_impl(const FileDescription&
file_desc, const P
int fd = -1;
RETRY_ON_EINTR(fd, open(abs_path.c_str(), O_RDONLY));
if (fd < 0) {
- return Status::IOError("failed to open {}: {}", abs_path.native(),
errno_to_str());
+ return localfs_error(errno, fmt::format("failed to open {}",
abs_path.native()));
}
*reader = std::make_shared<LocalFileReader>(
std::move(abs_path), fsize, fd,
@@ -94,13 +94,13 @@ Status LocalFileSystem::create_directory_impl(const Path&
dir, bool failed_if_ex
bool exists = true;
RETURN_IF_ERROR(exists_impl(dir, &exists));
if (exists) {
- return Status::IOError("failed to create {}, already exists",
dir.native());
+ return Status::AlreadyExist("failed to create {}, already exists",
dir.native());
}
}
std::error_code ec;
std::filesystem::create_directories(dir, ec);
if (ec) {
- return Status::IOError("failed to create {}: {}", dir.native(),
errcode_to_str(ec));
+ return localfs_error(ec, fmt::format("failed to create {}",
dir.native()));
}
return Status::OK();
}
@@ -112,12 +112,12 @@ Status LocalFileSystem::delete_file_impl(const Path&
file) {
return Status::OK();
}
if (!std::filesystem::is_regular_file(file)) {
- return Status::IOError("failed to delete {}, not a file",
file.native());
+ return Status::InternalError("failed to delete {}, not a file",
file.native());
}
std::error_code ec;
std::filesystem::remove(file, ec);
if (ec) {
- return Status::IOError("failed to delete {}: {}", file.native(),
errcode_to_str(ec));
+ return localfs_error(ec, fmt::format("failed to delete {}",
file.native()));
}
return Status::OK();
}
@@ -129,12 +129,12 @@ Status LocalFileSystem::delete_directory_impl(const Path&
dir) {
return Status::OK();
}
if (!std::filesystem::is_directory(dir)) {
- return Status::IOError("failed to delete {}, not a directory",
dir.native());
+ return Status::InternalError("failed to delete {}, not a dir",
dir.native());
}
std::error_code ec;
std::filesystem::remove_all(dir, ec);
if (ec) {
- return Status::IOError("failed to delete {}: {}", dir.native(),
errcode_to_str(ec));
+ return localfs_error(ec, fmt::format("failed to delete {}",
dir.native()));
}
return Status::OK();
}
@@ -174,7 +174,7 @@ Status LocalFileSystem::file_size_impl(const Path& file,
int64_t* file_size) con
std::error_code ec;
*file_size = std::filesystem::file_size(file, ec);
if (ec) {
- return Status::IOError("failed to get file size {}: {}",
file.native(), errcode_to_str(ec));
+ return localfs_error(ec, fmt::format("failed to check exists {}",
file.native()));
}
return Status::OK();
}
@@ -208,10 +208,10 @@ Status LocalFileSystem::list_impl(const Path& dir, bool
only_file, std::vector<F
} catch (const std::filesystem::filesystem_error& e) {
// although `directory_iterator(dir, ec)` does not throw an exception,
// it may throw an exception during iterator++, so we need to catch
the exception here
- return Status::IOError("failed to list {}: {}", dir.native(),
e.what());
+ return localfs_error(e.code(), fmt::format("failed to list {}",
dir.native()));
}
if (ec) {
- return Status::IOError("failed to list {}: {}", dir.native(),
errcode_to_str(ec));
+ return localfs_error(ec, fmt::format("failed to list {}",
dir.native()));
}
return Status::OK();
}
@@ -220,8 +220,8 @@ Status LocalFileSystem::rename_impl(const Path& orig_name,
const Path& new_name)
std::error_code ec;
std::filesystem::rename(orig_name, new_name, ec);
if (ec) {
- return Status::IOError("failed to rename {} to {}: {}",
orig_name.native(),
- new_name.native(), errcode_to_str(ec));
+ return localfs_error(ec, fmt::format("failed to rename {} to {}",
orig_name.native(),
+ new_name.native()));
}
return Status::OK();
}
@@ -238,8 +238,8 @@ Status LocalFileSystem::link_file(const Path& src, const
Path& dest) {
Status LocalFileSystem::link_file_impl(const Path& src, const Path& dest) {
if (::link(src.c_str(), dest.c_str()) != 0) {
- return Status::IOError("failed to create hard link from {} to {}: {}",
src.native(),
- dest.native(), errno_to_str());
+ return localfs_error(errno, fmt::format("failed to create hard link
from {} to {}",
+ src.native(), dest.native()));
}
return Status::OK();
}
@@ -248,8 +248,7 @@ Status LocalFileSystem::canonicalize(const Path& path,
std::string* real_path) {
std::error_code ec;
Path res = std::filesystem::canonical(path, ec);
if (ec) {
- return Status::IOError("failed to canonicalize path {}: {}",
path.native(),
- errcode_to_str(ec));
+ return localfs_error(ec, fmt::format("failed to canonicalize {}",
path.native()));
}
*real_path = res.string();
return Status::OK();
@@ -260,10 +259,7 @@ Status LocalFileSystem::is_directory(const Path& path,
bool* res) {
std::error_code ec;
*res = std::filesystem::is_directory(tmp_path, ec);
if (ec) {
- LOG(WARNING) << fmt::format("failed to check is dir {}: {}",
tmp_path.native(),
- errcode_to_str(ec));
- return Status::IOError("failed to check is dir {}: {}",
tmp_path.native(),
- errcode_to_str(ec));
+ return localfs_error(ec, fmt::format("failed to check is dir {}",
tmp_path.native()));
}
return Status::OK();
}
@@ -276,15 +272,15 @@ Status LocalFileSystem::md5sum(const Path& file,
std::string* md5sum) {
Status LocalFileSystem::md5sum_impl(const Path& file, std::string* md5sum) {
int fd = open(file.c_str(), O_RDONLY);
if (fd < 0) {
- return Status::IOError("failed to open file for md5sum {}: {}",
file.native(),
- errno_to_str());
+ return localfs_error(errno,
+ fmt::format("failed to open file for md5sum {}",
file.native()));
}
struct stat statbuf;
if (fstat(fd, &statbuf) < 0) {
- std::string err = errno_to_str();
+ int err = errno;
close(fd);
- return Status::InternalError("failed to stat file {}: {}",
file.native(), err);
+ return localfs_error(err, fmt::format("failed to stat file {}",
file.native()));
}
size_t file_len = statbuf.st_size;
CONSUME_THREAD_MEM_TRACKER(file_len);
@@ -330,16 +326,9 @@ Status LocalFileSystem::mtime(const Path& file, time_t*
m_time) {
}
Status LocalFileSystem::mtime_impl(const Path& file, time_t* m_time) {
- int fd = open(file.c_str(), O_RDONLY);
- if (fd < 0) {
- return Status::IOError("failed to get mtime for file {}: {}",
file.native(),
- errno_to_str());
- }
-
- Defer defer {[&]() { close(fd); }};
struct stat statbuf;
- if (fstat(fd, &statbuf) < 0) {
- return Status::IOError("failed to stat file {}: {}", file.native(),
errno_to_str());
+ if (stat(file.c_str(), &statbuf) < 0) {
+ return localfs_error(errno, fmt::format("failed to stat file {}",
file.native()));
}
*m_time = statbuf.st_mtime;
return Status::OK();
@@ -364,8 +353,8 @@ Status LocalFileSystem::get_space_info_impl(const Path&
path, size_t* capacity,
std::error_code ec;
std::filesystem::space_info info = std::filesystem::space(path, ec);
if (ec) {
- return Status::IOError("failed to get available space for path {}:
{}", path.native(),
- errcode_to_str(ec));
+ return localfs_error(
+ ec, fmt::format("failed to get available space for path {}",
path.native()));
}
*capacity = info.capacity;
*available = info.available;
@@ -382,8 +371,8 @@ Status LocalFileSystem::copy_path_impl(const Path& src,
const Path& dest) {
std::error_code ec;
std::filesystem::copy(src, dest, std::filesystem::copy_options::recursive,
ec);
if (ec) {
- return Status::IOError("failed to copy from {} to {}: {}",
src.native(), dest.native(),
- errcode_to_str(ec));
+ return localfs_error(
+ ec, fmt::format("failed to copy from {} to {}", src.native(),
dest.native()));
}
return Status::OK();
}
@@ -429,8 +418,9 @@ Status LocalFileSystem::read_file_to_string(const Path&
file, std::string* conte
size_t bytes_read = 0;
RETURN_IF_ERROR(file_reader->read_at(0, {*content}, &bytes_read));
if (bytes_read != file_size) {
- return Status::IOError("failed to read file {} to string. bytes read:
{}, file size: {}",
- file.native(), bytes_read, file_size);
+ return Status::InternalError(
+ "failed to read file {} to string. bytes read: {}, file size:
{}", file.native(),
+ bytes_read, file_size);
}
return file_reader->close();
}
@@ -479,7 +469,7 @@ Status LocalFileSystem::_glob(const std::string& pattern,
std::vector<std::strin
int rc = glob(pattern.c_str(), GLOB_TILDE, NULL, &glob_result);
if (rc != 0) {
globfree(&glob_result);
- return Status::IOError("failed to glob {}: {}", pattern,
glob_err_to_str(rc));
+ return Status::InternalError("failed to glob {}: {}", pattern,
glob_err_to_str(rc));
}
for (size_t i = 0; i < glob_result.gl_pathc; ++i) {
@@ -499,8 +489,7 @@ Status LocalFileSystem::permission_impl(const Path& file,
std::filesystem::perms
std::error_code ec;
std::filesystem::permissions(file, prms, ec);
if (ec) {
- return Status::IOError("failed to change file permission {}: {}",
file.native(),
- errcode_to_str(ec));
+ return localfs_error(ec, fmt::format("failed to change file permission
{}", file.native()));
}
return Status::OK();
}
diff --git a/be/src/io/fs/local_file_writer.cpp
b/be/src/io/fs/local_file_writer.cpp
index 893c6ea9034..635c5b396f2 100644
--- a/be/src/io/fs/local_file_writer.cpp
+++ b/be/src/io/fs/local_file_writer.cpp
@@ -36,36 +36,33 @@
#include "common/compiler_util.h" // IWYU pragma: keep
#include "common/status.h"
#include "gutil/macros.h"
+#include "io/fs/err_utils.h"
#include "io/fs/file_writer.h"
#include "io/fs/local_file_system.h"
#include "io/fs/path.h"
#include "util/doris_metrics.h"
-namespace doris {
-namespace detail {
+namespace doris::io {
-Status sync_dir(const io::Path& dirname) {
+namespace {
+Status sync_dir(const Path& dirname) {
int fd;
RETRY_ON_EINTR(fd, ::open(dirname.c_str(), O_DIRECTORY | O_RDONLY));
if (-1 == fd) {
- return Status::IOError("cannot open {}: {}", dirname.native(),
std::strerror(errno));
+ return localfs_error(errno, fmt::format("failed to open {}",
dirname.native()));
}
#ifdef __APPLE__
if (fcntl(fd, F_FULLFSYNC) < 0) {
- return Status::IOError("cannot sync {}: {}", dirname.native(),
std::strerror(errno));
- }
#else
if (0 != ::fdatasync(fd)) {
- return Status::IOError("cannot fdatasync {}: {}", dirname.native(),
std::strerror(errno));
- }
#endif
+ return localfs_error(errno, fmt::format("failed to sync {}",
dirname.native()));
+ }
::close(fd);
return Status::OK();
}
-} // namespace detail
-
-namespace io {
+} // namespace
LocalFileWriter::LocalFileWriter(Path path, int fd, FileSystemSPtr fs, bool
sync_data)
: FileWriter(std::move(path), fs), _fd(fd), _sync_data(sync_data) {
@@ -94,7 +91,9 @@ Status LocalFileWriter::abort() {
}
Status LocalFileWriter::appendv(const Slice* data, size_t data_cnt) {
- DCHECK(!_closed);
+ if (_closed) [[unlikely]] {
+ return Status::InternalError("append to closed file: {}",
_path.native());
+ }
_dirty = true;
// Convert the results into the iovec vector to request
@@ -115,7 +114,7 @@ Status LocalFileWriter::appendv(const Slice* data, size_t
data_cnt) {
ssize_t res;
RETRY_ON_EINTR(res, ::writev(_fd, iov + completed_iov, iov_count));
if (UNLIKELY(res < 0)) {
- return Status::IOError("cannot write to {}: {}", _path.native(),
std::strerror(errno));
+ return localfs_error(errno, fmt::format("failed to write {}",
_path.native()));
}
if (LIKELY(res == n_left)) {
@@ -155,7 +154,7 @@ Status LocalFileWriter::write_at(size_t offset, const
Slice& data) {
while (bytes_req != 0) {
auto res = ::pwrite(_fd, from, bytes_req, offset);
if (-1 == res && errno != EINTR) {
- return Status::IOError("cannot write to {}: {}", _path.native(),
std::strerror(errno));
+ return localfs_error(errno, fmt::format("failed to write {}",
_path.native()));
}
if (res > 0) {
from += res;
@@ -171,7 +170,7 @@ Status LocalFileWriter::finalize() {
#if defined(__linux__)
int flags = SYNC_FILE_RANGE_WRITE;
if (sync_file_range(_fd, 0, 0, flags) < 0) {
- return Status::IOError("cannot sync {}: {}", _path.native(),
std::strerror(errno));
+ return localfs_error(errno, fmt::format("failed to finalize {}",
_path.native()));
}
#endif
}
@@ -186,14 +185,12 @@ Status LocalFileWriter::_close(bool sync) {
if (sync && _dirty) {
#ifdef __APPLE__
if (fcntl(_fd, F_FULLFSYNC) < 0) {
- return Status::IOError("cannot sync {}: {}", _path.native(),
std::strerror(errno));
- }
#else
if (0 != ::fdatasync(_fd)) {
- return Status::IOError("cannot fdatasync {}: {}", _path.native(),
std::strerror(errno));
- }
#endif
- RETURN_IF_ERROR(detail::sync_dir(_path.parent_path()));
+ return localfs_error(errno, fmt::format("failed to sync {}",
_path.native()));
+ }
+ RETURN_IF_ERROR(sync_dir(_path.parent_path()));
_dirty = false;
}
@@ -202,10 +199,9 @@ Status LocalFileWriter::_close(bool sync) {
DorisMetrics::instance()->local_bytes_written_total->increment(_bytes_appended);
if (0 != ::close(_fd)) {
- return Status::IOError("cannot close {}: {}", _path.native(),
std::strerror(errno));
+ return localfs_error(errno, fmt::format("failed to close {}",
_path.native()));
}
return Status::OK();
}
-} // namespace io
-} // namespace doris
+} // namespace doris::io
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]