From 3bd9db420e1fdb004e77c79570640c42206ba9c8 Mon Sep 17 00:00:00 2001 From: Islam AbdelRahman Date: Wed, 12 Aug 2015 10:18:59 -0700 Subject: [PATCH] [Cleanup] Remove RandomRWFile Summary: RandomRWFile is not used anywhere in out code base, this patch remove RandomRWFile Test Plan: make check -j64 USE_CLANG=1 make all -j64 OPT=-DROCKSDB_LITE make release -j64 Reviewers: sdong, yhchiang, anthony, kradhakrishnan, rven, igor Reviewed By: igor Subscribers: dhruba Differential Revision: https://reviews.facebook.net/D44091 --- HISTORY.md | 5 + hdfs/env_hdfs.h | 10 -- include/rocksdb/env.h | 63 --------- port/win/env_win.cc | 144 --------------------- util/env_hdfs.cc | 6 - util/env_posix.cc | 130 ------------------- util/env_test.cc | 22 ---- util/file_reader_writer.cc | 53 -------- util/file_reader_writer.h | 20 --- util/mock_env.cc | 6 - util/mock_env.h | 4 - utilities/backupable/backupable_db_test.cc | 20 ++- 12 files changed, 13 insertions(+), 470 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index d0eaac53f4..21fe35b8c5 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,5 +1,10 @@ # Rocksdb Change Log +## Unreleased + +### Public API Changes +* Removed class Env::RandomRWFile and Env::NewRandomRWFile(). + ## 3.13.0 (8/6/2015) ### New Features * RollbackToSavePoint() in WriteBatch/WriteBatchWithIndex diff --git a/hdfs/env_hdfs.h b/hdfs/env_hdfs.h index efa63e53c4..e1e9430934 100644 --- a/hdfs/env_hdfs.h +++ b/hdfs/env_hdfs.h @@ -66,10 +66,6 @@ class HdfsEnv : public Env { std::unique_ptr* result, const EnvOptions& options); - virtual Status NewRandomRWFile(const std::string& fname, - std::unique_ptr* result, - const EnvOptions& options); - virtual Status NewDirectory(const std::string& name, std::unique_ptr* result); @@ -268,12 +264,6 @@ class HdfsEnv : public Env { return notsup; } - virtual Status NewRandomRWFile(const std::string& fname, - unique_ptr* result, - const EnvOptions& options) override { - return notsup; - } - virtual Status NewDirectory(const std::string& name, unique_ptr* result) override { return notsup; diff --git a/include/rocksdb/env.h b/include/rocksdb/env.h index b9c4db045f..54457853ff 100644 --- a/include/rocksdb/env.h +++ b/include/rocksdb/env.h @@ -40,7 +40,6 @@ class RandomAccessFile; class SequentialFile; class Slice; class WritableFile; -class RandomRWFile; class Directory; struct DBOptions; class RateLimiter; @@ -137,15 +136,6 @@ class Env { unique_ptr* result, const EnvOptions& options) = 0; - // Create an object that both reads and writes to a file on - // specified offsets (random access). If file already exists, - // does not overwrite it. On success, stores a pointer to the - // new file in *result and returns OK. On failure stores nullptr - // in *result and returns non-OK. - virtual Status NewRandomRWFile(const std::string& fname, - unique_ptr* result, - const EnvOptions& options) = 0; - // Create an object that represents a directory. Will fail if directory // doesn't exist. If the directory exists, it will open the directory // and create a new Directory object. @@ -568,55 +558,6 @@ class WritableFile { Env::IOPriority io_priority_; }; -// A file abstraction for random reading and writing. -class RandomRWFile { - public: - RandomRWFile() {} - virtual ~RandomRWFile() {} - - // Write data from Slice data to file starting from offset - // Returns IOError on failure, but does not guarantee - // atomicity of a write. Returns OK status on success. - // - // Safe for concurrent use. - virtual Status Write(uint64_t offset, const Slice& data) = 0; - // Read up to "n" bytes from the file starting at "offset". - // "scratch[0..n-1]" may be written by this routine. Sets "*result" - // to the data that was read (including if fewer than "n" bytes were - // successfully read). May set "*result" to point at data in - // "scratch[0..n-1]", so "scratch[0..n-1]" must be live when - // "*result" is used. If an error was encountered, returns a non-OK - // status. - // - // Safe for concurrent use by multiple threads. - virtual Status Read(uint64_t offset, size_t n, Slice* result, - char* scratch) const = 0; - virtual Status Close() = 0; // closes the file - virtual Status Sync() = 0; // sync data - - /* - * Sync data and/or metadata as well. - * By default, sync only data. - * Override this method for environments where we need to sync - * metadata as well. - */ - virtual Status Fsync() { - return Sync(); - } - - /* - * Pre-allocate space for a file. - */ - virtual Status Allocate(off_t offset, off_t len) { - return Status::OK(); - } - - private: - // No copying allowed - RandomRWFile(const RandomRWFile&); - void operator=(const RandomRWFile&); -}; - // Directory object represents collection of files and implements // filesystem operations that can be executed on directories. class Directory { @@ -766,10 +707,6 @@ class EnvWrapper : public Env { const EnvOptions& options) override { return target_->NewWritableFile(f, r, options); } - Status NewRandomRWFile(const std::string& f, unique_ptr* r, - const EnvOptions& options) override { - return target_->NewRandomRWFile(f, r, options); - } virtual Status NewDirectory(const std::string& name, unique_ptr* result) override { return target_->NewDirectory(name, result); diff --git a/port/win/env_win.cc b/port/win/env_win.cc index de31b0b9fb..53d1ccf361 100644 --- a/port/win/env_win.cc +++ b/port/win/env_win.cc @@ -207,118 +207,6 @@ inline Status ftruncate(const std::string& filename, HANDLE hFile, return status; } -class WinRandomRWFile : public RandomRWFile { - const std::string filename_; - HANDLE hFile_; - bool pending_fsync_; - - public: - WinRandomRWFile(const std::string& fname, HANDLE hFile, - const EnvOptions& options) - : filename_(fname), hFile_(hFile), pending_fsync_(false) { - assert(!options.use_mmap_writes && !options.use_mmap_reads); - } - - ~WinRandomRWFile() { - if (hFile_ != INVALID_HANDLE_VALUE && hFile_ != NULL) { - ::CloseHandle(hFile_); - } - } - - virtual Status Write(uint64_t offset, const Slice& data) override { - const char* src = data.data(); - size_t left = data.size(); - - pending_fsync_ = true; - - SSIZE_T done = 0; - { - IOSTATS_TIMER_GUARD(write_nanos); - done = pwrite(hFile_, src, left, offset); - } - - if (done < 0) { - return IOErrorFromWindowsError("pwrite failed to: " + filename_, - GetLastError()); - } - - IOSTATS_ADD(bytes_written, done); - - left -= done; - src += done; - offset += done; - - return Status::OK(); - } - - virtual Status Read(uint64_t offset, size_t n, Slice* result, - char* scratch) const override { - Status s; - - SSIZE_T r = -1; - char* ptr = scratch; - size_t left = n; - - while (left > 0) { - { - IOSTATS_TIMER_GUARD(read_nanos); - r = pread(hFile_, ptr, n, offset); - } - - if (r <= 0) { - break; - } - - ptr += r; - offset += r; - left -= r; - } - - IOSTATS_ADD_IF_POSITIVE(bytes_read, n - left); - - *result = Slice(scratch, (r < 0) ? 0 : r); - - if (r < 0) { - s = IOErrorFromWindowsError("pread failed from: " + filename_, - GetLastError()); - } - return s; - } - - virtual Status Close() override { - Status s = Status::OK(); - if (hFile_ != INVALID_HANDLE_VALUE && ::CloseHandle(hFile_) == FALSE) { - s = IOErrorFromWindowsError("Failed to close file: " + filename_, - GetLastError()); - } - hFile_ = INVALID_HANDLE_VALUE; - return s; - } - - virtual Status Sync() override { - if (pending_fsync_ && fsync(hFile_) < 0) { - return IOErrorFromWindowsError( - "Failed to Sync() buffers for: " + filename_, GetLastError()); - } - pending_fsync_ = false; - return Status::OK(); - } - - virtual Status Fsync() override { - if (pending_fsync_ && fsync(hFile_) < 0) { - return IOErrorFromWindowsError("Failed to Fsync() for: " + filename_, - GetLastError()); - } - pending_fsync_ = false; - return Status::OK(); - } - - virtual Status Allocate(off_t offset, off_t len) override { - IOSTATS_TIMER_GUARD(allocate_nanos); - return fallocate(filename_, hFile_, len); - } -}; - // mmap() based random-access class WinMmapReadableFile : public RandomAccessFile { const std::string fileName_; @@ -1616,38 +1504,6 @@ class WinEnv : public Env { return s; } - virtual Status NewRandomRWFile(const std::string& fname, - std::unique_ptr* result, - const EnvOptions& options) override { - result->reset(); - - // no support for mmap yet (same as POSIX env) - if (options.use_mmap_writes || options.use_mmap_reads) { - return Status::NotSupported("No support for mmap read/write yet"); - } - - Status s; - - HANDLE hFile = 0; - { - IOSTATS_TIMER_GUARD(open_nanos); - hFile = CreateFileA(fname.c_str(), GENERIC_READ | GENERIC_WRITE, - FILE_SHARE_READ, NULL, - OPEN_ALWAYS, // Posix env specifies O_CREAT, it will - // open existing file or create new - FILE_ATTRIBUTE_NORMAL, NULL); - } - - if (INVALID_HANDLE_VALUE == hFile) { - auto lastError = GetLastError(); - s = IOErrorFromWindowsError( - "Failed to Open/Create NewRandomRWFile" + fname, lastError); - } else { - result->reset(new WinRandomRWFile(fname, hFile, options)); - } - return s; - } - virtual Status NewDirectory(const std::string& name, std::unique_ptr* result) override { Status s; diff --git a/util/env_hdfs.cc b/util/env_hdfs.cc index e0016c66f2..77055aafb4 100644 --- a/util/env_hdfs.cc +++ b/util/env_hdfs.cc @@ -415,12 +415,6 @@ Status HdfsEnv::NewWritableFile(const std::string& fname, return Status::OK(); } -Status HdfsEnv::NewRandomRWFile(const std::string& fname, - unique_ptr* result, - const EnvOptions& options) { - return Status::NotSupported("NewRandomRWFile not supported on HdfsEnv"); -} - class HdfsDirectory : public Directory { public: explicit HdfsDirectory(int fd) : fd_(fd) {} diff --git a/util/env_posix.cc b/util/env_posix.cc index 13ce1d35d8..7aa0fbcfab 100644 --- a/util/env_posix.cc +++ b/util/env_posix.cc @@ -725,113 +725,6 @@ class PosixWritableFile : public WritableFile { #endif }; -class PosixRandomRWFile : public RandomRWFile { - private: - const std::string filename_; - int fd_; -#ifdef ROCKSDB_FALLOCATE_PRESENT - bool fallocate_with_keep_size_; -#endif - - public: - PosixRandomRWFile(const std::string& fname, int fd, const EnvOptions& options) - : filename_(fname), fd_(fd) { -#ifdef ROCKSDB_FALLOCATE_PRESENT - fallocate_with_keep_size_ = options.fallocate_with_keep_size; -#endif - assert(!options.use_mmap_writes && !options.use_mmap_reads); - } - - ~PosixRandomRWFile() { - if (fd_ >= 0) { - Close(); - } - } - - virtual Status Write(uint64_t offset, const Slice& data) override { - const char* src = data.data(); - size_t left = data.size(); - Status s; - - while (left != 0) { - ssize_t done = pwrite(fd_, src, left, offset); - if (done < 0) { - if (errno == EINTR) { - continue; - } - return IOError(filename_, errno); - } - - left -= done; - src += done; - offset += done; - } - - return Status::OK(); - } - - virtual Status Read(uint64_t offset, size_t n, Slice* result, - char* scratch) const override { - Status s; - ssize_t r = -1; - size_t left = n; - char* ptr = scratch; - while (left > 0) { - r = pread(fd_, ptr, left, static_cast(offset)); - if (r <= 0) { - if (errno == EINTR) { - continue; - } - break; - } - ptr += r; - offset += r; - left -= r; - } - *result = Slice(scratch, (r < 0) ? 0 : n - left); - if (r < 0) { - s = IOError(filename_, errno); - } - return s; - } - - virtual Status Close() override { - Status s = Status::OK(); - if (fd_ >= 0 && close(fd_) < 0) { - s = IOError(filename_, errno); - } - fd_ = -1; - return s; - } - - virtual Status Sync() override { - if (fdatasync(fd_) < 0) { - return IOError(filename_, errno); - } - return Status::OK(); - } - - virtual Status Fsync() override { - if (fsync(fd_) < 0) { - return IOError(filename_, errno); - } - return Status::OK(); - } - -#ifdef ROCKSDB_FALLOCATE_PRESENT - virtual Status Allocate(off_t offset, off_t len) override { - IOSTATS_TIMER_GUARD(allocate_nanos); - int alloc_status = fallocate( - fd_, fallocate_with_keep_size_ ? FALLOC_FL_KEEP_SIZE : 0, offset, len); - if (alloc_status == 0) { - return Status::OK(); - } else { - return IOError(filename_, errno); - } - } -#endif -}; - class PosixDirectory : public Directory { public: explicit PosixDirectory(int fd) : fd_(fd) {} @@ -1015,29 +908,6 @@ class PosixEnv : public Env { return s; } - virtual Status NewRandomRWFile(const std::string& fname, - unique_ptr* result, - const EnvOptions& options) override { - result->reset(); - // no support for mmap yet - if (options.use_mmap_writes || options.use_mmap_reads) { - return Status::NotSupported("No support for mmap read/write yet"); - } - Status s; - int fd; - { - IOSTATS_TIMER_GUARD(open_nanos); - fd = open(fname.c_str(), O_CREAT | O_RDWR, 0644); - } - if (fd < 0) { - s = IOError(fname, errno); - } else { - SetFD_CLOEXEC(fd, &options); - result->reset(new PosixRandomRWFile(fname, fd, options)); - } - return s; - } - virtual Status NewDirectory(const std::string& name, unique_ptr* result) override { result->reset(); diff --git a/util/env_test.cc b/util/env_test.cc index 1ae50fdbc0..0e4feabfdd 100644 --- a/util/env_test.cc +++ b/util/env_test.cc @@ -824,28 +824,6 @@ TEST_F(EnvPosixTest, InvalidateCache) { #endif // not TRAVIS #endif // OS_LINUX -TEST_F(EnvPosixTest, PosixRandomRWFileTest) { - EnvOptions soptions; - soptions.use_mmap_writes = soptions.use_mmap_reads = false; - std::string fname = test::TmpDir() + "/" + "testfile"; - - unique_ptr file; - ASSERT_OK(env_->NewRandomRWFile(fname, &file, soptions)); - // If you run the unit test on tmpfs, then tmpfs might not - // support fallocate. It is still better to trigger that - // code-path instead of eliminating it completely. - file.get()->Allocate(0, 10*1024*1024); - ASSERT_OK(file.get()->Write(100, Slice("Hello world"))); - ASSERT_OK(file.get()->Write(105, Slice("Hello world"))); - ASSERT_OK(file.get()->Sync()); - ASSERT_OK(file.get()->Fsync()); - char scratch[100]; - Slice result; - ASSERT_OK(file.get()->Read(100, 16, &result, scratch)); - ASSERT_EQ(result.compare("HelloHello world"), 0); - ASSERT_OK(file.get()->Close()); -} - class TestLogger : public Logger { public: using Logger::Logv; diff --git a/util/file_reader_writer.cc b/util/file_reader_writer.cc index e56d566ef0..6a91dc2987 100644 --- a/util/file_reader_writer.cc +++ b/util/file_reader_writer.cc @@ -205,57 +205,4 @@ size_t WritableFileWriter::RequestToken(size_t bytes) { } return bytes; } - -Status RandomRWFileAccessor::Write(uint64_t offset, const Slice& data) { - Status s; - pending_sync_ = true; - pending_fsync_ = true; - - { - IOSTATS_TIMER_GUARD(write_nanos); - s = random_rw_file_->Write(offset, data); - if (!s.ok()) { - return s; - } - } - IOSTATS_ADD(bytes_written, data.size()); - - return s; -} - -Status RandomRWFileAccessor::Read(uint64_t offset, size_t n, Slice* result, - char* scratch) const { - Status s; - { - IOSTATS_TIMER_GUARD(read_nanos); - s = random_rw_file_->Read(offset, n, result, scratch); - if (!s.ok()) { - return s; - } - } - IOSTATS_ADD_IF_POSITIVE(bytes_read, result->size()); - return s; -} - -Status RandomRWFileAccessor::Close() { return random_rw_file_->Close(); } - -Status RandomRWFileAccessor::Sync(bool use_fsync) { - Status s; - if (pending_sync_) { - if (use_fsync) { - s = random_rw_file_->Fsync(); - } else { - s = random_rw_file_->Sync(); - } - if (!s.ok()) { - return s; - } - } - if (use_fsync) { - pending_fsync_ = false; - } - pending_sync_ = false; - - return s; -} } // namespace rocksdb diff --git a/util/file_reader_writer.h b/util/file_reader_writer.h index a16acb266a..42de8a0e49 100644 --- a/util/file_reader_writer.h +++ b/util/file_reader_writer.h @@ -104,24 +104,4 @@ class WritableFileWriter { size_t RequestToken(size_t bytes); Status SyncInternal(bool use_fsync); }; - -class RandomRWFileAccessor { - private: - std::unique_ptr random_rw_file_; - bool pending_sync_; - bool pending_fsync_; - - public: - explicit RandomRWFileAccessor(std::unique_ptr&& f) - : random_rw_file_(std::move(f)), - pending_sync_(false), - pending_fsync_(false) {} - Status Write(uint64_t offset, const Slice& data); - - Status Read(uint64_t offset, size_t n, Slice* result, char* scratch) const; - - Status Close(); - - Status Sync(bool use_fsync); -}; } // namespace rocksdb diff --git a/util/mock_env.cc b/util/mock_env.cc index 3e62c040cb..088675071c 100644 --- a/util/mock_env.cc +++ b/util/mock_env.cc @@ -456,12 +456,6 @@ Status MockEnv::NewWritableFile(const std::string& fname, return Status::OK(); } -Status MockEnv::NewRandomRWFile(const std::string& fname, - unique_ptr* result, - const EnvOptions& options) { - return Status::OK(); -} - Status MockEnv::NewDirectory(const std::string& name, unique_ptr* result) { result->reset(new MockEnvDirectory()); diff --git a/util/mock_env.h b/util/mock_env.h index 4e10e7eac2..bcc74a7311 100644 --- a/util/mock_env.h +++ b/util/mock_env.h @@ -39,10 +39,6 @@ class MockEnv : public EnvWrapper { unique_ptr* result, const EnvOptions& env_options) override; - virtual Status NewRandomRWFile(const std::string& fname, - unique_ptr* result, - const EnvOptions& options) override; - virtual Status NewDirectory(const std::string& name, unique_ptr* result) override; diff --git a/utilities/backupable/backupable_db_test.cc b/utilities/backupable/backupable_db_test.cc index 8f2bd4ec2a..8cd4d356c1 100644 --- a/utilities/backupable/backupable_db_test.cc +++ b/utilities/backupable/backupable_db_test.cc @@ -283,26 +283,22 @@ class FileManager : public EnvWrapper { } Status CorruptFile(const std::string& fname, uint64_t bytes_to_corrupt) { - uint64_t size; - Status s = GetFileSize(fname, &size); + std::string file_contents; + Status s = ReadFileToString(this, fname, &file_contents); if (!s.ok()) { return s; } - unique_ptr file; - EnvOptions env_options; - env_options.use_mmap_writes = false; - s = NewRandomRWFile(fname, &file, env_options); + s = DeleteFile(fname); if (!s.ok()) { return s; } - RandomRWFileAccessor accessor(std::move(file)); - for (uint64_t i = 0; s.ok() && i < bytes_to_corrupt; ++i) { + + for (uint64_t i = 0; i < bytes_to_corrupt; ++i) { std::string tmp; - // write one random byte to a random position - s = accessor.Write(rnd_.Next() % size, - test::RandomString(&rnd_, 1, &tmp)); + test::RandomString(&rnd_, 1, &tmp); + file_contents[rnd_.Next() % file_contents.size()] = tmp[0]; } - return s; + return WriteToFile(fname, file_contents); } Status CorruptChecksum(const std::string& fname, bool appear_valid) {