From 3690276e742d60c526480b2ad58a219807897bad Mon Sep 17 00:00:00 2001
From: Siying Dong <siying.d@fb.com>
Date: Wed, 9 May 2018 10:24:11 -0700
Subject: [PATCH] Disallow to open RandomRW file if the file doesn't exist

Summary:
The only use of RandomRW is to change seqno when bulkloading, and in this use case, the file should exist. We should fail the file opening in this case.
Closes https://github.com/facebook/rocksdb/pull/3827

Differential Revision: D7913719

Pulled By: siying

fbshipit-source-id: 62cf6734f1a6acb9e14f715b927da388131c3492
---
 HISTORY.md       |  1 +
 env/env_posix.cc |  3 +--
 env/env_test.cc  | 28 ++++++++++++++++++++++++++++
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/HISTORY.md b/HISTORY.md
index 0ecefece80..5f6c9addd0 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -5,6 +5,7 @@
 * The background thread naming convention changed (on supporting platforms) to "rocksdb:<thread pool priority><thread number>", e.g., "rocksdb:low0".
 * Add a new ticker stat rocksdb.number.multiget.keys.found to count number of keys successfully read in MultiGet calls
 * Touch-up to write-related counters in PerfContext. New counters added: write_scheduling_flushes_compactions_time, write_thread_wait_nanos. Counters whose behavior was fixed or modified: write_memtable_time, write_pre_and_post_process_time, write_delay_time.
+* Posix Env's NewRandomRWFile() will fail if the file doesn't exist.
 
 ### New Features
 * Introduce TTL for level compaction so that all files older than ttl go through the compaction process to get rid of old data.
diff --git a/env/env_posix.cc b/env/env_posix.cc
index 038d7c44a4..660b3cf07a 100644
--- a/env/env_posix.cc
+++ b/env/env_posix.cc
@@ -441,8 +441,7 @@ class PosixEnv : public Env {
     int fd = -1;
     while (fd < 0) {
       IOSTATS_TIMER_GUARD(open_nanos);
-      fd = open(fname.c_str(), O_CREAT | O_RDWR,
-                GetDBFileMode(allow_non_owner_access_));
+      fd = open(fname.c_str(), O_RDWR, GetDBFileMode(allow_non_owner_access_));
       if (fd < 0) {
         // Error while opening the file
         if (errno == EINTR) {
diff --git a/env/env_test.cc b/env/env_test.cc
index 3e5892437e..6d2f0d8994 100644
--- a/env/env_test.cc
+++ b/env/env_test.cc
@@ -175,6 +175,8 @@ TEST_F(EnvPosixTest, DISABLED_FilePermission) {
         test::TmpDir(env_) + "/testfile", test::TmpDir(env_) + "/testfile1"};
     unique_ptr<WritableFile> wfile;
     ASSERT_OK(env_->NewWritableFile(fileNames[0], &wfile, soptions));
+    ASSERT_OK(env_->NewWritableFile(fileNames[1], &wfile, soptions));
+    wfile.reset();
     unique_ptr<RandomRWFile> rwfile;
     ASSERT_OK(env_->NewRandomRWFile(fileNames[1], &rwfile, soptions));
 
@@ -188,6 +190,8 @@ TEST_F(EnvPosixTest, DISABLED_FilePermission) {
 
     env_->SetAllowNonOwnerAccess(false);
     ASSERT_OK(env_->NewWritableFile(fileNames[0], &wfile, soptions));
+    ASSERT_OK(env_->NewWritableFile(fileNames[1], &wfile, soptions));
+    wfile.reset();
     ASSERT_OK(env_->NewRandomRWFile(fileNames[1], &rwfile, soptions));
 
     for (const auto& filename : fileNames) {
@@ -1432,6 +1436,18 @@ TEST_P(EnvPosixTestWithParam, PosixRandomRWFile) {
   env_->DeleteFile(path);
 
   std::unique_ptr<RandomRWFile> file;
+
+#ifdef OS_LINUX
+  // Cannot open non-existing file.
+  ASSERT_NOK(env_->NewRandomRWFile(path, &file, EnvOptions()));
+#endif
+
+  // Create the file using WriteableFile
+  {
+    std::unique_ptr<WritableFile> wf;
+    ASSERT_OK(env_->NewWritableFile(path, &wf, EnvOptions()));
+  }
+
   ASSERT_OK(env_->NewRandomRWFile(path, &file, EnvOptions()));
 
   char buf[10000];
@@ -1549,6 +1565,18 @@ TEST_P(EnvPosixTestWithParam, PosixRandomRWFileRandomized) {
   env_->DeleteFile(path);
 
   unique_ptr<RandomRWFile> file;
+
+#ifdef OS_LINUX
+  // Cannot open non-existing file.
+  ASSERT_NOK(env_->NewRandomRWFile(path, &file, EnvOptions()));
+#endif
+
+  // Create the file using WriteableFile
+  {
+    std::unique_ptr<WritableFile> wf;
+    ASSERT_OK(env_->NewWritableFile(path, &wf, EnvOptions()));
+  }
+
   ASSERT_OK(env_->NewRandomRWFile(path, &file, EnvOptions()));
   RandomRWFileWithMirrorString file_with_mirror(file.get());