From ea49b27d062c4bc998616cef7944f7f9088a327d Mon Sep 17 00:00:00 2001 From: cmumford Date: Tue, 19 Mar 2019 17:30:42 -0700 Subject: [PATCH] Switch corruption_test to use InMemEnv. This change switches corruption_test, which previously used direct file I/O to corrupt table files for open databases, to use InMemEnv. Using an Env eliminates some platform dependencies thus simplifying the tests. Also removed EnvWindowsTestHelper::RelaxFilePermissions(). This was only added because the Windows Env opens files for exclusive access. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=239305329 --- db/corruption_test.cc | 58 +++++++++--------------------------------- util/env_windows.cc | 20 --------------- util/env_windows_test_helper.h | 5 ---- util/testutil.h | 6 ++++- 4 files changed, 17 insertions(+), 72 deletions(-) diff --git a/db/corruption_test.cc b/db/corruption_test.cc index 98aaf8c..d50785a 100644 --- a/db/corruption_test.cc +++ b/db/corruption_test.cc @@ -4,12 +4,8 @@ #include "leveldb/db.h" -#include -#include -#include #include #include "leveldb/cache.h" -#include "leveldb/env.h" #include "leveldb/table.h" #include "leveldb/write_batch.h" #include "db/db_impl.h" @@ -20,10 +16,6 @@ #include "util/testharness.h" #include "util/testutil.h" -#if defined(LEVELDB_PLATFORM_WINDOWS) -#include "util/env_windows_test_helper.h" -#endif // defined(LEVELDB_PLATFORM_WINDOWS) - namespace leveldb { static const int kValueSize = 1000; @@ -36,22 +28,11 @@ class CorruptionTest { Options options_; DB* db_; -#if defined(LEVELDB_PLATFORM_WINDOWS) - static void SetFileLimits(int mmap_limit) { - EnvWindowsTestHelper::SetReadOnlyMMapLimit(mmap_limit); - } - - // TODO(cmumford): Modify corruption_test to use MemEnv and remove. - static void RelaxFilePermissions() { - EnvWindowsTestHelper::RelaxFilePermissions(); - } -#endif // defined(LEVELDB_PLATFORM_WINDOWS) - CorruptionTest() { tiny_cache_ = NewLRUCache(100); options_.env = &env_; options_.block_cache = tiny_cache_; - dbname_ = test::TmpDir() + "/corruption_test"; + dbname_ = "/memenv/corruption_test"; DestroyDB(dbname_, options_); db_ = nullptr; @@ -62,7 +43,6 @@ class CorruptionTest { ~CorruptionTest() { delete db_; - DestroyDB(dbname_, Options()); delete tiny_cache_; } @@ -141,7 +121,7 @@ class CorruptionTest { void Corrupt(FileType filetype, int offset, int bytes_to_corrupt) { // Pick file to corrupt std::vector filenames; - ASSERT_OK(env_.GetChildren(dbname_, &filenames)); + ASSERT_OK(env_.target()->GetChildren(dbname_, &filenames)); uint64_t number; FileType type; std::string fname; @@ -156,35 +136,32 @@ class CorruptionTest { } ASSERT_TRUE(!fname.empty()) << filetype; - struct stat sbuf; - if (stat(fname.c_str(), &sbuf) != 0) { - const char* msg = strerror(errno); - ASSERT_TRUE(false) << fname << ": " << msg; - } + uint64_t file_size; + ASSERT_OK(env_.target()->GetFileSize(fname, &file_size)); if (offset < 0) { // Relative to end of file; make it absolute - if (-offset > sbuf.st_size) { + if (-offset > file_size) { offset = 0; } else { - offset = sbuf.st_size + offset; + offset = file_size + offset; } } - if (offset > sbuf.st_size) { - offset = sbuf.st_size; + if (offset > file_size) { + offset = file_size; } - if (offset + bytes_to_corrupt > sbuf.st_size) { - bytes_to_corrupt = sbuf.st_size - offset; + if (offset + bytes_to_corrupt > file_size) { + bytes_to_corrupt = file_size - offset; } // Do it std::string contents; - Status s = ReadFileToString(Env::Default(), fname, &contents); + Status s = ReadFileToString(env_.target(), fname, &contents); ASSERT_TRUE(s.ok()) << s.ToString(); for (int i = 0; i < bytes_to_corrupt; i++) { contents[i + offset] ^= 0x80; } - s = WriteStringToFile(Env::Default(), contents, fname); + s = WriteStringToFile(env_.target(), contents, fname); ASSERT_TRUE(s.ok()) << s.ToString(); } @@ -385,16 +362,5 @@ TEST(CorruptionTest, UnrelatedKeys) { } // namespace leveldb int main(int argc, char** argv) { -#if defined(LEVELDB_PLATFORM_WINDOWS) - // When Windows maps the contents of a file into memory, even if read/write, - // subsequent attempts to open that file for write access will fail. Forcing - // all RandomAccessFile instances to use base file I/O (e.g. ReadFile) - // allows these tests to open files in order to corrupt their contents. - leveldb::CorruptionTest::SetFileLimits(0); - - // Allow this test to write to (and corrupt) files which are normally - // open for exclusive read access. - leveldb::CorruptionTest::RelaxFilePermissions(); -#endif // defined(LEVELDB_PLATFORM_WINDOWS) return leveldb::test::RunAllTests(); } diff --git a/util/env_windows.cc b/util/env_windows.cc index 3b4496b..93555a8 100644 --- a/util/env_windows.cc +++ b/util/env_windows.cc @@ -43,9 +43,6 @@ constexpr int kDefaultMmapLimit = sizeof(void*) >= 8 ? 1000 : 0; // Modified by EnvWindowsTestHelper::SetReadOnlyMMapLimit(). int g_mmap_limit = kDefaultMmapLimit; -// Relax some file access permissions for testing. -bool g_relax_permissions = false; - std::string GetWindowsErrorMessage(DWORD error_code) { std::string message; char* error_text = nullptr; @@ -366,10 +363,6 @@ class WindowsEnv : public Env { *result = nullptr; DWORD desired_access = GENERIC_READ; DWORD share_mode = FILE_SHARE_READ; - if (g_relax_permissions) { - desired_access |= GENERIC_WRITE; - share_mode |= FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE; - } ScopedHandle handle = ::CreateFileA(fname.c_str(), desired_access, share_mode, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); @@ -385,10 +378,6 @@ class WindowsEnv : public Env { *result = nullptr; DWORD desired_access = GENERIC_READ; DWORD share_mode = FILE_SHARE_READ; - if (g_relax_permissions) { - // desired_access |= GENERIC_WRITE; - share_mode |= FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE; - } DWORD file_flags = FILE_ATTRIBUTE_READONLY; ScopedHandle handle = @@ -433,10 +422,6 @@ class WindowsEnv : public Env { WritableFile** result) override { DWORD desired_access = GENERIC_WRITE; DWORD share_mode = 0; - if (g_relax_permissions) { - desired_access |= GENERIC_READ; - share_mode |= FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE; - } ScopedHandle handle = ::CreateFileA(fname.c_str(), desired_access, share_mode, nullptr, @@ -721,11 +706,6 @@ void EnvWindowsTestHelper::SetReadOnlyMMapLimit(int limit) { g_mmap_limit = limit; } -void EnvWindowsTestHelper::RelaxFilePermissions() { - assert(default_env == nullptr); - g_relax_permissions = true; -} - Env* Env::Default() { std::call_once(once, InitDefaultEnv); return default_env; diff --git a/util/env_windows_test_helper.h b/util/env_windows_test_helper.h index 5ffbe44..e6f6020 100644 --- a/util/env_windows_test_helper.h +++ b/util/env_windows_test_helper.h @@ -18,11 +18,6 @@ class EnvWindowsTestHelper { // Set the maximum number of read-only files that will be mapped via mmap. // Must be called before creating an Env. static void SetReadOnlyMMapLimit(int limit); - - // Relax file permissions for tests. This results in most files being opened - // with read-write permissions. This is helpful for corruption tests that - // need to corrupt the database files for open databases. - static void RelaxFilePermissions(); }; } // namespace leveldb diff --git a/util/testutil.h b/util/testutil.h index dc77ac3..3934242 100644 --- a/util/testutil.h +++ b/util/testutil.h @@ -5,6 +5,7 @@ #ifndef STORAGE_LEVELDB_UTIL_TESTUTIL_H_ #define STORAGE_LEVELDB_UTIL_TESTUTIL_H_ +#include "helpers/memenv/memenv.h" #include "leveldb/env.h" #include "leveldb/slice.h" #include "util/random.h" @@ -32,9 +33,12 @@ class ErrorEnv : public EnvWrapper { bool writable_file_error_; int num_writable_file_errors_; - ErrorEnv() : EnvWrapper(Env::Default()), + ErrorEnv() : EnvWrapper(NewMemEnv(Env::Default())), writable_file_error_(false), num_writable_file_errors_(0) { } + ~ErrorEnv() override { + delete target(); + } virtual Status NewWritableFile(const std::string& fname, WritableFile** result) {