Fix Windows environment issues

Summary:
Enable directIO on WritableFileImpl::Append
     with offset being current length of the file.
     Enable UniqueID tests on Windows, disable others but
     leeting them to compile. Unique tests are valuable to
     detect failures on different filesystems and upcoming
     ReFS.
     Clear output in WinEnv Getchildren.This is different from
     previous strategy, do not touch output on failure.
     Make sure DBTest.OpenWhenOpen works with windows error message
Closes https://github.com/facebook/rocksdb/pull/1746

Differential Revision: D4385681

Pulled By: IslamAbdelRahman

fbshipit-source-id: c07b702

db/db_test.cc

@@ -205,7 +205,7 @@ TEST_F(DBTest, OpenWhenOpen) {
 
   ASSERT_EQ(Status::Code::kIOError, s.code());
   ASSERT_EQ(Status::SubCode::kNone, s.subcode());
-  ASSERT_TRUE(strncmp("lock ", s.getState(), 5) == 0);
+  ASSERT_TRUE(strstr(s.getState(), "lock ") != nullptr);
 
   delete db2;
 }

port/win/env_win.cc

@@ -361,6 +361,8 @@ Status WinEnvIO::FileExists(const std::string& fname) {
 
 Status WinEnvIO::GetChildren(const std::string& dir,
   std::vector<std::string>* result) {
+
+  result->clear();
   std::vector<std::string> output;
 
   Status status;

port/win/io_win.cc

@@ -18,6 +18,30 @@
 namespace rocksdb {
 namespace port {
 
+/*
+* DirectIOHelper
+*/
+namespace {
+
+const size_t kSectorSize = 512;
+
+inline
+bool IsPowerOfTwo(const size_t alignment) {
+  return ((alignment) & (alignment - 1)) == 0;
+}
+
+inline
+bool IsSectorAligned(const size_t off) { 
+  return (off & (kSectorSize - 1)) == 0;
+}
+
+inline
+bool IsAligned(size_t alignment, const void* ptr) {
+  return ((uintptr_t(ptr)) & (alignment - 1)) == 0;
+}
+}
+
+
 std::string GetWindowsErrSz(DWORD err) {
   LPSTR lpMsgBuf;
   FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM |
@@ -135,6 +159,8 @@ size_t GetUniqueIdFromFile(HANDLE hFile, char* id, size_t max_size) {
     return 0;
   }
 
+  // This function has to be re-worked for cases when
+  // ReFS file system introduced on Windows Server 2012 is used
   BY_HANDLE_FILE_INFORMATION FileInfo;
 
   BOOL result = GetFileInformationByHandle(hFile, &FileInfo);
@@ -847,22 +873,50 @@ WinWritableImpl::WinWritableImpl(WinFileData* file_data, size_t alignment)
 
 Status WinWritableImpl::AppendImpl(const Slice& data) {
 
-  // Used for buffered access ONLY
-  assert(!file_data_->UseDirectIO());
+  Status s;
+
   assert(data.size() < std::numeric_limits<DWORD>::max());
 
-  Status s;
+  uint64_t written = 0;
 
-  DWORD bytesWritten = 0;
-  if (!WriteFile(file_data_->GetFileHandle(), data.data(),
-    static_cast<DWORD>(data.size()), &bytesWritten, NULL)) {
-    auto lastError = GetLastError();
-    s = IOErrorFromWindowsError(
-      "Failed to WriteFile: " + file_data_->GetName(),
-      lastError);
+  if (file_data_->UseDirectIO()) {
+
+    // With no offset specified we are appending
+    // to the end of the file
+
+    assert(IsSectorAligned(filesize_));
+    assert(IsSectorAligned(data.size()));
+    assert(IsAligned(GetAlignement(), data.data()));
+
+    SSIZE_T ret = pwrite(file_data_->GetFileHandle(), data.data(),
+     data.size(), filesize_);
+
+    if (ret < 0) {
+      auto lastError = GetLastError();
+      s = IOErrorFromWindowsError(
+        "Failed to pwrite for: " + file_data_->GetName(), lastError);
+    }
+    else {
+      written = ret;
+    }
+
+  } else {
+
+    DWORD bytesWritten = 0;
+    if (!WriteFile(file_data_->GetFileHandle(), data.data(),
+      static_cast<DWORD>(data.size()), &bytesWritten, NULL)) {
+      auto lastError = GetLastError();
+      s = IOErrorFromWindowsError(
+        "Failed to WriteFile: " + file_data_->GetName(),
+        lastError);
+    }
+    else {
+      written = bytesWritten;
+    }
   }
-  else {
-    assert(size_t(bytesWritten) == data.size());
+
+  if(s.ok()) {
+    assert(written == data.size());
     filesize_ += data.size();
   }
 
@@ -870,6 +924,13 @@ Status WinWritableImpl::AppendImpl(const Slice& data) {
 }
 
 Status WinWritableImpl::PositionedAppendImpl(const Slice& data, uint64_t offset) {
+
+  if(file_data_->UseDirectIO()) {
+    assert(IsSectorAligned(offset));
+    assert(IsSectorAligned(data.size()));
+    assert(IsAligned(GetAlignement(), data.data()));
+  }
+
   Status s;
 
   SSIZE_T ret = pwrite(file_data_->GetFileHandle(), data.data(), data.size(), offset);

util/env_test.cc

@@ -529,7 +529,7 @@ TEST_P(EnvPosixTestWithParam, DecreaseNumBgThreads) {
   ASSERT_TRUE(!tasks[5].IsSleeping());
 }
 
-#ifdef OS_LINUX
+#if (defined OS_LINUX || defined OS_WIN)
 // Travis doesn't support fallocate or getting unique ID from files for whatever
 // reason.
 #ifndef TRAVIS
@@ -564,6 +564,9 @@ char temp_id[MAX_ID_SIZE];
 // Note that this function "knows" that dir has just been created
 // and is empty, so we create a simply-named test file: "f".
 bool ioctl_support__FS_IOC_GETVERSION(const std::string& dir) {
+#ifdef OS_WIN
+  return true;
+#else
   const std::string file = dir + "/f";
   int fd;
   do {
@@ -576,6 +579,7 @@ bool ioctl_support__FS_IOC_GETVERSION(const std::string& dir) {
   unlink(file.c_str());
 
   return ok;
+#endif
 }
 
 // To ensure that Env::GetUniqueId-related tests work correctly, the files
@@ -590,10 +594,25 @@ class IoctlFriendlyTmpdir {
  public:
   explicit IoctlFriendlyTmpdir() {
     char dir_buf[100];
-    std::list<std::string> candidate_dir_list = {"/var/tmp", "/tmp"};
 
     const char *fmt = "%s/rocksdb.XXXXXX";
     const char *tmp = getenv("TEST_IOCTL_FRIENDLY_TMPDIR");
+
+#ifdef OS_WIN
+#define rmdir _rmdir
+    if(tmp == nullptr) {
+      tmp = getenv("TMP");
+    }
+
+    snprintf(dir_buf, sizeof dir_buf, fmt, tmp);
+    auto result = _mktemp(dir_buf);
+    assert(result != nullptr);
+    BOOL ret = CreateDirectory(dir_buf, NULL);
+    assert(ret == TRUE);
+    dir_ = dir_buf;
+#else
+    std::list<std::string> candidate_dir_list = {"/var/tmp", "/tmp"};
+
     // If $TEST_IOCTL_FRIENDLY_TMPDIR/rocksdb.XXXXXX fits, use
     // $TEST_IOCTL_FRIENDLY_TMPDIR; subtract 2 for the "%s", and
     // add 1 for the trailing NUL byte.
@@ -627,12 +646,14 @@ class IoctlFriendlyTmpdir {
     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());
   }
-  const std::string& name() {
+
+  const std::string& name() const {
     return dir_;
   }
 
@@ -807,7 +828,7 @@ bool HasPrefix(const std::unordered_set<std::string>& ss) {
   return false;
 }
 
-// Only works in linux platforms
+// Only works in linux and WIN platforms
 TEST_F(EnvPosixTest, RandomAccessUniqueIDConcurrent) {
   for (bool directio : {true, false}) {
     // Check whether a bunch of concurrently existing files have unique IDs.
@@ -849,7 +870,7 @@ TEST_F(EnvPosixTest, RandomAccessUniqueIDConcurrent) {
   }
 }
 
-// Only works in linux platforms
+// Only works in linux and WIN platforms
 TEST_F(EnvPosixTest, RandomAccessUniqueIDDeletes) {
   for (bool directio : {true, false}) {
     EnvOptions soptions;
@@ -891,7 +912,11 @@ TEST_F(EnvPosixTest, RandomAccessUniqueIDDeletes) {
 }
 
 // Only works in linux platforms
+#ifdef OS_WIN
+TEST_P(EnvPosixTestWithParam, DISABLED_InvalidateCache) {
+#else
 TEST_P(EnvPosixTestWithParam, InvalidateCache) {
+#endif
   rocksdb::SyncPoint::GetInstance()->EnableProcessing();
   for (bool directio : {true, false}) {
     EnvOptions soptions;
@@ -915,9 +940,9 @@ TEST_P(EnvPosixTestWithParam, InvalidateCache) {
       }
 #endif
       ASSERT_OK(env_->NewWritableFile(fname, &wfile, soptions));
-      ASSERT_OK(wfile.get()->Append(slice));
-      ASSERT_OK(wfile.get()->InvalidateCache(0, 0));
-      ASSERT_OK(wfile.get()->Close());
+      ASSERT_OK(wfile->Append(slice));
+      ASSERT_OK(wfile->InvalidateCache(0, 0));
+      ASSERT_OK(wfile->Close());
     }
 
     // Random Read
@@ -935,10 +960,10 @@ TEST_P(EnvPosixTestWithParam, InvalidateCache) {
       }
 #endif
       ASSERT_OK(env_->NewRandomAccessFile(fname, &file, soptions));
-      ASSERT_OK(file.get()->Read(0, kSectorSize, &result, scratch));
+      ASSERT_OK(file->Read(0, kSectorSize, &result, scratch));
       ASSERT_EQ(memcmp(scratch, data.get(), kSectorSize), 0);
-      ASSERT_OK(file.get()->InvalidateCache(0, 11));
-      ASSERT_OK(file.get()->InvalidateCache(0, 0));
+      ASSERT_OK(file->InvalidateCache(0, 11));
+      ASSERT_OK(file->InvalidateCache(0, 0));
     }
 
     // Sequential Read
@@ -956,10 +981,10 @@ TEST_P(EnvPosixTestWithParam, InvalidateCache) {
       }
 #endif
       ASSERT_OK(env_->NewSequentialFile(fname, &file, soptions));
-      ASSERT_OK(file.get()->Read(kSectorSize, &result, scratch));
+      ASSERT_OK(file->Read(kSectorSize, &result, scratch));
       ASSERT_EQ(memcmp(scratch, data.get(), kSectorSize), 0);
-      ASSERT_OK(file.get()->InvalidateCache(0, 11));
-      ASSERT_OK(file.get()->InvalidateCache(0, 0));
+      ASSERT_OK(file->InvalidateCache(0, 11));
+      ASSERT_OK(file->InvalidateCache(0, 0));
     }
     // Delete the file
     ASSERT_OK(env_->DeleteFile(fname));
@@ -967,7 +992,7 @@ TEST_P(EnvPosixTestWithParam, InvalidateCache) {
   rocksdb::SyncPoint::GetInstance()->ClearTrace();
 }
 #endif  // not TRAVIS
-#endif  // OS_LINUX
+#endif  // OS_LINUX || OS_WIN
 
 class TestLogger : public Logger {
  public:
@@ -1087,6 +1112,7 @@ TEST_P(EnvPosixTestWithParam, LogBufferMaxSizeTest) {
 
 TEST_P(EnvPosixTestWithParam, Preallocation) {
   rocksdb::SyncPoint::GetInstance()->EnableProcessing();
+
   for (bool directio : {true, false}) {
     const std::string src = test::TmpDir(env_) + "/" + "testfile";
     unique_ptr<WritableFile> srcfile;

utilities/transactions/transaction_test.cc

@@ -223,8 +223,13 @@ TEST_P(TransactionTest, WaitingTxn) {
 
   auto cf_iterator = lock_data.begin();
 
-  // Column family is 1 (cfa).
-  ASSERT_EQ(cf_iterator->first, 1);
+  // The iterator points to an unordered_multimap
+  // thus the test can not assume any particular order.
+
+  // Column family is 1 or 0 (cfa).
+  if (cf_iterator->first != 1 && cf_iterator->first != 0) {
+    ASSERT_FALSE(true);
+  }
   // The locked key is "foo" and is locked by txn1
   ASSERT_EQ(cf_iterator->second.key, "foo");
   ASSERT_EQ(cf_iterator->second.ids.size(), 1);
@@ -232,8 +237,10 @@ TEST_P(TransactionTest, WaitingTxn) {
 
   cf_iterator++;
 
-  // Column family is 0 (default).
-  ASSERT_EQ(cf_iterator->first, 0);
+  // Column family is 0 (default) or 1.
+  if (cf_iterator->first != 1 && cf_iterator->first != 0) {
+    ASSERT_FALSE(true);
+  }
   // The locked key is "foo" and is locked by txn1
   ASSERT_EQ(cf_iterator->second.key, "foo");
   ASSERT_EQ(cf_iterator->second.ids.size(), 1);