From 8fdd809a7b8a9e8ee7cd2aac618646104f846266 Mon Sep 17 00:00:00 2001 From: plat1ko Date: Thu, 14 Mar 2024 21:55:21 +0800 Subject: [PATCH] [bug](s3) Fix object data is overwritten by empty object Fix wrong empty s3 file (#32252) --- be/src/io/fs/err_utils.cpp | 8 ++++---- be/src/io/fs/s3_file_writer.cpp | 11 +++++------ be/src/io/fs/s3_file_writer.h | 1 - 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/be/src/io/fs/err_utils.cpp b/be/src/io/fs/err_utils.cpp index 5688b888f4481a..35f4b79a304fa0 100644 --- a/be/src/io/fs/err_utils.cpp +++ b/be/src/io/fs/err_utils.cpp @@ -89,7 +89,7 @@ Status localfs_error(const std::error_code& ec, std::string_view msg) { } else if (ec == std::errc::permission_denied) { return Status::Error(msg); } else { - return Status::Error("{}: {}", msg, ec.message()); + return Status::Error("{}: {}", msg, ec.message()); } } @@ -106,8 +106,8 @@ Status localfs_error(int posix_errno, std::string_view msg) { case EACCES: return Status::Error(msg); default: - return Status::Error("{}: {}", msg, - std::strerror(posix_errno)); + return Status::Error("{}: {}", msg, + std::strerror(posix_errno)); } } @@ -122,7 +122,7 @@ Status s3fs_error(const Aws::S3::S3Error& err, std::string_view msg) { err.GetExceptionName(), err.GetMessage(), err.GetErrorType()); default: - return Status::Error( + return Status::Error( "{}: {} {} code={} type={}", msg, err.GetExceptionName(), err.GetMessage(), err.GetResponseCode(), err.GetErrorType()); } diff --git a/be/src/io/fs/s3_file_writer.cpp b/be/src/io/fs/s3_file_writer.cpp index dbe5ce8e70cdab..5286fbf4a52df8 100644 --- a/be/src/io/fs/s3_file_writer.cpp +++ b/be/src/io/fs/s3_file_writer.cpp @@ -107,9 +107,8 @@ S3FileWriter::~S3FileWriter() { // if we don't abort multi part upload, the uploaded part in object // store will not automatically reclaim itself, it would cost more money static_cast(_abort()); - _bytes_written = 0; } - s3_bytes_written_total << _bytes_written; + s3_bytes_written_total << _bytes_appended; s3_file_being_written << -1; } @@ -207,8 +206,10 @@ Status S3FileWriter::close() { auto* buf = dynamic_cast(_pending_buf.get()); DCHECK(buf != nullptr); buf->set_upload_to_remote([this](UploadFileBuffer& b) { _put_object(b); }); - } else if (_create_empty_file) { - // if there is no pending buffer, we need to create an empty file + } + + if (_bytes_appended == 0 && _create_empty_file) { + // No data written, but need to create an empty file auto builder = FileBufferBuilder(); builder.set_type(BufferType::UPLOAD) .set_upload_callback([this](UploadFileBuffer& buf) { _put_object(buf); }) @@ -390,7 +391,6 @@ void S3FileWriter::_upload_one_part(int64_t part_num, UploadFileBuffer& buf) { std::unique_lock lck {_completed_lock}; _completed_parts.emplace_back(std::move(completed_part)); - _bytes_written += buf.get_size(); } Status S3FileWriter::_complete() { @@ -501,7 +501,6 @@ void S3FileWriter::_put_object(UploadFileBuffer& buf) { buf.set_status(_st); return; } - _bytes_written += buf.get_size(); s3_file_created_total << 1; } diff --git a/be/src/io/fs/s3_file_writer.h b/be/src/io/fs/s3_file_writer.h index 8774362f817a71..fc1d85b345a115 100644 --- a/be/src/io/fs/s3_file_writer.h +++ b/be/src/io/fs/s3_file_writer.h @@ -79,7 +79,6 @@ class S3FileWriter final : public FileWriter { std::atomic_bool _failed = false; Status _st; - size_t _bytes_written = 0; std::shared_ptr _pending_buf; int64_t _expiration_time;