From 857e9960be8a5e4ee3aecfd7d27fe7b1ef6bb352 Mon Sep 17 00:00:00 2001 From: Siying Dong Date: Mon, 26 Jun 2017 12:42:21 -0700 Subject: [PATCH] Improve the error message for I/O related errors. Summary: Force people to write something other than file name while returning status for IOError. Closes https://github.com/facebook/rocksdb/pull/2493 Differential Revision: D5321309 Pulled By: siying fbshipit-source-id: 38bcf6c19e80831cd3e300a047e975cbb131d822 --- env/env_posix.cc | 64 +++++++++++++++++---------------- env/io_posix.cc | 93 +++++++++++++++++++++++++++++++----------------- env/io_posix.h | 17 +++++++-- 3 files changed, 108 insertions(+), 66 deletions(-) diff --git a/env/env_posix.cc b/env/env_posix.cc index bdd1d9018..46fd453bf 100644 --- a/env/env_posix.cc +++ b/env/env_posix.cc @@ -171,7 +171,8 @@ class PosixEnv : public Env { fd = open(fname.c_str(), flags, 0644); } while (fd < 0 && errno == EINTR); if (fd < 0) { - return IOError(fname, errno); + return IOError("While opening a file for sequentially reading", fname, + errno); } SetFD_CLOEXEC(fd, &options); @@ -180,7 +181,7 @@ class PosixEnv : public Env { #ifdef OS_MACOSX if (fcntl(fd, F_NOCACHE, 1) == -1) { close(fd); - return IOError(fname, errno); + return IOError("While fcntl NoCache", fname, errno); } #endif } else { @@ -190,7 +191,8 @@ class PosixEnv : public Env { } while (file == nullptr && errno == EINTR); if (file == nullptr) { close(fd); - return IOError(fname, errno); + return IOError("While opening file for sequentially read", fname, + errno); } } result->reset(new PosixSequentialFile(fname, file, fd, options)); @@ -219,7 +221,7 @@ class PosixEnv : public Env { fd = open(fname.c_str(), flags, 0644); } while (fd < 0 && errno == EINTR); if (fd < 0) { - return IOError(fname, errno); + return IOError("While open a file for random read", fname, errno); } SetFD_CLOEXEC(fd, &options); @@ -235,7 +237,7 @@ class PosixEnv : public Env { result->reset(new PosixMmapReadableFile(fd, fname, base, size, options)); } else { - s = IOError(fname, errno); + s = IOError("while mmap file for read", fname, errno); } } close(fd); @@ -244,7 +246,7 @@ class PosixEnv : public Env { #ifdef OS_MACOSX if (fcntl(fd, F_NOCACHE, 1) == -1) { close(fd); - return IOError(fname, errno); + return IOError("while fcntl NoCache", fname, errno); } #endif } @@ -291,7 +293,7 @@ class PosixEnv : public Env { } while (fd < 0 && errno == EINTR); if (fd < 0) { - s = IOError(fname, errno); + s = IOError("While open a file for appending", fname, errno); return s; } SetFD_CLOEXEC(fd, &options); @@ -312,14 +314,15 @@ class PosixEnv : public Env { #ifdef OS_MACOSX if (fcntl(fd, F_NOCACHE, 1) == -1) { close(fd); - s = IOError(fname, errno); + s = IOError("While fcntl NoCache an opened file for appending", fname, + errno); return s; } #elif defined(OS_SOLARIS) if (directio(fd, DIRECTIO_ON) == -1) { if (errno != ENOTTY) { // ZFS filesystems don't support DIRECTIO_ON close(fd); - s = IOError(fname, errno); + s = IOError("While calling directio()", fname, errno); return s; } } @@ -377,14 +380,14 @@ class PosixEnv : public Env { fd = open(old_fname.c_str(), flags, 0644); } while (fd < 0 && errno == EINTR); if (fd < 0) { - s = IOError(fname, errno); + s = IOError("while reopen file for write", fname, errno); return s; } SetFD_CLOEXEC(fd, &options); // rename into place if (rename(old_fname.c_str(), fname.c_str()) != 0) { - s = IOError(old_fname, errno); + s = IOError("while rename file to " + fname, old_fname, errno); close(fd); return s; } @@ -405,14 +408,15 @@ class PosixEnv : public Env { #ifdef OS_MACOSX if (fcntl(fd, F_NOCACHE, 1) == -1) { close(fd); - s = IOError(fname, errno); + s = IOError("while fcntl NoCache for reopened file for append", fname, + errno); return s; } #elif defined(OS_SOLARIS) if (directio(fd, DIRECTIO_ON) == -1) { if (errno != ENOTTY) { // ZFS filesystems don't support DIRECTIO_ON close(fd); - s = IOError(fname, errno); + s = IOError("while calling directio()", fname, errno); return s; } } @@ -441,7 +445,7 @@ class PosixEnv : public Env { if (errno == EINTR) { continue; } - return IOError(fname, errno); + return IOError("While open file for random read/write", fname, errno); } } @@ -459,7 +463,7 @@ class PosixEnv : public Env { fd = open(name.c_str(), 0); } if (fd < 0) { - return IOError(name, errno); + return IOError("While open directory", name, errno); } else { result->reset(new PosixDirectory(fd)); } @@ -498,7 +502,7 @@ class PosixEnv : public Env { case ENOTDIR: return Status::NotFound(); default: - return IOError(dir, errno); + return IOError("While opendir", dir, errno); } } struct dirent* entry; @@ -512,7 +516,7 @@ class PosixEnv : public Env { virtual Status DeleteFile(const std::string& fname) override { Status result; if (unlink(fname.c_str()) != 0) { - result = IOError(fname, errno); + result = IOError("while unlink() file", fname, errno); } return result; }; @@ -520,7 +524,7 @@ class PosixEnv : public Env { virtual Status CreateDir(const std::string& name) override { Status result; if (mkdir(name.c_str(), 0755) != 0) { - result = IOError(name, errno); + result = IOError("While mkdir", name, errno); } return result; }; @@ -529,7 +533,7 @@ class PosixEnv : public Env { Status result; if (mkdir(name.c_str(), 0755) != 0) { if (errno != EEXIST) { - result = IOError(name, errno); + result = IOError("While mkdir if missing", name, errno); } else if (!DirExists(name)) { // Check that name is actually a // directory. // Message is taken from mkdir @@ -542,7 +546,7 @@ class PosixEnv : public Env { virtual Status DeleteDir(const std::string& name) override { Status result; if (rmdir(name.c_str()) != 0) { - result = IOError(name, errno); + result = IOError("file rmdir", name, errno); } return result; }; @@ -553,7 +557,7 @@ class PosixEnv : public Env { struct stat sbuf; if (stat(fname.c_str(), &sbuf) != 0) { *size = 0; - s = IOError(fname, errno); + s = IOError("while stat a file for size", fname, errno); } else { *size = sbuf.st_size; } @@ -564,7 +568,7 @@ class PosixEnv : public Env { uint64_t* file_mtime) override { struct stat s; if (stat(fname.c_str(), &s) !=0) { - return IOError(fname, errno); + return IOError("while stat a file for modification time", fname, errno); } *file_mtime = static_cast(s.st_mtime); return Status::OK(); @@ -573,7 +577,7 @@ class PosixEnv : public Env { const std::string& target) override { Status result; if (rename(src.c_str(), target.c_str()) != 0) { - result = IOError(src, errno); + result = IOError("While renaming a file to " + target, src, errno); } return result; } @@ -585,7 +589,7 @@ class PosixEnv : public Env { if (errno == EXDEV) { return Status::NotSupported("No cross FS links allowed"); } - result = IOError(src, errno); + result = IOError("while link file to " + target, src, errno); } return result; } @@ -599,9 +603,9 @@ class PosixEnv : public Env { fd = open(fname.c_str(), O_RDWR | O_CREAT, 0644); } if (fd < 0) { - result = IOError(fname, errno); + result = IOError("while open a file for lock", fname, errno); } else if (LockOrUnlock(fname, fd, true) == -1) { - result = IOError("lock " + fname, errno); + result = IOError("While lock file", fname, errno); close(fd); } else { SetFD_CLOEXEC(fd, nullptr); @@ -617,7 +621,7 @@ class PosixEnv : public Env { PosixFileLock* my_lock = reinterpret_cast(lock); Status result; if (LockOrUnlock(my_lock->filename, my_lock->fd_, false) == -1) { - result = IOError("unlock", errno); + result = IOError("unlock", my_lock->filename, errno); } close(my_lock->fd_); delete my_lock; @@ -680,7 +684,7 @@ class PosixEnv : public Env { } if (f == nullptr) { result->reset(); - return IOError(fname, errno); + return IOError("when fopen a file for new logger", fname, errno); } else { int fd = fileno(f); #ifdef ROCKSDB_FALLOCATE_PRESENT @@ -726,7 +730,7 @@ class PosixEnv : public Env { if (errno == EFAULT || errno == EINVAL) return Status::InvalidArgument(strerror(errno)); else - return IOError("GetHostName", errno); + return IOError("GetHostName", name, errno); } return Status::OK(); } @@ -734,7 +738,7 @@ class PosixEnv : public Env { virtual Status GetCurrentTime(int64_t* unix_time) override { time_t ret = time(nullptr); if (ret == (time_t) -1) { - return IOError("GetCurrentTime", errno); + return IOError("GetCurrentTime", "", errno); } *unix_time = (int64_t) ret; return Status::OK(); diff --git a/env/io_posix.cc b/env/io_posix.cc index ee44662dc..c6e0f7590 100644 --- a/env/io_posix.cc +++ b/env/io_posix.cc @@ -169,7 +169,7 @@ Status PosixSequentialFile::Read(size_t n, Slice* result, char* scratch) { clearerr(file_); } else { // A partial read with an error: return a non-ok status - s = IOError(filename_, errno); + s = IOError("While reading file sequentially", filename_, errno); } } // we need to fadvise away the entire range of pages because @@ -209,7 +209,9 @@ Status PosixSequentialFile::PositionedRead(uint64_t offset, size_t n, } if (r < 0) { // An error: return a non-ok status - s = IOError(filename_, errno); + s = IOError( + "While pread " + ToString(n) + " bytes from offset " + ToString(offset), + filename_, errno); } *result = Slice(scratch, (r < 0) ? 0 : n - left); return s; @@ -217,7 +219,8 @@ Status PosixSequentialFile::PositionedRead(uint64_t offset, size_t n, Status PosixSequentialFile::Skip(uint64_t n) { if (fseek(file_, static_cast(n), SEEK_CUR)) { - return IOError(filename_, errno); + return IOError("While fseek to skip " + ToString(n) + " bytes", filename_, + errno); } return Status::OK(); } @@ -230,7 +233,9 @@ Status PosixSequentialFile::InvalidateCache(size_t offset, size_t length) { // free OS pages int ret = Fadvise(fd_, offset, length, POSIX_FADV_DONTNEED); if (ret != 0) { - return IOError(filename_, errno); + return IOError("While fadvise NotNeeded offset " + ToString(offset) + + " len " + ToString(length), + filename_, errno); } } return Status::OK(); @@ -338,7 +343,9 @@ Status PosixRandomAccessFile::Read(uint64_t offset, size_t n, Slice* result, } if (r < 0) { // An error: return a non-ok status - s = IOError(filename_, errno); + s = IOError( + "While pread offset " + ToString(offset) + " len " + ToString(n), + filename_, errno); } *result = Slice(scratch, (r < 0) ? 0 : n - left); return s; @@ -358,7 +365,9 @@ Status PosixRandomAccessFile::Prefetch(uint64_t offset, size_t n) { r = fcntl(fd_, F_RDADVISE, &advice); #endif if (r == -1) { - s = IOError(filename_, errno); + s = IOError("While prefetching offset " + ToString(offset) + " len " + + ToString(n), + filename_, errno); } } return s; @@ -408,7 +417,9 @@ Status PosixRandomAccessFile::InvalidateCache(size_t offset, size_t length) { if (ret == 0) { return Status::OK(); } - return IOError(filename_, errno); + return IOError("While fadvise NotNeeded offset " + ToString(offset) + + " len " + ToString(length), + filename_, errno); #endif } @@ -441,7 +452,9 @@ Status PosixMmapReadableFile::Read(uint64_t offset, size_t n, Slice* result, Status s; if (offset > length_) { *result = Slice(); - return IOError(filename_, EINVAL); + return IOError("While mmap read offset " + ToString(offset) + + " larger than file length " + ToString(length_), + filename_, EINVAL); } else if (offset + n > length_) { n = static_cast(length_ - offset); } @@ -458,7 +471,9 @@ Status PosixMmapReadableFile::InvalidateCache(size_t offset, size_t length) { if (ret == 0) { return Status::OK(); } - return IOError(filename_, errno); + return IOError("While fadvise not needed. Offset " + ToString(offset) + + " len" + ToString(length), + filename_, errno); #endif } @@ -475,7 +490,7 @@ Status PosixMmapFile::UnmapCurrentRegion() { if (base_ != nullptr) { int munmap_status = munmap(base_, limit_ - base_); if (munmap_status != 0) { - return IOError(filename_, munmap_status); + return IOError("While munmap", filename_, munmap_status); } file_offset_ += limit_ - base_; base_ = nullptr; @@ -538,7 +553,7 @@ Status PosixMmapFile::Msync() { last_sync_ = dst_; TEST_KILL_RANDOM("PosixMmapFile::Msync:0", rocksdb_kill_odds); if (msync(base_ + p1, p2 - p1 + page_size_, MS_SYNC) < 0) { - return IOError(filename_, errno); + return IOError("While msync", filename_, errno); } return Status::OK(); } @@ -604,17 +619,17 @@ Status PosixMmapFile::Close() { s = UnmapCurrentRegion(); if (!s.ok()) { - s = IOError(filename_, errno); + s = IOError("While closing mmapped file", filename_, errno); } else if (unused > 0) { // Trim the extra space at the end of the file if (ftruncate(fd_, file_offset_ - unused) < 0) { - s = IOError(filename_, errno); + s = IOError("While ftruncating mmaped file", filename_, errno); } } if (close(fd_) < 0) { if (s.ok()) { - s = IOError(filename_, errno); + s = IOError("While closing mmapped file", filename_, errno); } } @@ -628,7 +643,7 @@ Status PosixMmapFile::Flush() { return Status::OK(); } Status PosixMmapFile::Sync() { if (fdatasync(fd_) < 0) { - return IOError(filename_, errno); + return IOError("While fdatasync mmapped file", filename_, errno); } return Msync(); @@ -639,7 +654,7 @@ Status PosixMmapFile::Sync() { */ Status PosixMmapFile::Fsync() { if (fsync(fd_) < 0) { - return IOError(filename_, errno); + return IOError("While fsync mmaped file", filename_, errno); } return Msync(); @@ -664,7 +679,7 @@ Status PosixMmapFile::InvalidateCache(size_t offset, size_t length) { if (ret == 0) { return Status::OK(); } - return IOError(filename_, errno); + return IOError("While fadvise NotNeeded mmapped file", filename_, errno); #endif } @@ -682,7 +697,9 @@ Status PosixMmapFile::Allocate(uint64_t offset, uint64_t len) { if (alloc_status == 0) { return Status::OK(); } else { - return IOError(filename_, errno); + return IOError( + "While fallocate offset " + ToString(offset) + " len " + ToString(len), + filename_, errno); } } #endif @@ -725,7 +742,7 @@ Status PosixWritableFile::Append(const Slice& data) { if (errno == EINTR) { continue; } - return IOError(filename_, errno); + return IOError("While appending to file", filename_, errno); } left -= done; src += done; @@ -749,7 +766,8 @@ Status PosixWritableFile::PositionedAppend(const Slice& data, uint64_t offset) { if (errno == EINTR) { continue; } - return IOError(filename_, errno); + return IOError("While pwrite to file at offset " + ToString(offset), + filename_, errno); } left -= done; offset += done; @@ -763,7 +781,8 @@ Status PosixWritableFile::Truncate(uint64_t size) { Status s; int r = ftruncate(fd_, size); if (r < 0) { - s = IOError(filename_, errno); + s = IOError("While ftruncate file to size " + ToString(size), filename_, + errno); } else { filesize_ = size; } @@ -817,7 +836,7 @@ Status PosixWritableFile::Close() { } if (close(fd_) < 0) { - s = IOError(filename_, errno); + s = IOError("While closing file after writing", filename_, errno); } fd_ = -1; return s; @@ -828,14 +847,14 @@ Status PosixWritableFile::Flush() { return Status::OK(); } Status PosixWritableFile::Sync() { if (fdatasync(fd_) < 0) { - return IOError(filename_, errno); + return IOError("While fdatasync", filename_, errno); } return Status::OK(); } Status PosixWritableFile::Fsync() { if (fsync(fd_) < 0) { - return IOError(filename_, errno); + return IOError("While fsync", filename_, errno); } return Status::OK(); } @@ -856,7 +875,7 @@ Status PosixWritableFile::InvalidateCache(size_t offset, size_t length) { if (ret == 0) { return Status::OK(); } - return IOError(filename_, errno); + return IOError("While fadvise NotNeeded", filename_, errno); #endif } @@ -875,7 +894,9 @@ Status PosixWritableFile::Allocate(uint64_t offset, uint64_t len) { if (alloc_status == 0) { return Status::OK(); } else { - return IOError(filename_, errno); + return IOError( + "While fallocate offset " + ToString(offset) + " len " + ToString(len), + filename_, errno); } } #endif @@ -888,7 +909,9 @@ Status PosixWritableFile::RangeSync(uint64_t offset, uint64_t nbytes) { static_cast(nbytes), SYNC_FILE_RANGE_WRITE) == 0) { return Status::OK(); } else { - return IOError(filename_, errno); + return IOError("While sync_file_range offset " + ToString(offset) + + " bytes " + ToString(nbytes), + filename_, errno); } } #endif @@ -924,7 +947,9 @@ Status PosixRandomRWFile::Write(uint64_t offset, const Slice& data) { // write was interrupted, try again. continue; } - return IOError(filename_, errno); + return IOError( + "While write random read/write file at offset " + ToString(offset), + filename_, errno); } // Wrote `done` bytes @@ -948,7 +973,9 @@ Status PosixRandomRWFile::Read(uint64_t offset, size_t n, Slice* result, // read was interrupted, try again. continue; } - return IOError(filename_, errno); + return IOError("While reading random read/write file offset " + + ToString(offset) + " len " + ToString(n), + filename_, errno); } else if (done == 0) { // Nothing more to read break; @@ -968,21 +995,21 @@ Status PosixRandomRWFile::Flush() { return Status::OK(); } Status PosixRandomRWFile::Sync() { if (fdatasync(fd_) < 0) { - return IOError(filename_, errno); + return IOError("While fdatasync random read/write file", filename_, errno); } return Status::OK(); } Status PosixRandomRWFile::Fsync() { if (fsync(fd_) < 0) { - return IOError(filename_, errno); + return IOError("While fsync random read/write file", filename_, errno); } return Status::OK(); } Status PosixRandomRWFile::Close() { if (close(fd_) < 0) { - return IOError(filename_, errno); + return IOError("While close random read/write file", filename_, errno); } fd_ = -1; return Status::OK(); @@ -997,7 +1024,7 @@ PosixDirectory::~PosixDirectory() { close(fd_); } Status PosixDirectory::Fsync() { #ifndef OS_AIX if (fsync(fd_) == -1) { - return IOError("directory", errno); + return IOError("While fsync", "a directory", errno); } #endif return Status::OK(); diff --git a/env/io_posix.h b/env/io_posix.h index b3409ef1a..7d1fc074f 100644 --- a/env/io_posix.h +++ b/env/io_posix.h @@ -26,15 +26,26 @@ #endif namespace rocksdb { +static std::string IOErrorMsg(const std::string& context, + const std::string& file_name) { + if (file_name.empty()) { + return context; + } + return context + ": " + file_name; +} -static Status IOError(const std::string& context, int err_number) { +// file_name can be left empty if it is not unkown. +static Status IOError(const std::string& context, const std::string& file_name, + int err_number) { switch (err_number) { case ENOSPC: - return Status::NoSpace(context, strerror(err_number)); + return Status::NoSpace(IOErrorMsg(context, file_name), + strerror(err_number)); case ESTALE: return Status::IOError(Status::kStaleFile); default: - return Status::IOError(context, strerror(err_number)); + return Status::IOError(IOErrorMsg(context, file_name), + strerror(err_number)); } }