From ee3dbdc0837210753eabb759c0f43230ec9cf431 Mon Sep 17 00:00:00 2001 From: akankshamahajan Date: Mon, 24 Oct 2022 17:54:14 -0700 Subject: [PATCH] Run clang-format on env/ folder (#10859) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/10859 Test Plan: CircleCI jobs Reviewed By: anand1976 Differential Revision: D40653839 Pulled By: akankshamahajan15 fbshipit-source-id: ce75205ee34ee3896a77a807d5c556886de78b01 --- env/env.cc | 35 ++-- env/env_basic_test.cc | 2 +- env/env_chroot.cc | 40 ++--- env/env_encryption.cc | 122 +++++++------- env/env_encryption_ctr.h | 2 +- env/env_test.cc | 353 ++++++++++++++++++++------------------- env/file_system.cc | 7 +- env/io_posix.cc | 5 +- env/io_posix.h | 3 +- env/mock_env.h | 1 + env/mock_env_test.cc | 8 +- 11 files changed, 290 insertions(+), 288 deletions(-) diff --git a/env/env.cc b/env/env.cc index c322acde9c..f70d1f0678 100644 --- a/env/env.cc +++ b/env/env.cc @@ -633,8 +633,7 @@ Env::Env(const std::shared_ptr& fs, const std::shared_ptr& clock) : thread_status_updater_(nullptr), file_system_(fs), system_clock_(clock) {} -Env::~Env() { -} +Env::~Env() {} Status Env::NewLogger(const std::string& fname, std::shared_ptr* result) { @@ -841,14 +840,11 @@ std::string Env::GenerateUniqueId() { return result; } -SequentialFile::~SequentialFile() { -} +SequentialFile::~SequentialFile() {} -RandomAccessFile::~RandomAccessFile() { -} +RandomAccessFile::~RandomAccessFile() {} -WritableFile::~WritableFile() { -} +WritableFile::~WritableFile() {} MemoryMappedFileBuffer::~MemoryMappedFileBuffer() {} @@ -865,16 +861,15 @@ Status Logger::Close() { Status Logger::CloseImpl() { return Status::NotSupported(); } -FileLock::~FileLock() { -} +FileLock::~FileLock() {} -void LogFlush(Logger *info_log) { +void LogFlush(Logger* info_log) { if (info_log) { info_log->Flush(); } } -static void Logv(Logger *info_log, const char* format, va_list ap) { +static void Logv(Logger* info_log, const char* format, va_list ap) { if (info_log && info_log->GetInfoLogLevel() <= InfoLogLevel::INFO_LEVEL) { info_log->Logv(InfoLogLevel::INFO_LEVEL, format, ap); } @@ -887,9 +882,10 @@ void Log(Logger* info_log, const char* format, ...) { va_end(ap); } -void Logger::Logv(const InfoLogLevel log_level, const char* format, va_list ap) { - static const char* kInfoLogLevelNames[5] = { "DEBUG", "INFO", "WARN", - "ERROR", "FATAL" }; +void Logger::Logv(const InfoLogLevel log_level, const char* format, + va_list ap) { + static const char* kInfoLogLevelNames[5] = {"DEBUG", "INFO", "WARN", "ERROR", + "FATAL"}; if (log_level < log_level_) { return; } @@ -906,7 +902,7 @@ void Logger::Logv(const InfoLogLevel log_level, const char* format, va_list ap) } else { char new_format[500]; snprintf(new_format, sizeof(new_format) - 1, "[%s] %s", - kInfoLogLevelNames[log_level], format); + kInfoLogLevelNames[log_level], format); Logv(new_format, ap); } @@ -919,7 +915,8 @@ void Logger::Logv(const InfoLogLevel log_level, const char* format, va_list ap) } } -static void Logv(const InfoLogLevel log_level, Logger *info_log, const char *format, va_list ap) { +static void Logv(const InfoLogLevel log_level, Logger* info_log, + const char* format, va_list ap) { if (info_log && info_log->GetInfoLogLevel() <= log_level) { if (log_level == InfoLogLevel::HEADER_LEVEL) { info_log->LogHeader(format, ap); @@ -937,7 +934,7 @@ void Log(const InfoLogLevel log_level, Logger* info_log, const char* format, va_end(ap); } -static void Headerv(Logger *info_log, const char *format, va_list ap) { +static void Headerv(Logger* info_log, const char* format, va_list ap) { if (info_log) { info_log->LogHeader(format, ap); } @@ -1106,7 +1103,7 @@ void AssignEnvOptions(EnvOptions* env_options, const DBOptions& options) { options.env->SanitizeEnvOptions(env_options); } -} +} // namespace EnvOptions Env::OptimizeForLogWrite(const EnvOptions& env_options, const DBOptions& db_options) const { diff --git a/env/env_basic_test.cc b/env/env_basic_test.cc index 2f7da52df2..0f18b32186 100644 --- a/env/env_basic_test.cc +++ b/env/env_basic_test.cc @@ -305,7 +305,7 @@ TEST_P(EnvBasicTestWithParam, LargeWrite) { read += result.size(); } ASSERT_TRUE(write_data == read_data); - delete [] scratch; + delete[] scratch; } TEST_P(EnvMoreTestWithParam, GetModTime) { diff --git a/env/env_chroot.cc b/env/env_chroot.cc index e7cd04031d..a64373517f 100644 --- a/env/env_chroot.cc +++ b/env/env_chroot.cc @@ -66,9 +66,9 @@ IOStatus ChrootFileSystem::GetTestDirectory(const IOOptions& options, return CreateDirIfMissing(*path, options, dbg); } - // Returns status and expanded absolute path including the chroot directory. - // Checks whether the provided path breaks out of the chroot. If it returns - // non-OK status, the returned path should not be used. +// Returns status and expanded absolute path including the chroot directory. +// Checks whether the provided path breaks out of the chroot. If it returns +// non-OK status, the returned path should not be used. std::pair ChrootFileSystem::EncodePath( const std::string& path) { if (path.empty() || path[0] != '/') { @@ -77,29 +77,29 @@ std::pair ChrootFileSystem::EncodePath( std::pair res; res.second = chroot_dir_ + path; #if defined(OS_AIX) - char resolvedName[PATH_MAX]; - char* normalized_path = realpath(res.second.c_str(), resolvedName); + char resolvedName[PATH_MAX]; + char* normalized_path = realpath(res.second.c_str(), resolvedName); #else - char* normalized_path = realpath(res.second.c_str(), nullptr); + char* normalized_path = realpath(res.second.c_str(), nullptr); #endif - if (normalized_path == nullptr) { - res.first = IOStatus::NotFound(res.second, errnoStr(errno).c_str()); - } else if (strlen(normalized_path) < chroot_dir_.size() || - strncmp(normalized_path, chroot_dir_.c_str(), - chroot_dir_.size()) != 0) { - res.first = IOStatus::IOError(res.second, - "Attempted to access path outside chroot"); - } else { - res.first = IOStatus::OK(); - } + if (normalized_path == nullptr) { + res.first = IOStatus::NotFound(res.second, errnoStr(errno).c_str()); + } else if (strlen(normalized_path) < chroot_dir_.size() || + strncmp(normalized_path, chroot_dir_.c_str(), + chroot_dir_.size()) != 0) { + res.first = IOStatus::IOError(res.second, + "Attempted to access path outside chroot"); + } else { + res.first = IOStatus::OK(); + } #if !defined(OS_AIX) - free(normalized_path); + free(normalized_path); #endif - return res; + return res; } - // Similar to EncodePath() except assumes the basename in the path hasn't been - // created yet. +// Similar to EncodePath() except assumes the basename in the path hasn't been +// created yet. std::pair ChrootFileSystem::EncodePathWithNewBasename( const std::string& path) { if (path.empty() || path[0] != '/') { diff --git a/env/env_encryption.cc b/env/env_encryption.cc index 147bd8ea4d..c6b0a257db 100644 --- a/env/env_encryption.cc +++ b/env/env_encryption.cc @@ -33,14 +33,14 @@ std::shared_ptr EncryptionProvider::NewCTRProvider( return std::make_shared(cipher); } - // Read up to "n" bytes from the file. "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. - // - // REQUIRES: External synchronization +// Read up to "n" bytes from the file. "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. +// +// REQUIRES: External synchronization IOStatus EncryptedSequentialFile::Read(size_t n, const IOOptions& options, Slice* result, char* scratch, IODebugContext* dbg) { @@ -89,16 +89,16 @@ size_t EncryptedSequentialFile::GetRequiredBufferAlignment() const { return file_->GetRequiredBufferAlignment(); } - // Remove any kind of caching of data from the offset to offset+length - // of this file. If the length is 0, then it refers to the end of file. - // If the system is not caching the file contents, then this is a noop. +// Remove any kind of caching of data from the offset to offset+length +// of this file. If the length is 0, then it refers to the end of file. +// If the system is not caching the file contents, then this is a noop. IOStatus EncryptedSequentialFile::InvalidateCache(size_t offset, size_t length) { return file_->InvalidateCache(offset + prefixLength_, length); } - // Positioned Read for direct I/O - // If Direct I/O enabled, offset, n, and scratch should be properly aligned +// Positioned Read for direct I/O +// If Direct I/O enabled, offset, n, and scratch should be properly aligned IOStatus EncryptedSequentialFile::PositionedRead(uint64_t offset, size_t n, const IOOptions& options, Slice* result, char* scratch, @@ -118,16 +118,16 @@ IOStatus EncryptedSequentialFile::PositionedRead(uint64_t offset, size_t n, return io_s; } - // 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. - // If Direct I/O enabled, offset, n, and scratch should be aligned properly. +// 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. +// If Direct I/O enabled, offset, n, and scratch should be aligned properly. IOStatus EncryptedRandomAccessFile::Read(uint64_t offset, size_t n, const IOOptions& options, Slice* result, char* scratch, @@ -146,7 +146,7 @@ IOStatus EncryptedRandomAccessFile::Read(uint64_t offset, size_t n, return io_s; } - // Readahead the file starting from offset by n bytes for caching. +// Readahead the file starting from offset by n bytes for caching. IOStatus EncryptedRandomAccessFile::Prefetch(uint64_t offset, size_t n, const IOOptions& options, IODebugContext* dbg) { @@ -154,21 +154,21 @@ IOStatus EncryptedRandomAccessFile::Prefetch(uint64_t offset, size_t n, return file_->Prefetch(offset + prefixLength_, n, options, dbg); } - // Tries to get an unique ID for this file that will be the same each time - // the file is opened (and will stay the same while the file is open). - // Furthermore, it tries to make this ID at most "max_size" bytes. If such an - // ID can be created this function returns the length of the ID and places it - // in "id"; otherwise, this function returns 0, in which case "id" - // may not have been modified. - // - // This function guarantees, for IDs from a given environment, two unique ids - // cannot be made equal to each other by adding arbitrary bytes to one of - // them. That is, no unique ID is the prefix of another. - // - // This function guarantees that the returned ID will not be interpretable as - // a single varint. - // - // Note: these IDs are only valid for the duration of the process. +// Tries to get an unique ID for this file that will be the same each time +// the file is opened (and will stay the same while the file is open). +// Furthermore, it tries to make this ID at most "max_size" bytes. If such an +// ID can be created this function returns the length of the ID and places it +// in "id"; otherwise, this function returns 0, in which case "id" +// may not have been modified. +// +// This function guarantees, for IDs from a given environment, two unique ids +// cannot be made equal to each other by adding arbitrary bytes to one of +// them. That is, no unique ID is the prefix of another. +// +// This function guarantees that the returned ID will not be interpretable as +// a single varint. +// +// Note: these IDs are only valid for the duration of the process. size_t EncryptedRandomAccessFile::GetUniqueId(char* id, size_t max_size) const { return file_->GetUniqueId(id, max_size); }; @@ -177,21 +177,21 @@ void EncryptedRandomAccessFile::Hint(AccessPattern pattern) { file_->Hint(pattern); } - // Indicates the upper layers if the current RandomAccessFile implementation - // uses direct IO. +// Indicates the upper layers if the current RandomAccessFile implementation +// uses direct IO. bool EncryptedRandomAccessFile::use_direct_io() const { return file_->use_direct_io(); } - // Use the returned alignment value to allocate - // aligned buffer for Direct I/O +// Use the returned alignment value to allocate +// aligned buffer for Direct I/O size_t EncryptedRandomAccessFile::GetRequiredBufferAlignment() const { return file_->GetRequiredBufferAlignment(); } - // Remove any kind of caching of data from the offset to offset+length - // of this file. If the length is 0, then it refers to the end of file. - // If the system is not caching the file contents, then this is a noop. +// Remove any kind of caching of data from the offset to offset+length +// of this file. If the length is 0, then it refers to the end of file. +// If the system is not caching the file contents, then this is a noop. IOStatus EncryptedRandomAccessFile::InvalidateCache(size_t offset, size_t length) { return file_->InvalidateCache(offset + prefixLength_, length); @@ -267,8 +267,8 @@ bool EncryptedWritableFile::IsSyncThreadSafe() const { return file_->IsSyncThreadSafe(); } - // Use the returned alignment value to allocate - // aligned buffer for Direct I/O +// Use the returned alignment value to allocate +// aligned buffer for Direct I/O size_t EncryptedWritableFile::GetRequiredBufferAlignment() const { return file_->GetRequiredBufferAlignment(); } @@ -363,14 +363,14 @@ bool EncryptedRandomRWFile::use_direct_io() const { return file_->use_direct_io(); } - // Use the returned alignment value to allocate - // aligned buffer for Direct I/O +// Use the returned alignment value to allocate +// aligned buffer for Direct I/O size_t EncryptedRandomRWFile::GetRequiredBufferAlignment() const { return file_->GetRequiredBufferAlignment(); } - // Write bytes in `data` at offset `offset`, Returns Status::OK() on success. - // Pass aligned buffer when use_direct_io() returns true. +// Write bytes in `data` at offset `offset`, Returns Status::OK() on success. +// Pass aligned buffer when use_direct_io() returns true. IOStatus EncryptedRandomRWFile::Write(uint64_t offset, const Slice& data, const IOOptions& options, IODebugContext* dbg) { @@ -397,9 +397,9 @@ IOStatus EncryptedRandomRWFile::Write(uint64_t offset, const Slice& data, return file_->Write(offset, dataToWrite, options, dbg); } - // Read up to `n` bytes starting from offset `offset` and store them in - // result, provided `scratch` size should be at least `n`. - // Returns Status::OK() on success. +// Read up to `n` bytes starting from offset `offset` and store them in +// result, provided `scratch` size should be at least `n`. +// Returns Status::OK() on success. IOStatus EncryptedRandomRWFile::Read(uint64_t offset, size_t n, const IOOptions& options, Slice* result, char* scratch, IODebugContext* dbg) const { @@ -953,7 +953,8 @@ Env* NewEncryptedEnv(Env* base_env, // Encrypt one or more (partial) blocks of data at the file offset. // Length of data is given in dataSize. -Status BlockAccessCipherStream::Encrypt(uint64_t fileOffset, char *data, size_t dataSize) { +Status BlockAccessCipherStream::Encrypt(uint64_t fileOffset, char* data, + size_t dataSize) { // Calculate block index auto blockSize = BlockSize(); uint64_t blockIndex = fileOffset / blockSize; @@ -965,7 +966,7 @@ Status BlockAccessCipherStream::Encrypt(uint64_t fileOffset, char *data, size_t // Encrypt individual blocks. while (1) { - char *block = data; + char* block = data; size_t n = std::min(dataSize, blockSize - blockOffset); if (n != blockSize) { // We're not encrypting a full block. @@ -998,7 +999,8 @@ Status BlockAccessCipherStream::Encrypt(uint64_t fileOffset, char *data, size_t // Decrypt one or more (partial) blocks of data at the file offset. // Length of data is given in dataSize. -Status BlockAccessCipherStream::Decrypt(uint64_t fileOffset, char *data, size_t dataSize) { +Status BlockAccessCipherStream::Decrypt(uint64_t fileOffset, char* data, + size_t dataSize) { // Calculate block index auto blockSize = BlockSize(); uint64_t blockIndex = fileOffset / blockSize; @@ -1010,7 +1012,7 @@ Status BlockAccessCipherStream::Decrypt(uint64_t fileOffset, char *data, size_t // Decrypt individual blocks. while (1) { - char *block = data; + char* block = data; size_t n = std::min(dataSize, blockSize - blockOffset); if (n != blockSize) { // We're not decrypting a full block. @@ -1344,6 +1346,6 @@ Status EncryptionProvider::CreateFromString( result); } -#endif // ROCKSDB_LITE +#endif // ROCKSDB_LITE } // namespace ROCKSDB_NAMESPACE diff --git a/env/env_encryption_ctr.h b/env/env_encryption_ctr.h index ce2d4b3e31..cfb440c72a 100644 --- a/env/env_encryption_ctr.h +++ b/env/env_encryption_ctr.h @@ -87,8 +87,8 @@ class CTREncryptionProvider : public EncryptionProvider { Status AddCipher(const std::string& descriptor, const char* /*cipher*/, size_t /*len*/, bool /*for_write*/) override; - protected: + protected: // PopulateSecretPrefixPart initializes the data into a new prefix block // that will be encrypted. This function will store the data in plain text. // It will be encrypted later (before written to disk). diff --git a/env/env_test.cc b/env/env_test.cc index 866f3eabe8..f4e9d50b23 100644 --- a/env/env_test.cc +++ b/env/env_test.cc @@ -91,7 +91,8 @@ extern "C" bool RocksDbIOUringEnable() { return true; } std::unique_ptr NewAligned(const size_t size, const char ch) { char* ptr = nullptr; #ifdef OS_WIN - if (nullptr == (ptr = reinterpret_cast(_aligned_malloc(size, kPageSize)))) { + if (nullptr == + (ptr = reinterpret_cast(_aligned_malloc(size, kPageSize)))) { return std::unique_ptr(nullptr, Deleter(_aligned_free)); } std::unique_ptr uptr(ptr, Deleter(_aligned_free)); @@ -183,8 +184,7 @@ TEST_F(EnvPosixTest, AreFilesSame) { std::string same_file_link_name = same_file_name + "_link"; std::unique_ptr same_file; - ASSERT_OK(env->NewWritableFile(same_file_name, - &same_file, soptions)); + ASSERT_OK(env->NewWritableFile(same_file_name, &same_file, soptions)); same_file->Append("random_data"); ASSERT_OK(same_file->Flush()); same_file.reset(); @@ -681,7 +681,7 @@ TEST_P(EnvPosixTestWithParam, TwoPools) { } TEST_P(EnvPosixTestWithParam, DecreaseNumBgThreads) { - constexpr int kWaitMicros = 60000000; // 1min + constexpr int kWaitMicros = 60000000; // 1min std::vector tasks(10); @@ -988,7 +988,6 @@ bool IsUniqueIDValid(const std::string& s) { const size_t MAX_ID_SIZE = 100; char temp_id[MAX_ID_SIZE]; - } // namespace // Determine whether we can use the FS_IOC_GETVERSION ioctl @@ -1030,12 +1029,12 @@ class IoctlFriendlyTmpdir { explicit IoctlFriendlyTmpdir() { char dir_buf[100]; - const char *fmt = "%s/rocksdb.XXXXXX"; - const char *tmp = getenv("TEST_IOCTL_FRIENDLY_TMPDIR"); + const char* fmt = "%s/rocksdb.XXXXXX"; + const char* tmp = getenv("TEST_IOCTL_FRIENDLY_TMPDIR"); #ifdef OS_WIN #define rmdir _rmdir - if(tmp == nullptr) { + if (tmp == nullptr) { tmp = getenv("TMP"); } @@ -1066,8 +1065,10 @@ class IoctlFriendlyTmpdir { // Diagnose ioctl-related failure only if this is the // directory specified via that envvar. if (tmp && tmp == d) { - fprintf(stderr, "TEST_IOCTL_FRIENDLY_TMPDIR-specified directory is " - "not suitable: %s\n", d.c_str()); + fprintf(stderr, + "TEST_IOCTL_FRIENDLY_TMPDIR-specified directory is " + "not suitable: %s\n", + d.c_str()); } rmdir(dir_buf); // ignore failure } @@ -1087,19 +1088,16 @@ class IoctlFriendlyTmpdir { return; } - fprintf(stderr, "failed to find an ioctl-friendly temporary directory;" + fprintf(stderr, + "failed to find an ioctl-friendly temporary directory;" " specify one via the TEST_IOCTL_FRIENDLY_TMPDIR envvar\n"); std::abort(); #endif } - ~IoctlFriendlyTmpdir() { - rmdir(dir_.c_str()); - } + ~IoctlFriendlyTmpdir() { rmdir(dir_.c_str()); } - const std::string& name() const { - return dir_; - } + const std::string& name() const { return dir_; } bool is_supported() const { return is_supported_; } @@ -1273,7 +1271,7 @@ TEST_P(EnvPosixTestWithParam, AllocateTest) { // Returns true if any of the strings in ss are the prefix of another string. bool HasPrefix(const std::unordered_set& ss) { - for (const std::string& s: ss) { + for (const std::string& s : ss) { if (s.empty()) { return true; } @@ -1506,19 +1504,23 @@ TEST_F(EnvPosixTest, MultiReadNonAlignedLargeNum) { for (int i = 0; i < num_reads; i++) { int rnd_off; // No repeat offsets. - while (start_offsets.find(rnd_off = rnd.Uniform(81920)) != start_offsets.end()) {} + while (start_offsets.find(rnd_off = rnd.Uniform(81920)) != + start_offsets.end()) { + } start_offsets.insert(rnd_off); } std::vector offsets; std::vector lens; // std::set already sorted the offsets. - for (int so: start_offsets) { + for (int so : start_offsets) { offsets.push_back(so); } for (size_t i = 0; i + 1 < offsets.size(); i++) { - lens.push_back(static_cast(rnd.Uniform(static_cast(offsets[i + 1] - offsets[i])) + 1)); + lens.push_back(static_cast( + rnd.Uniform(static_cast(offsets[i + 1] - offsets[i])) + 1)); } - lens.push_back(static_cast(rnd.Uniform(static_cast(kTotalSize - offsets.back())) + 1)); + lens.push_back(static_cast( + rnd.Uniform(static_cast(kTotalSize - offsets.back())) + 1)); ASSERT_EQ(num_reads, lens.size()); // Create requests @@ -1540,8 +1542,9 @@ TEST_F(EnvPosixTest, MultiReadNonAlignedLargeNum) { // Validate results for (int i = 0; i < num_reads; ++i) { ASSERT_OK(reqs[i].status); - ASSERT_EQ(Slice(expected_data.data() + offsets[i], lens[i]).ToString(true), - reqs[i].result.ToString(true)); + ASSERT_EQ( + Slice(expected_data.data() + offsets[i], lens[i]).ToString(true), + reqs[i].result.ToString(true)); } ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->DisableProcessing(); @@ -1754,57 +1757,60 @@ TEST_P(EnvPosixTestWithParam, InvalidateCache) { // Create file. { std::unique_ptr wfile; -#if !defined(OS_MACOSX) && !defined(OS_WIN) && !defined(OS_SOLARIS) && !defined(OS_AIX) - if (soptions.use_direct_writes) { - soptions.use_direct_writes = false; - } +#if !defined(OS_MACOSX) && !defined(OS_WIN) && !defined(OS_SOLARIS) && \ + !defined(OS_AIX) + if (soptions.use_direct_writes) { + soptions.use_direct_writes = false; + } #endif - ASSERT_OK(env_->NewWritableFile(fname, &wfile, soptions)); - ASSERT_OK(wfile->Append(slice)); - ASSERT_OK(wfile->InvalidateCache(0, 0)); - ASSERT_OK(wfile->Close()); + ASSERT_OK(env_->NewWritableFile(fname, &wfile, soptions)); + ASSERT_OK(wfile->Append(slice)); + ASSERT_OK(wfile->InvalidateCache(0, 0)); + ASSERT_OK(wfile->Close()); } - // Random Read - { - std::unique_ptr file; - auto scratch = NewAligned(kSectorSize, 0); - Slice result; -#if !defined(OS_MACOSX) && !defined(OS_WIN) && !defined(OS_SOLARIS) && !defined(OS_AIX) - if (soptions.use_direct_reads) { - soptions.use_direct_reads = false; - } -#endif - ASSERT_OK(env_->NewRandomAccessFile(fname, &file, soptions)); - ASSERT_OK(file->Read(0, kSectorSize, &result, scratch.get())); - ASSERT_EQ(memcmp(scratch.get(), data.get(), kSectorSize), 0); - ASSERT_OK(file->InvalidateCache(0, 11)); - ASSERT_OK(file->InvalidateCache(0, 0)); + // Random Read + { + std::unique_ptr file; + auto scratch = NewAligned(kSectorSize, 0); + Slice result; +#if !defined(OS_MACOSX) && !defined(OS_WIN) && !defined(OS_SOLARIS) && \ + !defined(OS_AIX) + if (soptions.use_direct_reads) { + soptions.use_direct_reads = false; } +#endif + ASSERT_OK(env_->NewRandomAccessFile(fname, &file, soptions)); + ASSERT_OK(file->Read(0, kSectorSize, &result, scratch.get())); + ASSERT_EQ(memcmp(scratch.get(), data.get(), kSectorSize), 0); + ASSERT_OK(file->InvalidateCache(0, 11)); + ASSERT_OK(file->InvalidateCache(0, 0)); + } - // Sequential Read - { - std::unique_ptr file; - auto scratch = NewAligned(kSectorSize, 0); - Slice result; -#if !defined(OS_MACOSX) && !defined(OS_WIN) && !defined(OS_SOLARIS) && !defined(OS_AIX) - if (soptions.use_direct_reads) { - soptions.use_direct_reads = false; - } -#endif - ASSERT_OK(env_->NewSequentialFile(fname, &file, soptions)); - if (file->use_direct_io()) { - ASSERT_OK(file->PositionedRead(0, kSectorSize, &result, scratch.get())); - } else { - ASSERT_OK(file->Read(kSectorSize, &result, scratch.get())); - } - ASSERT_EQ(memcmp(scratch.get(), data.get(), kSectorSize), 0); - ASSERT_OK(file->InvalidateCache(0, 11)); - ASSERT_OK(file->InvalidateCache(0, 0)); + // Sequential Read + { + std::unique_ptr file; + auto scratch = NewAligned(kSectorSize, 0); + Slice result; +#if !defined(OS_MACOSX) && !defined(OS_WIN) && !defined(OS_SOLARIS) && \ + !defined(OS_AIX) + if (soptions.use_direct_reads) { + soptions.use_direct_reads = false; } - // Delete the file - ASSERT_OK(env_->DeleteFile(fname)); - ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->ClearTrace(); +#endif + ASSERT_OK(env_->NewSequentialFile(fname, &file, soptions)); + if (file->use_direct_io()) { + ASSERT_OK(file->PositionedRead(0, kSectorSize, &result, scratch.get())); + } else { + ASSERT_OK(file->Read(kSectorSize, &result, scratch.get())); + } + ASSERT_EQ(memcmp(scratch.get(), data.get(), kSectorSize), 0); + ASSERT_OK(file->InvalidateCache(0, 11)); + ASSERT_OK(file->InvalidateCache(0, 0)); + } + // Delete the file + ASSERT_OK(env_->DeleteFile(fname)); + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->ClearTrace(); } #endif // OS_LINUX || OS_WIN @@ -1931,52 +1937,53 @@ TEST_P(EnvPosixTestWithParam, Preallocation) { std::unique_ptr srcfile; EnvOptions soptions; soptions.use_direct_reads = soptions.use_direct_writes = direct_io_; -#if !defined(OS_MACOSX) && !defined(OS_WIN) && !defined(OS_SOLARIS) && !defined(OS_AIX) && !defined(OS_OPENBSD) && !defined(OS_FREEBSD) - if (soptions.use_direct_writes) { - ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack( - "NewWritableFile:O_DIRECT", [&](void* arg) { - int* val = static_cast(arg); - *val &= ~O_DIRECT; - }); - } +#if !defined(OS_MACOSX) && !defined(OS_WIN) && !defined(OS_SOLARIS) && \ + !defined(OS_AIX) && !defined(OS_OPENBSD) && !defined(OS_FREEBSD) + if (soptions.use_direct_writes) { + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack( + "NewWritableFile:O_DIRECT", [&](void* arg) { + int* val = static_cast(arg); + *val &= ~O_DIRECT; + }); + } #endif - ASSERT_OK(env_->NewWritableFile(src, &srcfile, soptions)); - srcfile->SetPreallocationBlockSize(1024 * 1024); + ASSERT_OK(env_->NewWritableFile(src, &srcfile, soptions)); + srcfile->SetPreallocationBlockSize(1024 * 1024); - // No writes should mean no preallocation - size_t block_size, last_allocated_block; + // No writes should mean no preallocation + size_t block_size, last_allocated_block; + srcfile->GetPreallocationStatus(&block_size, &last_allocated_block); + ASSERT_EQ(last_allocated_block, 0UL); + + // Small write should preallocate one block + size_t kStrSize = 4096; + auto data = NewAligned(kStrSize, 'A'); + Slice str(data.get(), kStrSize); + srcfile->PrepareWrite(srcfile->GetFileSize(), kStrSize); + ASSERT_OK(srcfile->Append(str)); + srcfile->GetPreallocationStatus(&block_size, &last_allocated_block); + ASSERT_EQ(last_allocated_block, 1UL); + + // Write an entire preallocation block, make sure we increased by two. + { + auto buf_ptr = NewAligned(block_size, ' '); + Slice buf(buf_ptr.get(), block_size); + srcfile->PrepareWrite(srcfile->GetFileSize(), block_size); + ASSERT_OK(srcfile->Append(buf)); srcfile->GetPreallocationStatus(&block_size, &last_allocated_block); - ASSERT_EQ(last_allocated_block, 0UL); + ASSERT_EQ(last_allocated_block, 2UL); + } - // Small write should preallocate one block - size_t kStrSize = 4096; - auto data = NewAligned(kStrSize, 'A'); - Slice str(data.get(), kStrSize); - srcfile->PrepareWrite(srcfile->GetFileSize(), kStrSize); - ASSERT_OK(srcfile->Append(str)); + // Write five more blocks at once, ensure we're where we need to be. + { + auto buf_ptr = NewAligned(block_size * 5, ' '); + Slice buf = Slice(buf_ptr.get(), block_size * 5); + srcfile->PrepareWrite(srcfile->GetFileSize(), buf.size()); + ASSERT_OK(srcfile->Append(buf)); srcfile->GetPreallocationStatus(&block_size, &last_allocated_block); - ASSERT_EQ(last_allocated_block, 1UL); - - // Write an entire preallocation block, make sure we increased by two. - { - auto buf_ptr = NewAligned(block_size, ' '); - Slice buf(buf_ptr.get(), block_size); - srcfile->PrepareWrite(srcfile->GetFileSize(), block_size); - ASSERT_OK(srcfile->Append(buf)); - srcfile->GetPreallocationStatus(&block_size, &last_allocated_block); - ASSERT_EQ(last_allocated_block, 2UL); - } - - // Write five more blocks at once, ensure we're where we need to be. - { - auto buf_ptr = NewAligned(block_size * 5, ' '); - Slice buf = Slice(buf_ptr.get(), block_size * 5); - srcfile->PrepareWrite(srcfile->GetFileSize(), buf.size()); - ASSERT_OK(srcfile->Append(buf)); - srcfile->GetPreallocationStatus(&block_size, &last_allocated_block); - ASSERT_EQ(last_allocated_block, 7UL); - } - ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->ClearTrace(); + ASSERT_EQ(last_allocated_block, 7UL); + } + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->ClearTrace(); } // Test that the two ways to get children file attributes (in bulk or @@ -1993,53 +2000,50 @@ TEST_P(EnvPosixTestWithParam, ConsistentChildrenAttributes) { for (int i = 0; i < kNumChildren; ++i) { const std::string path = test_base_dir + "/testfile_" + std::to_string(i); std::unique_ptr file; -#if !defined(OS_MACOSX) && !defined(OS_WIN) && !defined(OS_SOLARIS) && !defined(OS_AIX) && !defined(OS_OPENBSD) && !defined(OS_FREEBSD) - if (soptions.use_direct_writes) { - ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack( - "NewWritableFile:O_DIRECT", [&](void* arg) { - int* val = static_cast(arg); - *val &= ~O_DIRECT; - }); - } +#if !defined(OS_MACOSX) && !defined(OS_WIN) && !defined(OS_SOLARIS) && \ + !defined(OS_AIX) && !defined(OS_OPENBSD) && !defined(OS_FREEBSD) + if (soptions.use_direct_writes) { + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack( + "NewWritableFile:O_DIRECT", [&](void* arg) { + int* val = static_cast(arg); + *val &= ~O_DIRECT; + }); + } #endif - ASSERT_OK(env_->NewWritableFile(path, &file, soptions)); - auto buf_ptr = NewAligned(data.size(), 'T'); - Slice buf(buf_ptr.get(), data.size()); - ASSERT_OK(file->Append(buf)); - data.append(std::string(4096, 'T')); + ASSERT_OK(env_->NewWritableFile(path, &file, soptions)); + auto buf_ptr = NewAligned(data.size(), 'T'); + Slice buf(buf_ptr.get(), data.size()); + ASSERT_OK(file->Append(buf)); + data.append(std::string(4096, 'T')); } - std::vector file_attrs; - ASSERT_OK(env_->GetChildrenFileAttributes(test_base_dir, &file_attrs)); - for (int i = 0; i < kNumChildren; ++i) { - const std::string name = "testfile_" + std::to_string(i); - const std::string path = test_base_dir + "/" + name; + std::vector file_attrs; + ASSERT_OK(env_->GetChildrenFileAttributes(test_base_dir, &file_attrs)); + for (int i = 0; i < kNumChildren; ++i) { + const std::string name = "testfile_" + std::to_string(i); + const std::string path = test_base_dir + "/" + name; - auto file_attrs_iter = std::find_if( - file_attrs.begin(), file_attrs.end(), - [&name](const Env::FileAttributes& fm) { return fm.name == name; }); - ASSERT_TRUE(file_attrs_iter != file_attrs.end()); - uint64_t size; - ASSERT_OK(env_->GetFileSize(path, &size)); - ASSERT_EQ(size, 4096 * i); - ASSERT_EQ(size, file_attrs_iter->size_bytes); - } - ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->ClearTrace(); + auto file_attrs_iter = std::find_if( + file_attrs.begin(), file_attrs.end(), + [&name](const Env::FileAttributes& fm) { return fm.name == name; }); + ASSERT_TRUE(file_attrs_iter != file_attrs.end()); + uint64_t size; + ASSERT_OK(env_->GetFileSize(path, &size)); + ASSERT_EQ(size, 4096 * i); + ASSERT_EQ(size, file_attrs_iter->size_bytes); + } + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->ClearTrace(); } // Test that all WritableFileWrapper forwards all calls to WritableFile. TEST_P(EnvPosixTestWithParam, WritableFileWrapper) { class Base : public WritableFile { public: - mutable int *step_; + mutable int* step_; - void inc(int x) const { - EXPECT_EQ(x, (*step_)++); - } + void inc(int x) const { EXPECT_EQ(x, (*step_)++); } - explicit Base(int* step) : step_(step) { - inc(0); - } + explicit Base(int* step) : step_(step) { inc(0); } Status Append(const Slice& /*data*/) override { inc(1); @@ -2372,32 +2376,31 @@ TEST_P(EnvPosixTestWithParam, PosixRandomRWFileRandomized) { } class TestEnv : public EnvWrapper { - public: - explicit TestEnv() : EnvWrapper(Env::Default()), - close_count(0) { } - const char* Name() const override { return "TestEnv"; } - class TestLogger : public Logger { - public: - using Logger::Logv; - explicit TestLogger(TestEnv* env_ptr) : Logger() { env = env_ptr; } - ~TestLogger() override { - if (!closed_) { - Status s = CloseHelper(); - s.PermitUncheckedError(); - } + public: + explicit TestEnv() : EnvWrapper(Env::Default()), close_count(0) {} + const char* Name() const override { return "TestEnv"; } + class TestLogger : public Logger { + public: + using Logger::Logv; + explicit TestLogger(TestEnv* env_ptr) : Logger() { env = env_ptr; } + ~TestLogger() override { + if (!closed_) { + Status s = CloseHelper(); + s.PermitUncheckedError(); } - void Logv(const char* /*format*/, va_list /*ap*/) override {} + } + void Logv(const char* /*format*/, va_list /*ap*/) override {} - protected: - Status CloseImpl() override { return CloseHelper(); } + protected: + Status CloseImpl() override { return CloseHelper(); } - private: - Status CloseHelper() { - env->CloseCountInc(); - return Status::OK(); - } - TestEnv* env; - }; + private: + Status CloseHelper() { + env->CloseCountInc(); + return Status::OK(); + } + TestEnv* env; + }; void CloseCountInc() { close_count++; } @@ -2504,7 +2507,8 @@ class EnvFSTestWithParam env_ptr_ = NewCompositeEnv(fs_); } if (env_non_null && !env_default && fs_default) { - env_ptr_ = std::unique_ptr(new FaultInjectionTestEnv(Env::Default())); + env_ptr_ = + std::unique_ptr(new FaultInjectionTestEnv(Env::Default())); fs_.reset(); } if (env_non_null && !env_default && !fs_default) { @@ -2572,17 +2576,16 @@ TEST_P(EnvFSTestWithParam, OptionsTest) { // 1. True means Options::env is non-null, false means null // 2. True means use Env::Default, false means custom // 3. True means use FileSystem::Default, false means custom -INSTANTIATE_TEST_CASE_P( - EnvFSTest, EnvFSTestWithParam, - ::testing::Combine(::testing::Bool(), ::testing::Bool(), - ::testing::Bool())); +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) { std::shared_ptr fs1 = - std::make_shared(FileSystem::Default()); + std::make_shared(FileSystem::Default()); std::shared_ptr fs2 = - std::make_shared(FileSystem::Default()); + std::make_shared(FileSystem::Default()); std::unique_ptr env1 = NewCompositeEnv(fs1); std::unique_ptr env2 = NewCompositeEnv(fs2); Env::Default()->SetBackgroundThreads(8, Env::HIGH); diff --git a/env/file_system.cc b/env/file_system.cc index ab5b3c450d..f9dda429a3 100644 --- a/env/file_system.cc +++ b/env/file_system.cc @@ -136,7 +136,7 @@ IOStatus FileSystem::NewLogger(const std::string& fname, } FileOptions FileSystem::OptimizeForLogRead( - const FileOptions& file_options) const { + const FileOptions& file_options) const { FileOptions optimized_file_options(file_options); optimized_file_options.use_direct_reads = false; return optimized_file_options; @@ -150,7 +150,7 @@ FileOptions FileSystem::OptimizeForManifestRead( } FileOptions FileSystem::OptimizeForLogWrite(const FileOptions& file_options, - const DBOptions& db_options) const { + const DBOptions& db_options) const { FileOptions optimized_file_options(file_options); optimized_file_options.bytes_per_sync = db_options.wal_bytes_per_sync; optimized_file_options.writable_file_max_buffer_size = @@ -220,8 +220,7 @@ IOStatus ReadFileToString(FileSystem* fs, const std::string& fname, char* space = new char[kBufferSize]; while (true) { Slice fragment; - s = file->Read(kBufferSize, IOOptions(), &fragment, space, - nullptr); + s = file->Read(kBufferSize, IOOptions(), &fragment, space, nullptr); if (!s.ok()) { break; } diff --git a/env/io_posix.cc b/env/io_posix.cc index 59a94acd46..0ec0e9c83b 100644 --- a/env/io_posix.cc +++ b/env/io_posix.cc @@ -9,8 +9,10 @@ #ifdef ROCKSDB_LIB_IO_POSIX #include "env/io_posix.h" + #include #include + #include #if defined(OS_LINUX) #include @@ -601,8 +603,7 @@ IOStatus PosixRandomAccessFile::Read(uint64_t offset, size_t n, return s; } -IOStatus PosixRandomAccessFile::MultiRead(FSReadRequest* reqs, - size_t num_reqs, +IOStatus PosixRandomAccessFile::MultiRead(FSReadRequest* reqs, size_t num_reqs, const IOOptions& options, IODebugContext* dbg) { if (use_direct_io()) { diff --git a/env/io_posix.h b/env/io_posix.h index d766427f8f..7307b20310 100644 --- a/env/io_posix.h +++ b/env/io_posix.h @@ -284,8 +284,7 @@ class PosixRandomAccessFile : public FSRandomAccessFile { public: PosixRandomAccessFile(const std::string& fname, int fd, - size_t logical_block_size, - const EnvOptions& options + size_t logical_block_size, const EnvOptions& options #if defined(ROCKSDB_IOURING_PRESENT) , ThreadLocalPtr* thread_local_io_urings diff --git a/env/mock_env.h b/env/mock_env.h index a8d5283c5f..406a31f635 100644 --- a/env/mock_env.h +++ b/env/mock_env.h @@ -135,6 +135,7 @@ class MockEnv : public CompositeEnvWrapper { const char* Name() const override { return kClassName(); } Status CorruptBuffer(const std::string& fname); + private: MockEnv(Env* env, const std::shared_ptr& fs, const std::shared_ptr& clock); diff --git a/env/mock_env_test.cc b/env/mock_env_test.cc index bcd8ed530a..be174bd73d 100644 --- a/env/mock_env_test.cc +++ b/env/mock_env_test.cc @@ -51,14 +51,14 @@ TEST_F(MockEnvTest, Corrupt) { ASSERT_OK(writable_file->Append(kCorrupted)); ASSERT_TRUE(writable_file->GetFileSize() == kGood.size() + kCorrupted.size()); result.clear(); - ASSERT_OK(rand_file->Read(kGood.size(), kCorrupted.size(), - &result, &(scratch[0]))); + ASSERT_OK( + rand_file->Read(kGood.size(), kCorrupted.size(), &result, &(scratch[0]))); ASSERT_EQ(result.compare(kCorrupted), 0); // Corrupted ASSERT_OK(dynamic_cast(env_)->CorruptBuffer(kFileName)); result.clear(); - ASSERT_OK(rand_file->Read(kGood.size(), kCorrupted.size(), - &result, &(scratch[0]))); + ASSERT_OK( + rand_file->Read(kGood.size(), kCorrupted.size(), &result, &(scratch[0]))); ASSERT_NE(result.compare(kCorrupted), 0); }