Get block size only in direct IO mode (#6522)

Summary:
When `use_direct_reads` and `use_direct_writes` are `false`, `logical_sector_size_` inside various `*File` implementations are not actually used, so `GetLogicalBlockSize` does not necessarily need to be called for `logical_sector_size_`, just set a default page size.

This is a follow up PR for https://github.com/facebook/rocksdb/pull/6457.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6522

Test Plan: make check

Reviewed By: siying

Differential Revision: D20408885

Pulled By: cheng-chang

fbshipit-source-id: f2d3808f41265237e7fa2c0be9f084f8fa97fe3d

db/db_logical_block_size_cache_test.cc

@@ -37,17 +37,15 @@ class DBLogicalBlockSizeCacheTest : public testing::Test {
         data_path_1_(dbname_ + "/data_path_1"),
         cf_path_0_(dbname_ + "/cf_path_0"),
         cf_path_1_(dbname_ + "/cf_path_1") {
-    auto get_fd_block_size = [&](int fd) {
+    auto get_fd_block_size = [&](int fd) { return fd; };
-      return fd;
-    };
     auto get_dir_block_size = [&](const std::string& /*dir*/, size_t* size) {
       *size = 1024;
       return Status::OK();
     };
-    cache_.reset(new LogicalBlockSizeCache(
+    cache_.reset(
-        get_fd_block_size, get_dir_block_size));
+        new LogicalBlockSizeCache(get_fd_block_size, get_dir_block_size));
-    env_.reset(new EnvWithCustomLogicalBlockSizeCache(
+    env_.reset(
-        Env::Default(), cache_.get()));
+        new EnvWithCustomLogicalBlockSizeCache(Env::Default(), cache_.get()));
   }
 
  protected:
@@ -507,7 +505,7 @@ TEST_F(DBLogicalBlockSizeCacheTest, MultiDBWithSamePaths) {
 }
 
 }  // namespace ROCKSDB_NAMESPACE
-#endif // OS_LINUX
+#endif  // OS_LINUX
 
 int main(int argc, char** argv) {
   ::testing::InitGoogleTest(&argc, argv);

env/fs_posix.cc

@@ -184,7 +184,8 @@ class PosixFileSystem : public FileSystem {
       }
     }
     result->reset(new PosixSequentialFile(
-        fname, file, fd, GetLogicalBlockSize(fname, fd), options));
+        fname, file, fd, GetLogicalBlockSizeForReadIfNeeded(options, fname, fd),
+        options));
     return IOStatus::OK();
   }
 
@@ -244,7 +245,8 @@ class PosixFileSystem : public FileSystem {
 #endif
       }
       result->reset(new PosixRandomAccessFile(
-          fname, fd, GetLogicalBlockSize(fname, fd), options
+          fname, fd, GetLogicalBlockSizeForReadIfNeeded(options, fname, fd),
+          options
 #if defined(ROCKSDB_IOURING_PRESENT)
           ,
           thread_local_io_urings_.get()
@@ -331,13 +333,17 @@ class PosixFileSystem : public FileSystem {
       }
 #endif
       result->reset(new PosixWritableFile(
-          fname, fd, GetLogicalBlockSize(fname, fd), options));
+          fname, fd, GetLogicalBlockSizeForWriteIfNeeded(options, fname, fd),
+          options));
     } else {
       // disable mmap writes
       EnvOptions no_mmap_writes_options = options;
       no_mmap_writes_options.use_mmap_writes = false;
-      result->reset(new PosixWritableFile(
+      result->reset(
-          fname, fd, GetLogicalBlockSize(fname, fd), no_mmap_writes_options));
+          new PosixWritableFile(fname, fd,
+                                GetLogicalBlockSizeForWriteIfNeeded(
+                                    no_mmap_writes_options, fname, fd),
+                                no_mmap_writes_options));
     }
     return s;
   }
@@ -433,13 +439,17 @@ class PosixFileSystem : public FileSystem {
       }
 #endif
       result->reset(new PosixWritableFile(
-          fname, fd, GetLogicalBlockSize(fname, fd), options));
+          fname, fd, GetLogicalBlockSizeForWriteIfNeeded(options, fname, fd),
+          options));
     } else {
       // disable mmap writes
       FileOptions no_mmap_writes_options = options;
       no_mmap_writes_options.use_mmap_writes = false;
-      result->reset(new PosixWritableFile(
+      result->reset(
-          fname, fd, GetLogicalBlockSize(fname, fd), no_mmap_writes_options));
+          new PosixWritableFile(fname, fd,
+                                GetLogicalBlockSizeForWriteIfNeeded(
+                                    no_mmap_writes_options, fname, fd),
+                                no_mmap_writes_options));
     }
     return s;
   }
@@ -918,6 +928,14 @@ class PosixFileSystem : public FileSystem {
   static LogicalBlockSizeCache logical_block_size_cache_;
 #endif
   static size_t GetLogicalBlockSize(const std::string& fname, int fd);
+  // In non-direct IO mode, this directly returns kDefaultPageSize.
+  // Otherwise call GetLogicalBlockSize.
+  static size_t GetLogicalBlockSizeForReadIfNeeded(const EnvOptions& options,
+                                                   const std::string& fname,
+                                                   int fd);
+  static size_t GetLogicalBlockSizeForWriteIfNeeded(const EnvOptions& options,
+                                                    const std::string& fname,
+                                                    int fd);
 };
 
 #ifdef OS_LINUX
@@ -928,11 +946,25 @@ size_t PosixFileSystem::GetLogicalBlockSize(const std::string& fname, int fd) {
 #ifdef OS_LINUX
   return logical_block_size_cache_.GetLogicalBlockSize(fname, fd);
 #else
-  (void) fname;
+  (void)fname;
   return PosixHelper::GetLogicalBlockSizeOfFd(fd);
 #endif
 }
 
+size_t PosixFileSystem::GetLogicalBlockSizeForReadIfNeeded(
+    const EnvOptions& options, const std::string& fname, int fd) {
+  return options.use_direct_reads
+             ? PosixFileSystem::GetLogicalBlockSize(fname, fd)
+             : kDefaultPageSize;
+}
+
+size_t PosixFileSystem::GetLogicalBlockSizeForWriteIfNeeded(
+    const EnvOptions& options, const std::string& fname, int fd) {
+  return options.use_direct_writes
+             ? PosixFileSystem::GetLogicalBlockSize(fname, fd)
+             : kDefaultPageSize;
+}
+
 PosixFileSystem::PosixFileSystem()
     : checkedDiskForMmap_(false),
       forceMmapOff_(false),