From d9d190742c0a8a3d5ff0b0867383bd38c1d0f5f0 Mon Sep 17 00:00:00 2001
From: mrambacher <mrambach@gmail.com>
Date: Tue, 28 Jul 2020 22:58:28 -0700
Subject: [PATCH] Make env*_test work with ASSERT_STATUS_CHECKED (#7176)
Summary:
Make (most of) the env*_test pass when ASSERT_STATUS_CHECKED is enabled.
One test that opens a database is currently disabled in this mode, as there are many errors that need revisited for DB tests and status checks.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/7176
Reviewed By: cheng-chang
Differential Revision: D22799278
Pulled By: ajkr
fbshipit-source-id: 16d8a02eaeecd6df1060249b6a5811292801f2ed
---
Makefile | 9 +++-
db/db_impl/db_impl_files.cc | 2 +-
env/env_basic_test.cc | 23 +++++-----
env/env_chroot.cc | 3 +-
env/env_encryption.cc | 13 +++---
env/env_posix.cc | 1 -
env/env_test.cc | 77 ++++++++++++++++++----------------
env/fs_posix.cc | 45 +++++++++-----------
env/io_posix.cc | 9 ++--
env/mock_env.cc | 2 +-
file/sst_file_manager_impl.cc | 5 ++-
include/rocksdb/file_system.h | 3 +-
test_util/testharness.cc | 2 +-
utilities/env_timed_test.cc | 2 +-
utilities/fault_injection_fs.h | 8 ++--
15 files changed, 111 insertions(+), 93 deletions(-)
diff --git a/Makefile b/Makefile
index 847e7b47df..e69ac2639c 100644
--- a/Makefile
+++ b/Makefile
@@ -572,9 +572,11 @@ ifdef ASSERT_STATUS_CHECKED
dbformat_test \
defer_test \
dynamic_bloom_test \
+ env_basic_test \
+ env_test \
+ env_logger_test \
event_logger_test \
file_indexer_test \
- folly_synchronization_distributed_mutex_test \
hash_table_test \
hash_test \
heap_test \
@@ -596,6 +598,7 @@ ifdef ASSERT_STATUS_CHECKED
slice_test \
statistics_test \
thread_local_test \
+ env_timed_test \
timer_queue_test \
timer_test \
util_merge_operators_test \
@@ -603,6 +606,10 @@ ifdef ASSERT_STATUS_CHECKED
work_queue_test \
write_controller_test \
+ifeq ($(USE_FOLLY_DISTRIBUTED_MUTEX),1)
+TESTS_PASSING_ASC += folly_synchronization_distributed_mutex_test
+endif
+
# Enable building all unit tests, but use check_some to run only tests
# known to pass ASC
SUBSET := $(TESTS_PASSING_ASC)
diff --git a/db/db_impl/db_impl_files.cc b/db/db_impl/db_impl_files.cc
index ea0d12296f..5fd7697766 100644
--- a/db/db_impl/db_impl_files.cc
+++ b/db/db_impl/db_impl_files.cc
@@ -190,7 +190,7 @@ void DBImpl::FindObsoleteFiles(JobContext* job_context, bool force,
// set of all files in the directory. We'll exclude files that are still
// alive in the subsequent processings.
std::vector<std::string> files;
- env_->GetChildren(path, &files); // Ignore errors
+ env_->GetChildren(path, &files).PermitUncheckedError(); // Ignore errors
for (const std::string& file : files) {
uint64_t number;
FileType type;
diff --git a/env/env_basic_test.cc b/env/env_basic_test.cc
index 194853b677..f72e805ac3 100644
--- a/env/env_basic_test.cc
+++ b/env/env_basic_test.cc
@@ -62,11 +62,13 @@ class EnvBasicTestWithParam : public testing::Test,
test_dir_ = test::PerThreadDBPath(env_, "env_basic_test");
}
- void SetUp() override { env_->CreateDirIfMissing(test_dir_); }
+ void SetUp() override {
+ env_->CreateDirIfMissing(test_dir_).PermitUncheckedError();
+ }
void TearDown() override {
std::vector<std::string> files;
- env_->GetChildren(test_dir_, &files);
+ env_->GetChildren(test_dir_, &files).PermitUncheckedError();
for (const auto& file : files) {
// don't know whether it's file or directory, try both. The tests must
// only create files or empty directories, so one must succeed, else the
@@ -209,13 +211,12 @@ TEST_P(EnvBasicTestWithParam, Basics) {
soptions_)
.ok());
ASSERT_TRUE(!seq_file);
- ASSERT_TRUE(!env_->NewRandomAccessFile(test_dir_ + "/non_existent",
- &rand_file, soptions_)
- .ok());
+ ASSERT_NOK(env_->NewRandomAccessFile(test_dir_ + "/non_existent", &rand_file,
+ soptions_));
ASSERT_TRUE(!rand_file);
// Check that deleting works.
- ASSERT_TRUE(!env_->DeleteFile(test_dir_ + "/non_existent").ok());
+ ASSERT_NOK(env_->DeleteFile(test_dir_ + "/non_existent"));
ASSERT_OK(env_->DeleteFile(test_dir_ + "/g"));
ASSERT_EQ(Status::NotFound(), env_->FileExists(test_dir_ + "/g"));
ASSERT_OK(env_->GetChildren(test_dir_, &children));
@@ -346,14 +347,14 @@ TEST_P(EnvMoreTestWithParam, GetChildren) {
ASSERT_EQ(3U, children.size());
ASSERT_EQ(3U, childAttr.size());
for (auto each : children) {
- env_->DeleteDir(test_dir_ + "/" + each);
+ env_->DeleteDir(test_dir_ + "/" + each).PermitUncheckedError();
} // necessary for default POSIX env
// non-exist directory returns IOError
ASSERT_OK(env_->DeleteDir(test_dir_));
- ASSERT_TRUE(!env_->FileExists(test_dir_).ok());
- ASSERT_TRUE(!env_->GetChildren(test_dir_, &children).ok());
- ASSERT_TRUE(!env_->GetChildrenFileAttributes(test_dir_, &childAttr).ok());
+ ASSERT_NOK(env_->FileExists(test_dir_));
+ ASSERT_NOK(env_->GetChildren(test_dir_, &children));
+ ASSERT_NOK(env_->GetChildrenFileAttributes(test_dir_, &childAttr));
// if dir is a file, returns IOError
ASSERT_OK(env_->CreateDir(test_dir_));
@@ -362,7 +363,7 @@ TEST_P(EnvMoreTestWithParam, GetChildren) {
env_->NewWritableFile(test_dir_ + "/file", &writable_file, soptions_));
ASSERT_OK(writable_file->Close());
writable_file.reset();
- ASSERT_TRUE(!env_->GetChildren(test_dir_ + "/file", &children).ok());
+ ASSERT_NOK(env_->GetChildren(test_dir_ + "/file", &children));
ASSERT_EQ(0U, children.size());
}
diff --git a/env/env_chroot.cc b/env/env_chroot.cc
index d0019c37e5..4bc2f9a250 100644
--- a/env/env_chroot.cc
+++ b/env/env_chroot.cc
@@ -256,8 +256,7 @@ class ChrootEnv : public EnvWrapper {
*path = buf;
// Directory may already exist, so ignore return
- CreateDir(*path);
- return Status::OK();
+ return CreateDirIfMissing(*path);
}
Status NewLogger(const std::string& fname,
diff --git a/env/env_encryption.cc b/env/env_encryption.cc
index 2a2a42dd81..c7eac4c4cf 100644
--- a/env/env_encryption.cc
+++ b/env/env_encryption.cc
@@ -472,11 +472,14 @@ class EncryptedEnv : public EnvWrapper {
// Initialize prefix
prefixBuf.Alignment(underlying->GetRequiredBufferAlignment());
prefixBuf.AllocateNewBuffer(prefixLength);
- provider_->CreateNewPrefix(fname, prefixBuf.BufferStart(), prefixLength);
- prefixBuf.Size(prefixLength);
- prefixSlice = Slice(prefixBuf.BufferStart(), prefixBuf.CurrentSize());
- // Write prefix
- status = underlying->Append(prefixSlice);
+ status = provider_->CreateNewPrefix(fname, prefixBuf.BufferStart(),
+ prefixLength);
+ if (status.ok()) {
+ prefixBuf.Size(prefixLength);
+ prefixSlice = Slice(prefixBuf.BufferStart(), prefixBuf.CurrentSize());
+ // Write prefix
+ status = underlying->Append(prefixSlice);
+ }
if (!status.ok()) {
return status;
}
diff --git a/env/env_posix.cc b/env/env_posix.cc
index c6677f6ea9..46f1b42fdf 100644
--- a/env/env_posix.cc
+++ b/env/env_posix.cc
@@ -167,7 +167,6 @@ class PosixEnv : public CompositeEnvWrapper {
// provided by the search path
Status LoadLibrary(const std::string& name, const std::string& path,
std::shared_ptr<DynamicLibrary>* result) override {
- Status status;
assert(result != nullptr);
if (name.empty()) {
void* hndl = dlopen(NULL, RTLD_NOW);
diff --git a/env/env_test.cc b/env/env_test.cc
index 73b5c95a69..dae14f0155 100644
--- a/env/env_test.cc
+++ b/env/env_test.cc
@@ -186,7 +186,7 @@ TEST_F(EnvPosixTest, DISABLED_FilePermission) {
if (::stat(filename.c_str(), &sb) == 0) {
ASSERT_EQ(sb.st_mode & 0777, 0644);
}
- env_->DeleteFile(filename);
+ ASSERT_OK(env_->DeleteFile(filename));
}
env_->SetAllowNonOwnerAccess(false);
@@ -199,7 +199,7 @@ TEST_F(EnvPosixTest, DISABLED_FilePermission) {
if (::stat(filename.c_str(), &sb) == 0) {
ASSERT_EQ(sb.st_mode & 0777, 0600);
}
- env_->DeleteFile(filename);
+ ASSERT_OK(env_->DeleteFile(filename));
}
}
}
@@ -235,7 +235,8 @@ TEST_F(EnvPosixTest, LowerThreadPoolCpuPriority) {
{
// Same priority, no-op.
env_->LowerThreadPoolCPUPriority(Env::Priority::BOTTOM,
- CpuPriority::kNormal);
+ CpuPriority::kNormal)
+ .PermitUncheckedError();
RunTask(Env::Priority::BOTTOM);
ASSERT_EQ(from_priority, CpuPriority::kNormal);
ASSERT_EQ(to_priority, CpuPriority::kNormal);
@@ -243,7 +244,8 @@ TEST_F(EnvPosixTest, LowerThreadPoolCpuPriority) {
{
// Higher priority, no-op.
- env_->LowerThreadPoolCPUPriority(Env::Priority::BOTTOM, CpuPriority::kHigh);
+ env_->LowerThreadPoolCPUPriority(Env::Priority::BOTTOM, CpuPriority::kHigh)
+ .PermitUncheckedError();
RunTask(Env::Priority::BOTTOM);
ASSERT_EQ(from_priority, CpuPriority::kNormal);
ASSERT_EQ(to_priority, CpuPriority::kNormal);
@@ -251,7 +253,8 @@ TEST_F(EnvPosixTest, LowerThreadPoolCpuPriority) {
{
// Lower priority from kNormal -> kLow.
- env_->LowerThreadPoolCPUPriority(Env::Priority::BOTTOM, CpuPriority::kLow);
+ env_->LowerThreadPoolCPUPriority(Env::Priority::BOTTOM, CpuPriority::kLow)
+ .PermitUncheckedError();
RunTask(Env::Priority::BOTTOM);
ASSERT_EQ(from_priority, CpuPriority::kNormal);
ASSERT_EQ(to_priority, CpuPriority::kLow);
@@ -259,7 +262,8 @@ TEST_F(EnvPosixTest, LowerThreadPoolCpuPriority) {
{
// Lower priority from kLow -> kIdle.
- env_->LowerThreadPoolCPUPriority(Env::Priority::BOTTOM, CpuPriority::kIdle);
+ env_->LowerThreadPoolCPUPriority(Env::Priority::BOTTOM, CpuPriority::kIdle)
+ .PermitUncheckedError();
RunTask(Env::Priority::BOTTOM);
ASSERT_EQ(from_priority, CpuPriority::kLow);
ASSERT_EQ(to_priority, CpuPriority::kIdle);
@@ -267,7 +271,8 @@ TEST_F(EnvPosixTest, LowerThreadPoolCpuPriority) {
{
// Lower priority from kNormal -> kIdle for another pool.
- env_->LowerThreadPoolCPUPriority(Env::Priority::HIGH, CpuPriority::kIdle);
+ env_->LowerThreadPoolCPUPriority(Env::Priority::HIGH, CpuPriority::kIdle)
+ .PermitUncheckedError();
RunTask(Env::Priority::HIGH);
ASSERT_EQ(from_priority, CpuPriority::kNormal);
ASSERT_EQ(to_priority, CpuPriority::kIdle);
@@ -1006,7 +1011,7 @@ TEST_P(EnvPosixTestWithParam, RandomAccessUniqueID) {
ASSERT_EQ(unique_id2, unique_id3);
// Delete the file
- env_->DeleteFile(fname);
+ ASSERT_OK(env_->DeleteFile(fname));
}
}
#endif // !defined(OS_WIN)
@@ -1567,7 +1572,7 @@ TEST_P(EnvPosixTestWithParam, Preallocation) {
auto data = NewAligned(kStrSize, 'A');
Slice str(data.get(), kStrSize);
srcfile->PrepareWrite(srcfile->GetFileSize(), kStrSize);
- srcfile->Append(str);
+ ASSERT_OK(srcfile->Append(str));
srcfile->GetPreallocationStatus(&block_size, &last_allocated_block);
ASSERT_EQ(last_allocated_block, 1UL);
@@ -1576,7 +1581,7 @@ TEST_P(EnvPosixTestWithParam, Preallocation) {
auto buf_ptr = NewAligned(block_size, ' ');
Slice buf(buf_ptr.get(), block_size);
srcfile->PrepareWrite(srcfile->GetFileSize(), block_size);
- srcfile->Append(buf);
+ ASSERT_OK(srcfile->Append(buf));
srcfile->GetPreallocationStatus(&block_size, &last_allocated_block);
ASSERT_EQ(last_allocated_block, 2UL);
}
@@ -1586,7 +1591,7 @@ TEST_P(EnvPosixTestWithParam, Preallocation) {
auto buf_ptr = NewAligned(block_size * 5, ' ');
Slice buf = Slice(buf_ptr.get(), block_size * 5);
srcfile->PrepareWrite(srcfile->GetFileSize(), buf.size());
- srcfile->Append(buf);
+ ASSERT_OK(srcfile->Append(buf));
srcfile->GetPreallocationStatus(&block_size, &last_allocated_block);
ASSERT_EQ(last_allocated_block, 7UL);
}
@@ -1603,7 +1608,7 @@ TEST_P(EnvPosixTestWithParam, ConsistentChildrenAttributes) {
std::string data;
std::string test_base_dir = test::PerThreadDBPath(env_, "env_test_chr_attr");
- env_->CreateDir(test_base_dir);
+ env_->CreateDir(test_base_dir).PermitUncheckedError();
for (int i = 0; i < kNumChildren; ++i) {
const std::string path = test_base_dir + "/testfile_" + std::to_string(i);
std::unique_ptr<WritableFile> file;
@@ -1619,7 +1624,7 @@ TEST_P(EnvPosixTestWithParam, ConsistentChildrenAttributes) {
ASSERT_OK(env_->NewWritableFile(path, &file, soptions));
auto buf_ptr = NewAligned(data.size(), 'T');
Slice buf(buf_ptr.get(), data.size());
- file->Append(buf);
+ ASSERT_OK(file->Append(buf));
data.append(std::string(4096, 'T'));
}
@@ -1770,13 +1775,13 @@ TEST_P(EnvPosixTestWithParam, WritableFileWrapper) {
{
Base b(&step);
Wrapper w(&b);
- w.Append(Slice());
- w.PositionedAppend(Slice(), 0);
- w.Truncate(0);
- w.Close();
- w.Flush();
- w.Sync();
- w.Fsync();
+ ASSERT_OK(w.Append(Slice()));
+ ASSERT_OK(w.PositionedAppend(Slice(), 0));
+ ASSERT_OK(w.Truncate(0));
+ ASSERT_OK(w.Close());
+ ASSERT_OK(w.Flush());
+ ASSERT_OK(w.Sync());
+ ASSERT_OK(w.Fsync());
w.IsSyncThreadSafe();
w.use_direct_io();
w.GetRequiredBufferAlignment();
@@ -1788,10 +1793,10 @@ TEST_P(EnvPosixTestWithParam, WritableFileWrapper) {
w.SetPreallocationBlockSize(0);
w.GetPreallocationStatus(nullptr, nullptr);
w.GetUniqueId(nullptr, 0);
- w.InvalidateCache(0, 0);
- w.RangeSync(0, 0);
+ ASSERT_OK(w.InvalidateCache(0, 0));
+ ASSERT_OK(w.RangeSync(0, 0));
w.PrepareWrite(0, 0);
- w.Allocate(0, 0);
+ ASSERT_OK(w.Allocate(0, 0));
}
EXPECT_EQ(24, step);
@@ -1800,7 +1805,7 @@ TEST_P(EnvPosixTestWithParam, WritableFileWrapper) {
TEST_P(EnvPosixTestWithParam, PosixRandomRWFile) {
const std::string path = test::PerThreadDBPath(env_, "random_rw_file");
- env_->DeleteFile(path);
+ env_->DeleteFile(path).PermitUncheckedError();
std::unique_ptr<RandomRWFile> file;
@@ -1850,7 +1855,7 @@ TEST_P(EnvPosixTestWithParam, PosixRandomRWFile) {
ASSERT_EQ(read_res.ToString(), "XXXQ");
// Close file and reopen it
- file->Close();
+ ASSERT_OK(file->Close());
ASSERT_OK(env_->NewRandomRWFile(path, &file, EnvOptions()));
ASSERT_OK(file->Read(0, 9, &read_res, buf));
@@ -1867,7 +1872,7 @@ TEST_P(EnvPosixTestWithParam, PosixRandomRWFile) {
ASSERT_EQ(read_res.ToString(), "ABXXTTTTTT");
// Clean up
- env_->DeleteFile(path);
+ ASSERT_OK(env_->DeleteFile(path));
}
class RandomRWFileWithMirrorString {
@@ -1927,7 +1932,7 @@ class RandomRWFileWithMirrorString {
TEST_P(EnvPosixTestWithParam, PosixRandomRWFileRandomized) {
const std::string path = test::PerThreadDBPath(env_, "random_rw_file_rand");
- env_->DeleteFile(path);
+ env_->DeleteFile(path).PermitUncheckedError();
std::unique_ptr<RandomRWFile> file;
@@ -1968,7 +1973,7 @@ TEST_P(EnvPosixTestWithParam, PosixRandomRWFileRandomized) {
}
// clean up
- env_->DeleteFile(path);
+ ASSERT_OK(env_->DeleteFile(path));
}
class TestEnv : public EnvWrapper {
@@ -1982,7 +1987,8 @@ class TestEnv : public EnvWrapper {
TestLogger(TestEnv* env_ptr) : Logger() { env = env_ptr; }
~TestLogger() override {
if (!closed_) {
- CloseHelper();
+ Status s = CloseHelper();
+ s.PermitUncheckedError();
}
}
void Logv(const char* /*format*/, va_list /*ap*/) override{};
@@ -2026,17 +2032,17 @@ TEST_F(EnvTest, Close) {
Status s;
s = env->NewLogger("", &logger);
- ASSERT_EQ(s, Status::OK());
- logger.get()->Close();
+ ASSERT_OK(s);
+ ASSERT_OK(logger.get()->Close());
ASSERT_EQ(env->GetCloseCount(), 1);
// Call Close() again. CloseHelper() should not be called again
- logger.get()->Close();
+ ASSERT_OK(logger.get()->Close());
ASSERT_EQ(env->GetCloseCount(), 1);
logger.reset();
ASSERT_EQ(env->GetCloseCount(), 1);
s = env->NewLogger("", &logger);
- ASSERT_EQ(s, Status::OK());
+ ASSERT_OK(s);
logger.reset();
ASSERT_EQ(env->GetCloseCount(), 2);
@@ -2105,6 +2111,8 @@ class EnvFSTestWithParam
std::string dbname2_;
};
+#ifndef ROCKSDB_ASSERT_STATUS_CHECKED // Database tests do not do well with
+ // this flag
TEST_P(EnvFSTestWithParam, OptionsTest) {
Options opts;
opts.env = env_;
@@ -2143,6 +2151,7 @@ TEST_P(EnvFSTestWithParam, OptionsTest) {
dbname = dbname2_;
}
}
+#endif // ROCKSDB_ASSERT_STATUS_CHECKED
// The parameters are as follows -
// 1. True means Options::env is non-null, false means null
@@ -2152,7 +2161,6 @@ INSTANTIATE_TEST_CASE_P(
EnvFSTest, EnvFSTestWithParam,
::testing::Combine(::testing::Bool(), ::testing::Bool(),
::testing::Bool()));
-
// This test ensures that default Env and those allocated by
// NewCompositeEnv() all share the same threadpool
TEST_F(EnvTest, MultipleCompositeEnv) {
@@ -2164,7 +2172,6 @@ TEST_F(EnvTest, MultipleCompositeEnv) {
std::unique_ptr<Env> env2 = NewCompositeEnv(fs2);
Env::Default()->SetBackgroundThreads(8, Env::HIGH);
Env::Default()->SetBackgroundThreads(16, Env::LOW);
-
ASSERT_EQ(env1->GetBackgroundThreads(Env::LOW), 16);
ASSERT_EQ(env1->GetBackgroundThreads(Env::HIGH), 8);
ASSERT_EQ(env2->GetBackgroundThreads(Env::LOW), 16);
diff --git a/env/fs_posix.cc b/env/fs_posix.cc
index b00db6f678..8b06225df0 100644
--- a/env/fs_posix.cc
+++ b/env/fs_posix.cc
@@ -201,7 +201,7 @@ class PosixFileSystem : public FileSystem {
std::unique_ptr<FSRandomAccessFile>* result,
IODebugContext* /*dbg*/) override {
result->reset();
- IOStatus s;
+ IOStatus s = IOStatus::OK();
int fd;
int flags = cloexec_flags(O_RDONLY, &options);
@@ -221,7 +221,8 @@ class PosixFileSystem : public FileSystem {
fd = open(fname.c_str(), flags, GetDBFileMode(allow_non_owner_access_));
} while (fd < 0 && errno == EINTR);
if (fd < 0) {
- return IOError("While open a file for random read", fname, errno);
+ s = IOError("While open a file for random read", fname, errno);
+ return s;
}
SetFD_CLOEXEC(fd, &options);
@@ -636,50 +637,46 @@ class PosixFileSystem : public FileSystem {
IOStatus CreateDir(const std::string& name, const IOOptions& /*opts*/,
IODebugContext* /*dbg*/) override {
- IOStatus result;
if (mkdir(name.c_str(), 0755) != 0) {
- result = IOError("While mkdir", name, errno);
+ return IOError("While mkdir", name, errno);
}
- return result;
+ return IOStatus::OK();
}
IOStatus CreateDirIfMissing(const std::string& name,
const IOOptions& /*opts*/,
IODebugContext* /*dbg*/) override {
- IOStatus result;
if (mkdir(name.c_str(), 0755) != 0) {
if (errno != EEXIST) {
- result = IOError("While mkdir if missing", name, errno);
+ return IOError("While mkdir if missing", name, errno);
} else if (!DirExists(name)) { // Check that name is actually a
// directory.
// Message is taken from mkdir
- result =
- IOStatus::IOError("`" + name + "' exists but is not a directory");
+ return IOStatus::IOError("`" + name +
+ "' exists but is not a directory");
}
}
- return result;
+ return IOStatus::OK();
}
IOStatus DeleteDir(const std::string& name, const IOOptions& /*opts*/,
IODebugContext* /*dbg*/) override {
- IOStatus result;
if (rmdir(name.c_str()) != 0) {
- result = IOError("file rmdir", name, errno);
+ return IOError("file rmdir", name, errno);
}
- return result;
+ return IOStatus::OK();
}
IOStatus GetFileSize(const std::string& fname, const IOOptions& /*opts*/,
uint64_t* size, IODebugContext* /*dbg*/) override {
- IOStatus s;
struct stat sbuf;
if (stat(fname.c_str(), &sbuf) != 0) {
*size = 0;
- s = IOError("while stat a file for size", fname, errno);
+ return IOError("while stat a file for size", fname, errno);
} else {
*size = sbuf.st_size;
}
- return s;
+ return IOStatus::OK();
}
IOStatus GetFileModificationTime(const std::string& fname,
@@ -697,24 +694,22 @@ class PosixFileSystem : public FileSystem {
IOStatus RenameFile(const std::string& src, const std::string& target,
const IOOptions& /*opts*/,
IODebugContext* /*dbg*/) override {
- IOStatus result;
if (rename(src.c_str(), target.c_str()) != 0) {
- result = IOError("While renaming a file to " + target, src, errno);
+ return IOError("While renaming a file to " + target, src, errno);
}
- return result;
+ return IOStatus::OK();
}
IOStatus LinkFile(const std::string& src, const std::string& target,
const IOOptions& /*opts*/,
IODebugContext* /*dbg*/) override {
- IOStatus result;
if (link(src.c_str(), target.c_str()) != 0) {
if (errno == EXDEV) {
return IOStatus::NotSupported("No cross FS links allowed");
}
- result = IOError("while link file to " + target, src, errno);
+ return IOError("while link file to " + target, src, errno);
}
- return result;
+ return IOStatus::OK();
}
IOStatus NumFileLinks(const std::string& fname, const IOOptions& /*opts*/,
@@ -751,12 +746,11 @@ class PosixFileSystem : public FileSystem {
IOStatus LockFile(const std::string& fname, const IOOptions& /*opts*/,
FileLock** lock, IODebugContext* /*dbg*/) override {
*lock = nullptr;
- IOStatus result;
LockHoldingInfo lhi;
int64_t current_time = 0;
// Ignore status code as the time is only used for error message.
- Env::Default()->GetCurrentTime(¤t_time);
+ Env::Default()->GetCurrentTime(¤t_time).PermitUncheckedError();
lhi.acquire_time = current_time;
lhi.acquiring_thread = Env::Default()->GetThreadID();
@@ -784,6 +778,7 @@ class PosixFileSystem : public FileSystem {
fname, errno);
}
+ IOStatus result = IOStatus::OK();
int fd;
int flags = cloexec_flags(O_RDWR | O_CREAT, nullptr);
@@ -861,7 +856,7 @@ class PosixFileSystem : public FileSystem {
// Directory may already exist
{
IOOptions opts;
- CreateDir(*result, opts, nullptr);
+ return CreateDirIfMissing(*result, opts, nullptr);
}
return IOStatus::OK();
}
diff --git a/env/io_posix.cc b/env/io_posix.cc
index f5848f9f5a..689d898120 100644
--- a/env/io_posix.cc
+++ b/env/io_posix.cc
@@ -990,7 +990,8 @@ PosixMmapFile::PosixMmapFile(const std::string& fname, int fd, size_t page_size,
PosixMmapFile::~PosixMmapFile() {
if (fd_ >= 0) {
- PosixMmapFile::Close(IOOptions(), nullptr);
+ IOStatus s = PosixMmapFile::Close(IOOptions(), nullptr);
+ s.PermitUncheckedError();
}
}
@@ -1152,7 +1153,8 @@ PosixWritableFile::PosixWritableFile(const std::string& fname, int fd,
PosixWritableFile::~PosixWritableFile() {
if (fd_ >= 0) {
- PosixWritableFile::Close(IOOptions(), nullptr);
+ IOStatus s = PosixWritableFile::Close(IOOptions(), nullptr);
+ s.PermitUncheckedError();
}
}
@@ -1394,7 +1396,8 @@ PosixRandomRWFile::PosixRandomRWFile(const std::string& fname, int fd,
PosixRandomRWFile::~PosixRandomRWFile() {
if (fd_ >= 0) {
- Close(IOOptions(), nullptr);
+ IOStatus s = Close(IOOptions(), nullptr);
+ s.PermitUncheckedError();
}
}
diff --git a/env/mock_env.cc b/env/mock_env.cc
index 16e427949f..6ecb52e1b8 100644
--- a/env/mock_env.cc
+++ b/env/mock_env.cc
@@ -600,7 +600,7 @@ Status MockEnv::CreateDir(const std::string& dirname) {
}
Status MockEnv::CreateDirIfMissing(const std::string& dirname) {
- CreateDir(dirname);
+ CreateDir(dirname).PermitUncheckedError();
return Status::OK();
}
diff --git a/file/sst_file_manager_impl.cc b/file/sst_file_manager_impl.cc
index 35056429e0..494deaf634 100644
--- a/file/sst_file_manager_impl.cc
+++ b/file/sst_file_manager_impl.cc
@@ -509,7 +509,7 @@ SstFileManager* NewSstFileManager(Env* env, std::shared_ptr<FileSystem> fs,
// trash_dir is deprecated and not needed anymore, but if user passed it
// we will still remove files in it.
- Status s;
+ Status s = Status::OK();
if (delete_existing_trash && trash_dir != "") {
std::vector<std::string> files_in_trash;
s = fs->GetChildren(trash_dir, IOOptions(), &files_in_trash, nullptr);
@@ -532,6 +532,9 @@ SstFileManager* NewSstFileManager(Env* env, std::shared_ptr<FileSystem> fs,
if (status) {
*status = s;
+ } else {
+ // No one passed us a Status, so they must not care about the error...
+ s.PermitUncheckedError();
}
return res;
diff --git a/include/rocksdb/file_system.h b/include/rocksdb/file_system.h
index 559d210d69..8292f0ee24 100644
--- a/include/rocksdb/file_system.h
+++ b/include/rocksdb/file_system.h
@@ -869,7 +869,8 @@ class FSWritableFile {
size_t num_spanned_blocks =
new_last_preallocated_block - last_preallocated_block_;
Allocate(block_size * last_preallocated_block_,
- block_size * num_spanned_blocks, options, dbg);
+ block_size * num_spanned_blocks, options, dbg)
+ .PermitUncheckedError();
last_preallocated_block_ = new_last_preallocated_block;
}
}
diff --git a/test_util/testharness.cc b/test_util/testharness.cc
index d38d430806..50e105c51d 100644
--- a/test_util/testharness.cc
+++ b/test_util/testharness.cc
@@ -26,7 +26,7 @@ namespace test {
std::string TmpDir(Env* env) {
std::string dir;
Status s = env->GetTestDirectory(&dir);
- EXPECT_TRUE(s.ok()) << s.ToString();
+ EXPECT_OK(s);
return dir;
}
diff --git a/utilities/env_timed_test.cc b/utilities/env_timed_test.cc
index f1695185e6..7bc8f41b07 100644
--- a/utilities/env_timed_test.cc
+++ b/utilities/env_timed_test.cc
@@ -21,7 +21,7 @@ TEST_F(TimedEnvTest, BasicTest) {
std::unique_ptr<Env> mem_env(NewMemEnv(Env::Default()));
std::unique_ptr<Env> timed_env(NewTimedEnv(mem_env.get()));
std::unique_ptr<WritableFile> writable_file;
- timed_env->NewWritableFile("f", &writable_file, EnvOptions());
+ ASSERT_OK(timed_env->NewWritableFile("f", &writable_file, EnvOptions()));
ASSERT_GT(get_perf_context()->env_new_writable_file_nanos, 0);
}
diff --git a/utilities/fault_injection_fs.h b/utilities/fault_injection_fs.h
index 07b7efdad0..d856cba5db 100644
--- a/utilities/fault_injection_fs.h
+++ b/utilities/fault_injection_fs.h
@@ -157,13 +157,13 @@ class TestFSDirectory : public FSDirectory {
class FaultInjectionTestFS : public FileSystemWrapper {
public:
- explicit FaultInjectionTestFS(std::shared_ptr<FileSystem> base)
+ explicit FaultInjectionTestFS(const std::shared_ptr<FileSystem>& base)
: FileSystemWrapper(base),
filesystem_active_(true),
filesystem_writable_(false),
- thread_local_error_(
- new ThreadLocalPtr(DeleteThreadLocalErrorContext)) {}
- virtual ~FaultInjectionTestFS() {}
+ thread_local_error_(new ThreadLocalPtr(DeleteThreadLocalErrorContext)) {
+ }
+ virtual ~FaultInjectionTestFS() { error_.PermitUncheckedError(); }
const char* Name() const override { return "FaultInjectionTestFS"; }