From 784525a531d0b3f5103c0baf41e8712052fdea29 Mon Sep 17 00:00:00 2001 From: liulx20 <68941872+liulx20@users.noreply.github.com> Date: Tue, 30 Apr 2024 17:52:39 +0800 Subject: [PATCH] fix(interactive): add error handling during file operations (#3763) Fixes --- flex/engines/graph_db/database/wal.cc | 16 ++--- flex/utils/mmap_array.h | 89 ++++++++++++++++++++------- 2 files changed, 76 insertions(+), 29 deletions(-) diff --git a/flex/engines/graph_db/database/wal.cc b/flex/engines/graph_db/database/wal.cc index d3ab1cf434f8..552b77d1fe58 100644 --- a/flex/engines/graph_db/database/wal.cc +++ b/flex/engines/graph_db/database/wal.cc @@ -32,10 +32,10 @@ void WalWriter::open(const std::string& prefix, int thread_id) { break; } if (fd_ == -1) { - LOG(FATAL) << "Failed to open wal file"; + LOG(FATAL) << "Failed to open wal file " << strerror(errno); } if (ftruncate(fd_, TRUNC_SIZE) != 0) { - LOG(FATAL) << "Failed to truncate wal file"; + LOG(FATAL) << "Failed to truncate wal file " << strerror(errno); } file_size_ = TRUNC_SIZE; file_used_ = 0; @@ -43,7 +43,9 @@ void WalWriter::open(const std::string& prefix, int thread_id) { void WalWriter::close() { if (fd_ != -1) { - ::close(fd_); + if (::close(fd_) != 0) { + LOG(FATAL) << "Failed to close file" << strerror(errno); + } fd_ = -1; file_size_ = 0; file_used_ = 0; @@ -60,7 +62,7 @@ void WalWriter::append(const char* data, size_t length) { if (expected_size > file_size_) { size_t new_file_size = (expected_size / TRUNC_SIZE + 1) * TRUNC_SIZE; if (ftruncate(fd_, new_file_size) != 0) { - LOG(FATAL) << "Failed to truncate wal file"; + LOG(FATAL) << "Failed to truncate wal file " << strerror(errno); } file_size_ = new_file_size; } @@ -68,18 +70,18 @@ void WalWriter::append(const char* data, size_t length) { file_used_ += length; if (static_cast(write(fd_, data, length)) != length) { - LOG(FATAL) << "Failed to write wal file"; + LOG(FATAL) << "Failed to write wal file " << strerror(errno); } #if 1 #ifdef F_FULLFSYNC if (fcntl(fd_, F_FULLFSYNC) != 0) { - LOG(FATAL) << "Failed to fcntl sync wal file"; + LOG(FATAL) << "Failed to fcntl sync wal file " << strerrno(errno); } #else // if (fsync(fd_) != 0) { if (fdatasync(fd_) != 0) { - LOG(FATAL) << "Failed to fsync wal file"; + LOG(FATAL) << "Failed to fsync wal file " << strerror(errno); } #endif #endif diff --git a/flex/utils/mmap_array.h b/flex/utils/mmap_array.h index 10de31a12242..c6d82cbb2cce 100644 --- a/flex/utils/mmap_array.h +++ b/flex/utils/mmap_array.h @@ -82,17 +82,23 @@ class mmap_array { ~mmap_array() {} void reset() { - filename_ = ""; if (data_ != NULL && mmap_size_ != 0) { - munmap(data_, mmap_size_); + if (munmap(data_, mmap_size_) != 0) { + LOG(FATAL) << "Failed to mummap file [ " << filename_ << " ] " + << strerror(errno); + } } data_ = NULL; size_ = 0; mmap_size_ = 0; if (fd_ != -1) { - close(fd_); + if (close(fd_) != 0) { + LOG(FATAL) << "Failed to close file [ " << filename_ << " ] " + << strerror(errno); + } fd_ = -1; } + filename_ = ""; sync_to_file_ = false; } @@ -109,7 +115,7 @@ class mmap_array { bool creat = !std::filesystem::exists(filename_); fd_ = ::open(filename_.c_str(), O_RDWR | O_CREAT, 0777); if (fd_ == -1) { - LOG(FATAL) << "open file [" << filename_ << "] failed, " + LOG(FATAL) << "Failed to open file [" << filename_ << "], " << strerror(errno); } if (creat) { @@ -121,8 +127,8 @@ class mmap_array { std::filesystem::perm_options::add, errorCode); if (errorCode) { - LOG(INFO) << "Failed to set read/write permission for file: " - << filename << " " << errorCode.message() << std::endl; + LOG(FATAL) << "Failed to set read/write permission for file: " + << filename << " " << errorCode.message() << std::endl; } } @@ -135,15 +141,23 @@ class mmap_array { data_ = reinterpret_cast( mmap(NULL, mmap_size_, PROT_READ | PROT_WRITE, MAP_SHARED, fd_, 0)); if (data_ == MAP_FAILED) { - LOG(FATAL) << "mmap file [" << filename_ << "] failed, " + LOG(FATAL) << "Failed to mmap file [" << filename_ << "], " + << strerror(errno); + } + int rt = madvise(data_, mmap_size_, MADV_RANDOM | MADV_WILLNEED); + if (rt != 0) { + LOG(FATAL) << "Failed to madvise file [" << filename_ << "], " << strerror(errno); } - madvise(data_, mmap_size_, MADV_RANDOM | MADV_WILLNEED); } } else { if (!filename_.empty() && std::filesystem::exists(filename_)) { size_t file_size = std::filesystem::file_size(filename_); fd_ = ::open(filename_.c_str(), O_RDWR, 0777); + if (fd_ == -1) { + LOG(FATAL) << "Failed to open file [" << filename_ << "], " + << strerror(errno); + } size_ = file_size / sizeof(T); mmap_size_ = file_size; if (mmap_size_ == 0) { @@ -152,7 +166,7 @@ class mmap_array { data_ = reinterpret_cast(mmap( NULL, mmap_size_, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd_, 0)); if (data_ == MAP_FAILED) { - LOG(FATAL) << "mmap file [" << filename_ << "] failed, " + LOG(FATAL) << "Failed to mmap file [" << filename_ << "], " << strerror(errno); } } @@ -172,10 +186,20 @@ class mmap_array { data_ = static_cast(allocate_hugepages(mmap_size_)); if (data_ != MAP_FAILED) { FILE* fin = fopen(filename.c_str(), "rb"); - CHECK_EQ(fread(data_, sizeof(T), size_, fin), size_); - fclose(fin); + if (fin == NULL) { + LOG(FATAL) << "Failed to open file [ " << filename << " ], " + << strerror(errno); + } + if (fread(data_, sizeof(T), size_, fin) != size_) { + LOG(FATAL) << "Failed to fread file [ " << filename << " ], " + << strerror(errno); + } + if (fclose(fin) != 0) { + LOG(FATAL) << "Failed to fclose file [ " << filename << " ], " + << strerror(errno); + } } else { - LOG(ERROR) << "allocating hugepage failed, " << strerror(errno) + LOG(FATAL) << "allocating hugepage failed, " << strerror(errno) << ", try with normal pages"; open(filename, false); } @@ -189,12 +213,30 @@ class mmap_array { if (sync_to_file_) { std::string old_filename = filename_; reset(); - std::filesystem::rename(old_filename, filename); + std::error_code errorCode; + std::filesystem::rename(old_filename, filename, errorCode); + if (errorCode) { + LOG(FATAL) << "Failed to rename file " << old_filename << " to " + << filename << " " << errorCode.message() << std::endl; + } } else { FILE* fout = fopen(filename.c_str(), "wb"); - CHECK_EQ(fwrite(data_, sizeof(T), size_, fout), size_); - fflush(fout); - fclose(fout); + if (fout == NULL) { + LOG(FATAL) << "Failed to open file [ " << filename << " ], " + << strerror(errno); + } + if (fwrite(data_, sizeof(T), size_, fout) != size_) { + LOG(FATAL) << "Failed to fwrite file [ " << filename << " ], " + << strerror(errno); + } + if (fflush(fout) != 0) { + LOG(FATAL) << "Failed to fflush file [ " << filename << " ], " + << strerror(errno); + } + if (fclose(fout) != 0) { + LOG(FATAL) << "Failed to fclose file [ " << filename << " ], " + << strerror(errno); + } reset(); } @@ -205,8 +247,8 @@ class mmap_array { std::filesystem::perm_options::add, errorCode); if (errorCode) { - LOG(INFO) << "Failed to set read permission for file: " << filename << " " - << errorCode.message() << std::endl; + LOG(FATAL) << "Failed to set read permission for file: " << filename + << " " << errorCode.message() << std::endl; } } @@ -217,12 +259,15 @@ class mmap_array { if (sync_to_file_) { if (data_ != NULL && mmap_size_ != 0) { - munmap(data_, mmap_size_); + if (munmap(data_, mmap_size_) != 0) { + LOG(FATAL) << "Failed to mummap file [ " << filename_ << " ], " + << strerror(errno); + } } size_t new_mmap_size = size * sizeof(T); int rt = ftruncate(fd_, new_mmap_size); if (rt == -1) { - LOG(FATAL) << "ftruncate failed: " << rt << ", " << strerror(errno); + LOG(FATAL) << "Failed to ftruncate " << rt << ", " << strerror(errno); } if (new_mmap_size == 0) { data_ = NULL; @@ -230,7 +275,7 @@ class mmap_array { data_ = reinterpret_cast(mmap( NULL, new_mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd_, 0)); if (data_ == MAP_FAILED) { - LOG(FATAL) << "mmap failed " << strerror(errno); + LOG(FATAL) << "Failed to mmap, " << strerror(errno); } } size_ = size; @@ -245,7 +290,7 @@ class mmap_array { if (hugepage_prefered_) { new_data = reinterpret_cast(allocate_hugepages(new_mmap_size)); if (new_data == MAP_FAILED) { - LOG(ERROR) << "mmap with hugepage failed, " << strerror(errno) + LOG(FATAL) << "mmap with hugepage failed, " << strerror(errno) << ", try with normal pages"; new_data = NULL; } else {