diff --git a/env/fs_posix.cc b/env/fs_posix.cc index 34efe1204f6d..03ea7fe40a9d 100644 --- a/env/fs_posix.cc +++ b/env/fs_posix.cc @@ -167,7 +167,6 @@ class PosixFileSystem : public FileSystem { result->reset(); int fd = -1; int flags = cloexec_flags(O_RDONLY, &options); - FILE* file = nullptr; if (options.use_direct_reads && !options.use_mmap_reads) { #if !defined(OS_MACOSX) && !defined(OS_OPENBSD) && !defined(OS_SOLARIS) @@ -187,26 +186,16 @@ class PosixFileSystem : public FileSystem { SetFD_CLOEXEC(fd, &options); - if (options.use_direct_reads && !options.use_mmap_reads) { #ifdef OS_MACOSX + if (options.use_direct_reads && !options.use_mmap_reads) { if (fcntl(fd, F_NOCACHE, 1) == -1) { close(fd); return IOError("While fcntl NoCache", fname, errno); } -#endif - } else { - do { - IOSTATS_TIMER_GUARD(open_nanos); - file = fdopen(fd, "r"); - } while (file == nullptr && errno == EINTR); - if (file == nullptr) { - close(fd); - return IOError("While opening file for sequentially read", fname, - errno); - } } +#endif result->reset(new PosixSequentialFile( - fname, file, fd, GetLogicalBlockSizeForReadIfNeeded(options, fname, fd), + fname, fd, GetLogicalBlockSizeForReadIfNeeded(options, fname, fd), options)); return IOStatus::OK(); } diff --git a/env/io_posix.cc b/env/io_posix.cc index 489e5b3a9e50..6b0dc41699e5 100644 --- a/env/io_posix.cc +++ b/env/io_posix.cc @@ -205,11 +205,10 @@ bool IsSyncFileRangeSupported(int fd) { /* * PosixSequentialFile */ -PosixSequentialFile::PosixSequentialFile(const std::string& fname, FILE* file, +PosixSequentialFile::PosixSequentialFile(const std::string& fname, int fd, size_t logical_block_size, const EnvOptions& options) : filename_(fname), - file_(file), fd_(fd), use_direct_io_(options.use_direct_reads), logical_sector_size_(logical_block_size) { @@ -217,13 +216,8 @@ PosixSequentialFile::PosixSequentialFile(const std::string& fname, FILE* file, } PosixSequentialFile::~PosixSequentialFile() { - if (!use_direct_io()) { - assert(file_); - fclose(file_); - } else { - assert(fd_); - close(fd_); - } + assert(fd_ >= 0); + close(fd_); } IOStatus PosixSequentialFile::Read(size_t n, const IOOptions& /*opts*/, @@ -232,22 +226,27 @@ IOStatus PosixSequentialFile::Read(size_t n, const IOOptions& /*opts*/, assert(result != nullptr && !use_direct_io()); IOStatus s; size_t r = 0; - do { - clearerr(file_); - r = fread_unlocked(scratch, 1, n, file_); - } while (r == 0 && ferror(file_) && errno == EINTR); - *result = Slice(scratch, r); - if (r < n) { - if (feof(file_)) { - // We leave status as ok if we hit the end of the file - // We also clear the error so that the reads can continue - // if a new data is written to the file - clearerr(file_); - } else { - // A partial read with an error: return a non-ok status + while (r < n) { + const ssize_t q = read(fd_, &scratch[r], n - r); + + // Retry read if interrupted + if (q == -1 && errno == EINTR) continue; + + // Advance write count and read again + if (q > 0) { + r += q; + continue; + } + + // If error on read, return a non-ok status + if (q < 0) { s = IOError("While reading file sequentially", filename_, errno); } + + // Treat end of file as ok; exit loop regardless + break; } + *result = Slice(scratch, r); return s; } @@ -292,7 +291,7 @@ IOStatus PosixSequentialFile::PositionedRead(uint64_t offset, size_t n, } IOStatus PosixSequentialFile::Skip(uint64_t n) { - if (fseek(file_, static_cast(n), SEEK_CUR)) { + if (lseek(fd_, static_cast(n), SEEK_CUR) == static_cast(-1)) { return IOError("While fseek to skip " + std::to_string(n) + " bytes", filename_, errno); } diff --git a/env/io_posix.h b/env/io_posix.h index ca33b8e3e948..c09132dcb5d3 100644 --- a/env/io_posix.h +++ b/env/io_posix.h @@ -275,13 +275,12 @@ class LogicalBlockSizeCache { class PosixSequentialFile : public FSSequentialFile { private: std::string filename_; - FILE* file_; int fd_; bool use_direct_io_; size_t logical_sector_size_; public: - PosixSequentialFile(const std::string& fname, FILE* file, int fd, + PosixSequentialFile(const std::string& fname, int fd, size_t logical_block_size, const EnvOptions& options); virtual ~PosixSequentialFile(); diff --git a/tools/db_bench_tool.cc b/tools/db_bench_tool.cc index 16033434f564..b99606426af9 100644 --- a/tools/db_bench_tool.cc +++ b/tools/db_bench_tool.cc @@ -2267,7 +2267,7 @@ static std::unordered_map> OperationTypeString = {{kRead, "read"}, {kWrite, "write"}, {kDelete, "delete"}, {kSeek, "seek"}, {kMerge, "merge"}, {kUpdate, "update"}, - {kCompress, "compress"}, {kCompress, "uncompress"}, + {kCompress, "compress"}, {kUncompress, "uncompress"}, {kCrc, "crc"}, {kHash, "hash"}, {kOthers, "op"}, {kMultiScan, "multiscan"}}; diff --git a/util/compression.cc b/util/compression.cc index 30b7e8b09e1d..57b259b28aae 100644 --- a/util/compression.cc +++ b/util/compression.cc @@ -1404,6 +1404,7 @@ class BuiltinDecompressorV2SnappyOnly final : public BuiltinDecompressorV2 { args.uncompressed_size = uncompressed_length; return Status::OK(); #else + (void)args; return Status::NotSupported("Snappy not supported in this build"); #endif } diff --git a/utilities/write_batch_with_index/write_batch_with_index_test.cc b/utilities/write_batch_with_index/write_batch_with_index_test.cc index a61de9129f23..6ea1951de237 100644 --- a/utilities/write_batch_with_index/write_batch_with_index_test.cc +++ b/utilities/write_batch_with_index/write_batch_with_index_test.cc @@ -3663,7 +3663,7 @@ TEST_P(WriteBatchWithIndexTest, TrackAndClearCFStats) { { auto& cf_id_to_count = batch_->GetCFStats(); ASSERT_EQ(2, cf_id_to_count.size()); - for (const auto [cf_id, stat] : cf_id_to_count) { + for (const auto& [cf_id, stat] : cf_id_to_count) { if (cf_id == 0) { ASSERT_EQ(2, stat.entry_count); ASSERT_EQ(0, stat.overwritten_sd_count);