From d9d190742c0a8a3d5ff0b0867383bd38c1d0f5f0 Mon Sep 17 00:00:00 2001 From: mrambacher 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 847e7b47dfd..e69ac2639c1 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 ea0d12296f0..5fd76977667 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 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 194853b677d..f72e805ac37 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 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 d0019c37e5c..4bc2f9a2507 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 2a2a42dd815..c7eac4c4cf5 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 c6677f6ea9d..46f1b42fdff 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* 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 73b5c95a692..dae14f01559 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 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 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 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 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 b00db6f6787..8b06225df0d 100644 --- a/env/fs_posix.cc +++ b/env/fs_posix.cc @@ -201,7 +201,7 @@ class PosixFileSystem : public FileSystem { std::unique_ptr* 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 f5848f9f5a1..689d898120b 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 16e427949fa..6ecb52e1b8e 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 35056429e0d..494deaf634e 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 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 files_in_trash; s = fs->GetChildren(trash_dir, IOOptions(), &files_in_trash, nullptr); @@ -532,6 +532,9 @@ SstFileManager* NewSstFileManager(Env* env, std::shared_ptr 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 559d210d69f..8292f0ee243 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 d38d4308067..50e105c51d3 100644 --- a/test_util/testharness.cc +++ b/test_util/testharness.cc @@ -26,7 +26,7 @@ ::testing::AssertionResult AssertStatus(const char* s_expr, const Status& s) { 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 f1695185e60..7bc8f41b07c 100644 --- a/utilities/env_timed_test.cc +++ b/utilities/env_timed_test.cc @@ -21,7 +21,7 @@ TEST_F(TimedEnvTest, BasicTest) { std::unique_ptr mem_env(NewMemEnv(Env::Default())); std::unique_ptr timed_env(NewTimedEnv(mem_env.get())); std::unique_ptr 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 07b7efdad03..d856cba5dbe 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 base) + explicit FaultInjectionTestFS(const std::shared_ptr& 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"; }