Implement WinRandomRW file and improve code reuse (#1388)

include/rocksdb/env.h

@@ -504,15 +504,16 @@ class WritableFile {
   virtual ~WritableFile();
 
   // Indicates if the class makes use of unbuffered I/O
+  // If false you must pass aligned buffer to Write()
   virtual bool UseOSBuffer() const {
     return true;
   }
 
   const size_t c_DefaultPageSize = 4 * 1024;
 
-  // This is needed when you want to allocate
-  // AlignedBuffer for use with file I/O classes
-  // Used for unbuffered file I/O when UseOSBuffer() returns false
+  // Use the returned alignment value to allocate
+  // aligned buffer for Write() when UseOSBuffer()
+  // returns false
   virtual size_t GetRequiredBufferAlignment() const {
     return c_DefaultPageSize;
   }
@@ -664,7 +665,34 @@ class RandomRWFile {
   RandomRWFile() {}
   virtual ~RandomRWFile() {}
 
+  // Indicates if the class makes use of unbuffered I/O
+  // If false you must pass aligned buffer to Write()
+  virtual bool UseOSBuffer() const {
+    return true;
+  }
+
+  const size_t c_DefaultPageSize = 4 * 1024;
+
+  // Use the returned alignment value to allocate
+  // aligned buffer for Write() when UseOSBuffer()
+  // returns false
+  virtual size_t GetRequiredBufferAlignment() const {
+    return c_DefaultPageSize;
+  }
+
+  // Used by the file_reader_writer to decide if the ReadAhead wrapper
+  // should simply forward the call and do not enact read_ahead buffering or locking.
+  // The implementation below takes care of reading ahead
+  virtual bool ShouldForwardRawRequest() const {
+    return false;
+  }
+
+  // For cases when read-ahead is implemented in the platform dependent
+  // layer. This is when ShouldForwardRawRequest() returns true.
+  virtual void EnableReadAhead() {}
+
   // Write bytes in `data` at  offset `offset`, Returns Status::OK() on success.
+  // Pass aligned buffer when UseOSBuffer() returns false.
   virtual Status Write(uint64_t offset, const Slice& data) = 0;
 
   // Read up to `n` bytes starting from offset `offset` and store them in
@@ -681,7 +709,6 @@ class RandomRWFile {
 
   virtual Status Close() = 0;
 
- private:
   // No copying allowed
   RandomRWFile(const RandomRWFile&) = delete;
   RandomRWFile& operator=(const RandomRWFile&) = delete;

port/win/env_win.cc

@@ -293,6 +293,50 @@ Status WinEnvIO::NewWritableFile(const std::string& fname,
   return s;
 }
 
+Status WinEnvIO::NewRandomRWFile(const std::string & fname,
+  unique_ptr<RandomRWFile>* result, const EnvOptions & options) {
+
+  Status s;
+
+  // Open the file for read-only random access
+  // Random access is to disable read-ahead as the system reads too much data
+  DWORD desired_access = GENERIC_READ | GENERIC_WRITE;
+  DWORD shared_mode = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE;
+  DWORD creation_disposition = OPEN_ALWAYS; // Create if necessary or open existing
+  DWORD file_flags = FILE_FLAG_RANDOM_ACCESS;
+
+  if (!options.use_os_buffer) {
+    file_flags |= FILE_FLAG_NO_BUFFERING;
+  }
+
+  /// Shared access is necessary for corruption test to pass
+  // almost all tests would work with a possible exception of fault_injection
+  HANDLE hFile = 0;
+  {
+    IOSTATS_TIMER_GUARD(open_nanos);
+    hFile =
+      CreateFileA(fname.c_str(),
+        desired_access,
+        shared_mode,
+        NULL, // Security attributes
+        creation_disposition,
+        file_flags,
+        NULL);
+  }
+
+  if (INVALID_HANDLE_VALUE == hFile) {
+    auto lastError = GetLastError();
+    return IOErrorFromWindowsError(
+      "NewRandomRWFile failed to Create/Open: " + fname, lastError);
+  }
+
+  UniqueCloseHandlePtr fileGuard(hFile, CloseHandleFunc);
+  result->reset(new WinRandomRWFile(fname, hFile, page_size_, options));
+  fileGuard.release();
+
+  return s;
+}
+
 Status WinEnvIO::NewDirectory(const std::string& name,
   std::unique_ptr<Directory>* result) {
   Status s;
@@ -868,6 +912,11 @@ Status WinEnv::NewWritableFile(const std::string& fname,
   return winenv_io_.NewWritableFile(fname, result, options);
 }
 
+Status WinEnv::NewRandomRWFile(const std::string & fname,
+  unique_ptr<RandomRWFile>* result, const EnvOptions & options) {
+  return winenv_io_.NewRandomRWFile(fname, result, options);
+}
+
 Status WinEnv::NewDirectory(const std::string& name,
   std::unique_ptr<Directory>* result) {
   return winenv_io_.NewDirectory(name, result);

port/win/env_win.h

@@ -92,6 +92,11 @@ public:
     std::unique_ptr<WritableFile>* result,
     const EnvOptions& options);
 
+  // The returned file will only be accessed by one thread at a time.
+  virtual Status NewRandomRWFile(const std::string& fname,
+    unique_ptr<RandomRWFile>* result,
+    const EnvOptions& options);
+
   virtual Status NewDirectory(const std::string& name,
     std::unique_ptr<Directory>* result);
 
@@ -188,6 +193,11 @@ public:
     std::unique_ptr<WritableFile>* result,
     const EnvOptions& options) override;
 
+  // The returned file will only be accessed by one thread at a time.
+  Status NewRandomRWFile(const std::string& fname,
+    unique_ptr<RandomRWFile>* result,
+    const EnvOptions& options) override;
+
   Status NewDirectory(const std::string& name,
     std::unique_ptr<Directory>* result) override;
 

port/win/io_win.h

@@ -68,10 +68,58 @@ Status ftruncate(const std::string& filename, HANDLE hFile,
 
 size_t GetUniqueIdFromFile(HANDLE hFile, char* id, size_t max_size);
 
-// mmap() based random-access
-class WinMmapReadableFile : public RandomAccessFile {
-  const std::string fileName_;
+class WinFileData {
+protected:
+
+  const std::string filename_;
   HANDLE hFile_;
+  // There is no equivalent of advising away buffered pages as in posix.
+  // To implement this flag we would need to do unbuffered reads which
+  // will need to be aligned (not sure there is a guarantee that the buffer
+  // passed in is aligned).
+  // Hence we currently ignore this flag. It is used only in a few cases
+  // which should not be perf critical.
+  // If perf evaluation finds this to be a problem, we can look into
+  // implementing this.
+  const bool use_os_buffer_;
+
+public:
+
+  // We want this class be usable both for inheritance (prive
+  // or protected) and for containment so __ctor and __dtor public
+  WinFileData(const std::string& filename, HANDLE hFile, bool use_os_buffer) :
+    filename_(filename), hFile_(hFile), use_os_buffer_(use_os_buffer)
+  {}
+
+  virtual ~WinFileData() {
+    this->CloseFile();
+  }
+
+  bool CloseFile() {
+
+    bool result = true;
+
+    if (hFile_ != NULL && hFile_ != INVALID_HANDLE_VALUE) {
+      result = ::CloseHandle(hFile_);
+      assert(result);
+      hFile_ = NULL;
+    }
+    return result;
+  }
+
+  const std::string& GetName() const { return filename_; }
+
+  HANDLE GetFileHandle() const { return hFile_; }
+
+  bool UseOSBuffer() const { return use_os_buffer_; }
+
+  WinFileData(const WinFileData&) = delete;
+  WinFileData& operator=(const WinFileData&) = delete;
+};
+
+
+// mmap() based random-access
+class WinMmapReadableFile : private WinFileData, public RandomAccessFile {
   HANDLE hMap_;
 
   const void* mapped_region_;
@@ -84,6 +132,9 @@ public:
 
   ~WinMmapReadableFile();
 
+  WinMmapReadableFile(const WinMmapReadableFile&) = delete;
+  WinMmapReadableFile& operator=(const WinMmapReadableFile&) = delete;
+
   virtual Status Read(uint64_t offset, size_t n, Slice* result,
     char* scratch) const override;
 
@@ -96,10 +147,8 @@ public:
 // data to the file.  This is safe since we either properly close the
 // file before reading from it, or for log files, the reading code
 // knows enough to skip zero suffixes.
-class WinMmapFile : public WritableFile {
+class WinMmapFile : private WinFileData, public WritableFile {
 private:
-  const std::string filename_;
-  HANDLE hFile_;
   HANDLE hMap_;
 
   const size_t page_size_;  // We flush the mapping view in page_size
@@ -142,6 +191,9 @@ public:
 
   ~WinMmapFile();
 
+  WinMmapFile(const WinMmapFile&) = delete;
+  WinMmapFile& operator=(const WinMmapFile&) = delete;
+
   virtual Status Append(const Slice& data) override;
 
   // Means Close() will properly take care of truncate
@@ -174,27 +226,16 @@ public:
   virtual size_t GetUniqueId(char* id, size_t max_size) const override;
 };
 
-class WinSequentialFile : public SequentialFile {
-private:
-  const std::string filename_;
-  HANDLE file_;
-
-  // There is no equivalent of advising away buffered pages as in posix.
-  // To implement this flag we would need to do unbuffered reads which
-  // will need to be aligned (not sure there is a guarantee that the buffer
-  // passed in is aligned).
-  // Hence we currently ignore this flag. It is used only in a few cases
-  // which should not be perf critical.
-  // If perf evaluation finds this to be a problem, we can look into
-  // implementing this.
-  bool use_os_buffer_;
-
+class WinSequentialFile : private WinFileData, public SequentialFile {
 public:
   WinSequentialFile(const std::string& fname, HANDLE f,
     const EnvOptions& options);
 
   ~WinSequentialFile();
 
+  WinSequentialFile(const WinSequentialFile&) = delete;
+  WinSequentialFile& operator=(const WinSequentialFile&) = delete;
+
   virtual Status Read(size_t n, Slice* result, char* scratch) override;
 
   virtual Status Skip(uint64_t n) override;
@@ -202,45 +243,49 @@ public:
   virtual Status InvalidateCache(size_t offset, size_t length) override;
 };
 
-// pread() based random-access
-class WinRandomAccessFile : public RandomAccessFile {
-  const std::string filename_;
-  HANDLE hFile_;
-  const bool use_os_buffer_;
-  bool read_ahead_;
+class WinRandomAccessImpl {
+protected:
+
+  WinFileData* file_base_;
+  bool         read_ahead_;
   const size_t compaction_readahead_size_;
   const size_t random_access_max_buffer_size_;
-  mutable std::mutex buffer_mut_;
+  mutable std::mutex    buffer_mut_;
   mutable AlignedBuffer buffer_;
   mutable uint64_t
     buffered_start_;  // file offset set that is currently buffered
 
-  /*
-  * The function reads a requested amount of bytes into the specified aligned
-  * buffer Upon success the function sets the length of the buffer to the
-  * amount of bytes actually read even though it might be less than actually
-  * requested. It then copies the amount of bytes requested by the user (left)
-  * to the user supplied buffer (dest) and reduces left by the amount of bytes
-  * copied to the user buffer
-  *
-  * @user_offset [in] - offset on disk where the read was requested by the user
-  * @first_page_start [in] - actual page aligned disk offset that we want to
-  *                          read from
-  * @bytes_to_read [in] - total amount of bytes that will be read from disk
-  *                       which is generally greater or equal to the amount
-  *                       that the user has requested due to the
-  *                       either alignment requirements or read_ahead in
-  *                       effect.
-  * @left [in/out] total amount of bytes that needs to be copied to the user
-  *                buffer. It is reduced by the amount of bytes that actually
-  *                copied
-  * @buffer - buffer to use
-  * @dest - user supplied buffer
-  */
+  // Override for behavior change when creating a custom env
+  virtual SSIZE_T PositionedReadInternal(char* src, size_t numBytes,
+    uint64_t offset) const;
+
+    /*
+    * The function reads a requested amount of bytes into the specified aligned
+    * buffer Upon success the function sets the length of the buffer to the
+    * amount of bytes actually read even though it might be less than actually
+    * requested. It then copies the amount of bytes requested by the user (left)
+    * to the user supplied buffer (dest) and reduces left by the amount of bytes
+    * copied to the user buffer
+    *
+    * @user_offset [in] - offset on disk where the read was requested by the user
+    * @first_page_start [in] - actual page aligned disk offset that we want to
+    *                          read from
+    * @bytes_to_read [in] - total amount of bytes that will be read from disk
+    *                       which is generally greater or equal to the amount
+    *                       that the user has requested due to the
+    *                       either alignment requirements or read_ahead in
+    *                       effect.
+    * @left [in/out] total amount of bytes that needs to be copied to the user
+    *                buffer. It is reduced by the amount of bytes that actually
+    *                copied
+    * @buffer - buffer to use
+    * @dest - user supplied buffer
+    */
+
   SSIZE_T ReadIntoBuffer(uint64_t user_offset, uint64_t first_page_start,
     size_t bytes_to_read, size_t& left,
     AlignedBuffer& buffer, char* dest) const;
-  
+
   SSIZE_T ReadIntoOneShotBuffer(uint64_t user_offset, uint64_t first_page_start,
     size_t bytes_to_read, size_t& left,
     char* dest) const;
@@ -250,13 +295,27 @@ class WinRandomAccessFile : public RandomAccessFile {
     size_t bytes_to_read, size_t& left,
     char* dest) const;
 
-  void CalculateReadParameters(uint64_t offset, size_t bytes_requested,
-    size_t& actual_bytes_toread,
-    uint64_t& first_page_start) const;
+  WinRandomAccessImpl(WinFileData* file_base, size_t alignment,
+    const EnvOptions& options);
 
-  // Override for behavior change
-  virtual SSIZE_T PositionedReadInternal(char* src, size_t numBytes,
-     uint64_t offset) const;
+  virtual ~WinRandomAccessImpl() {}
+
+public:
+
+  WinRandomAccessImpl(const WinRandomAccessImpl&) = delete;
+  WinRandomAccessImpl& operator=(const WinRandomAccessImpl&) = delete;
+
+
+  Status ReadImpl(uint64_t offset, size_t n, Slice* result,
+    char* scratch) const;
+
+  void HintImpl(RandomAccessFile::AccessPattern pattern);
+};
+
+// pread() based random-access
+class WinRandomAccessFile : private WinFileData, 
+  protected WinRandomAccessImpl, // Want to be able to override PositionedReadInternal
+  public RandomAccessFile {
 
 public:
   WinRandomAccessFile(const std::string& fname, HANDLE hFile, size_t alignment,
@@ -291,18 +350,55 @@ public:
 // the tail for the next write OR for Close() at which point we pad with zeros.
 // No padding is required for
 // buffered access.
-class WinWritableFile : public WritableFile {
-private:
-  const std::string filename_;
-  HANDLE            hFile_;
-  const bool        use_os_buffer_;  // Used to indicate unbuffered access, the file
+class WinWritableImpl {
+protected:
+
+  WinFileData*      file_data_;
   const uint64_t    alignment_;
-  // must be opened as unbuffered if false
   uint64_t          filesize_;      // How much data is actually written disk
   uint64_t          reservedsize_;  // how far we have reserved space
 
   virtual Status PreallocateInternal(uint64_t spaceToReserve);
 
+  WinWritableImpl(WinFileData* file_data, size_t alignment);
+
+  ~WinWritableImpl() {}
+
+  uint64_t GetAlignement() const { return alignment_; }
+
+  Status AppendImpl(const Slice& data);
+
+  // Requires that the data is aligned as specified by GetRequiredBufferAlignment()
+  Status PositionedAppendImpl(const Slice& data, uint64_t offset);
+
+  Status TruncateImpl(uint64_t size);
+
+  Status CloseImpl();
+
+  Status SyncImpl();
+
+  uint64_t GetFileSizeImpl() {
+    // Double accounting now here with WritableFileWriter
+    // and this size will be wrong when unbuffered access is used
+    // but tests implement their own writable files and do not use WritableFileWrapper
+    // so we need to squeeze a square peg through
+    // a round hole here.
+    return filesize_;
+  }
+
+  Status AllocateImpl(uint64_t offset, uint64_t len);
+
+public:
+
+  WinWritableImpl(const WinWritableImpl&) = delete;
+  WinWritableImpl& operator=(const WinWritableImpl&) = delete;
+};
+
+
+class WinWritableFile : private WinFileData,
+  protected WinWritableImpl,
+  public WritableFile {
+
 public:
   WinWritableFile(const std::string& fname, HANDLE hFile, size_t alignment,
     size_t capacity, const EnvOptions& options);
@@ -310,12 +406,14 @@ public:
   ~WinWritableFile();
 
   // Indicates if the class makes use of unbuffered I/O
+  // Use PositionedAppend
   virtual bool UseOSBuffer() const override;
 
   virtual size_t GetRequiredBufferAlignment() const override;
 
   virtual Status Append(const Slice& data) override;
 
+  // Requires that the data is aligned as specified by GetRequiredBufferAlignment()
   virtual Status PositionedAppend(const Slice& data, uint64_t offset) override;
 
   // Need to implement this so the file is truncated correctly
@@ -339,6 +437,57 @@ public:
   virtual size_t GetUniqueId(char* id, size_t max_size) const override;
 };
 
+
+class WinRandomRWFile : private WinFileData,
+  protected WinRandomAccessImpl,
+  protected WinWritableImpl,
+  public RandomRWFile {
+
+public:
+
+  WinRandomRWFile(const std::string& fname, HANDLE hFile, size_t alignment,
+    const EnvOptions& options);
+
+  ~WinRandomRWFile() {}
+
+  // Indicates if the class makes use of unbuffered I/O
+  // If false you must pass aligned buffer to Write()
+  virtual bool UseOSBuffer() const override;
+
+  // Use the returned alignment value to allocate
+  // aligned buffer for Write() when UseOSBuffer()
+  // returns false
+  virtual size_t GetRequiredBufferAlignment() const override;
+
+  // Used by the file_reader_writer to decide if the ReadAhead wrapper
+  // should simply forward the call and do not enact read_ahead buffering or locking.
+  // The implementation below takes care of reading ahead
+  virtual bool ShouldForwardRawRequest() const override;
+
+  // For cases when read-ahead is implemented in the platform dependent
+  // layer. This is when ShouldForwardRawRequest() returns true.
+  virtual void EnableReadAhead() override;
+
+  // Write bytes in `data` at  offset `offset`, Returns Status::OK() on success.
+  // Pass aligned buffer when UseOSBuffer() returns false.
+  virtual Status Write(uint64_t offset, const Slice& data) override;
+
+  // 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.
+  virtual Status Read(uint64_t offset, size_t n, Slice* result,
+    char* scratch) const override;
+
+  virtual Status Flush() override;
+
+  virtual Status Sync() override;
+
+  virtual Status Fsync() { return Sync(); }
+
+  virtual Status Close() override;
+};
+
+
 class WinDirectory : public Directory {
 public:
   WinDirectory() {}