LevelDB: Using base::File for all file I/O

third_party/leveldatabase/env_chromium.cc

 // Copyright (c) 2011 The LevelDB Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file. See the AUTHORS file for names of contributors.
 
 #include "third_party/leveldatabase/env_chromium.h"
 
-#if defined(OS_WIN)
-#include <io.h>
-#endif
-
 #include "base/debug/trace_event.h"
+#include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
 #include "base/lazy_instance.h"
+#include "base/memory/shared_memory.h"
 #include "base/metrics/histogram.h"
+#include "base/process/process_metrics.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "third_party/leveldatabase/env_chromium_stdio.h"
+#include "third_party/leveldatabase/chromium_logger.h"
 #include "third_party/re2/re2/re2.h"
 
-#if defined(OS_WIN)
-#include "base/command_line.h"
-#include "base/win/win_util.h"
-#include "third_party/leveldatabase/env_chromium_win.h"
-#endif
-
 using leveldb::FileLock;
 using leveldb::Slice;
 using leveldb::Status;
 
 namespace leveldb_env {
 
 namespace {
 
 const base::FilePath::CharType backup_table_extension[] =
     FILE_PATH_LITERAL(".bak");
 const base::FilePath::CharType table_extension[] = FILE_PATH_LITERAL(".ldb");
 
 static const base::FilePath::CharType kLevelDBTestDirectoryPrefix[]
     = FILE_PATH_LITERAL("leveldb-test-");
 
+static base::File::Error LastFileError() {
+#if defined(OS_WIN)
+  return base::File::OSErrorToFileError(GetLastError());
+#else
+  return base::File::OSErrorToFileError(errno);
+#endif
+}
+
 class ChromiumFileLock : public FileLock {
  public:
   ::base::File file_;
   std::string name_;
 };
 
 class Retrier {
  public:
   Retrier(MethodID method, RetrierProvider* provider)
       : start_(base::TimeTicks::Now()),
@@ skipping 30 matching lines @@
   base::TimeTicks start_;
   base::TimeTicks limit_;
   base::TimeTicks last_;
   base::TimeDelta time_to_sleep_;
   bool success_;
   MethodID method_;
   base::File::Error last_error_;
   RetrierProvider* provider_;
 };
 
-class IDBEnvStdio : public ChromiumEnvStdio {
+class ChromiumSequentialFile : public leveldb::SequentialFile {
  public:
-  IDBEnvStdio() : ChromiumEnvStdio() {
+  ChromiumSequentialFile(const std::string& fname,
+                         base::File* f,
+                         const UMALogger* uma_logger)
+      : filename_(fname), file_(f), uma_logger_(uma_logger) {}
+  virtual ~ChromiumSequentialFile() {}
+
+  Status Read(size_t n, Slice* result, char* scratch) override {
+    int bytes_read = file_->ReadAtCurrentPos(scratch, n);
+    if (bytes_read == -1) {
+      base::File::Error error = LastFileError();
+      uma_logger_->RecordErrorAt(kSequentialFileRead);
+      return MakeIOError(filename_, base::File::ErrorToString(error),
+                         kSequentialFileRead, error);
+    } else {
+      *result = Slice(scratch, bytes_read);
+      return Status::OK();
+    }
+  }
+
+  Status Skip(uint64_t n) override {
+    if (file_->Seek(base::File::FROM_CURRENT, n) == -1) {
+      base::File::Error error = LastFileError();
+      uma_logger_->RecordErrorAt(kSequentialFileSkip);
+      return MakeIOError(filename_, base::File::ErrorToString(error),
+                         kSequentialFileSkip, error);
+    } else {
+      return Status::OK();
+    }
+  }
+
+ private:
+  std::string filename_;
+  scoped_ptr<base::File> file_;
+  const UMALogger* uma_logger_;
+};
+
+class ChromiumRandomAccessFile : public leveldb::RandomAccessFile {
+ public:
+  ChromiumRandomAccessFile(const std::string& fname,
+                           ::base::File file,
+                           const UMALogger* uma_logger)
+      : filename_(fname), file_(file.Pass()), uma_logger_(uma_logger) {}
+  virtual ~ChromiumRandomAccessFile() {}
+
+  Status Read(uint64_t offset,
+                      size_t n,
+                      Slice* result,
+                      char* scratch) const override {
+    Status s;
+    int r = file_.Read(offset, scratch, n);
+    *result = Slice(scratch, (r < 0) ? 0 : r);
+    if (r < 0) {
+      // An error: return a non-ok status
+      s = MakeIOError(filename_, "Could not perform read",
+                      kRandomAccessFileRead);
+      uma_logger_->RecordErrorAt(kRandomAccessFileRead);
+    }
+    return s;
+  }
+
+ private:
+  std::string filename_;
+  mutable ::base::File file_;
+  const UMALogger* uma_logger_;
+};
+
+class ChromiumWritableFile : public leveldb::WritableFile {
+ public:
+  ChromiumWritableFile(const std::string& fname,
+                       base::File* f,
+                       const UMALogger* uma_logger,
+                       WriteTracker* tracker,
+                       bool make_backup);
+  virtual ~ChromiumWritableFile() {}
+  leveldb::Status Append(const leveldb::Slice& data) override;
+  leveldb::Status Close() override;
+  leveldb::Status Flush() override;
+  leveldb::Status Sync() override;
+
+ private:
+  enum Type { kManifest, kTable, kOther };
+  leveldb::Status SyncParent();
+
+  std::string filename_;
+  scoped_ptr<base::File> file_;
+  const UMALogger* uma_logger_;
+  WriteTracker* tracker_;
+  Type file_type_;
+  std::string parent_dir_;
+  bool make_backup_;
+};
+
+ChromiumWritableFile::ChromiumWritableFile(const std::string& fname,
+                                           base::File* f,
+                                           const UMALogger* uma_logger,
+                                           WriteTracker* tracker,
+                                           bool make_backup)
+    : filename_(fname),
+      file_(f),
+      uma_logger_(uma_logger),
+      tracker_(tracker),
+      file_type_(kOther),
+      make_backup_(make_backup) {
+  base::FilePath path = base::FilePath::FromUTF8Unsafe(fname);
+  if (FilePathToString(path.BaseName()).find("MANIFEST") == 0)
+    file_type_ = kManifest;
+  else if (ChromiumEnv::HasTableExtension(path))
+    file_type_ = kTable;
+  if (file_type_ != kManifest)
+    tracker_->DidCreateNewFile(filename_);
+  parent_dir_ = FilePathToString(ChromiumEnv::CreateFilePath(fname).DirName());
+}
+
+Status ChromiumWritableFile::SyncParent() {
+  TRACE_EVENT0("leveldb", "SyncParent");
+#if defined(OS_POSIX)
+  base::FilePath path = base::FilePath::FromUTF8Unsafe(parent_dir_);
+  base::File f(path, base::File::FLAG_OPEN | base::File::FLAG_READ);
+  if (!f.IsValid()) {
+    return MakeIOError(parent_dir_, "Unable to open directory", kSyncParent,
+                       f.error_details());
+  }
+  if (!f.Flush()) {
+    base::File::Error error = LastFileError();
+    return MakeIOError(parent_dir_, base::File::ErrorToString(error),
+                       kSyncParent, error);
+  }
+#endif
+  return Status::OK();
+}
+
+Status ChromiumWritableFile::Append(const Slice& data) {
+  if (file_type_ == kManifest && tracker_->DoesDirNeedSync(filename_)) {
+    Status s = SyncParent();
+    if (!s.ok())
+      return s;
+    tracker_->DidSyncDir(filename_);
+  }
+
+  int bytes_written = file_->WriteAtCurrentPos(data.data(), data.size());
+  if (bytes_written != data.size()) {
+    base::File::Error error = LastFileError();
+    uma_logger_->RecordOSError(kWritableFileAppend, error);
+    return MakeIOError(filename_, base::File::ErrorToString(error),
+                       kWritableFileAppend, error);
+  }
+
+  return Status::OK();
+}
+
+Status ChromiumWritableFile::Close() {
+  file_->Close();
+  return Status::OK();
+}
+
+Status ChromiumWritableFile::Flush() {
+  // base::File doesn't do buffered I/O (i.e. POSIX FILE streams) so nothing to
+  // flush.
+  return Status::OK();
+}
+
+Status ChromiumWritableFile::Sync() {
+  TRACE_EVENT0("leveldb", "WritableFile::Sync");
+
+  if (!file_->Flush()) {
+    base::File::Error error = LastFileError();
+    uma_logger_->RecordErrorAt(kWritableFileSync);
+    return MakeIOError(filename_, base::File::ErrorToString(error),
+                       kWritableFileSync, error);
+  }
+
+  if (make_backup_ && file_type_ == kTable)
+    uma_logger_->RecordBackupResult(ChromiumEnv::MakeBackup(filename_));
+
+  return Status::OK();
+}
+
+class IDBEnv : public ChromiumEnv {
+ public:
+  IDBEnv() : ChromiumEnv() {
     name_ = "LevelDBEnv.IDB";
     make_backup_ = true;
   }
 };
 
-#if defined(OS_WIN)
-class IDBEnvWin : public ChromiumEnvWin {
- public:
-  IDBEnvWin() : ChromiumEnvWin() {
-    name_ = "LevelDBEnv.IDB";
-    make_backup_ = true;
-  }
-};
-#endif
+::base::LazyInstance<IDBEnv>::Leaky idb_env = LAZY_INSTANCE_INITIALIZER;
 
-#if defined(OS_WIN)
-::base::LazyInstance<IDBEnvWin>::Leaky idb_env =
-    LAZY_INSTANCE_INITIALIZER;
-#else
-::base::LazyInstance<IDBEnvStdio>::Leaky idb_env =
-    LAZY_INSTANCE_INITIALIZER;
-#endif
-
-::base::LazyInstance<ChromiumEnvStdio>::Leaky default_env =
+::base::LazyInstance<ChromiumEnv>::Leaky default_env =
     LAZY_INSTANCE_INITIALIZER;
 
 }  // unnamed namespace
 
 const char* MethodIDToString(MethodID method) {
   switch (method) {
     case kSequentialFileRead:
       return "SequentialFileRead";
     case kSequentialFileSkip:
       return "SequentialFileSkip";
@@ skipping 37 matching lines @@
       return "GetChildren";
     case kNumEntries:
       NOTREACHED();
       return "kNumEntries";
   }
   NOTREACHED();
   return "Unknown";
 }
 
 Status MakeIOError(Slice filename,
-                   const char* message,
+                   const std::string& message,
                    MethodID method,
-                   int saved_errno) {
+                   base::File::Error error) {
+  DCHECK_LT(error, 0);
   char buf[512];
-  snprintf(buf,
-           sizeof(buf),
-           "%s (ChromeMethodErrno: %d::%s::%d)",
-           message,
-           method,
-           MethodIDToString(method),
-           saved_errno);
+  snprintf(buf, sizeof(buf), "%s (ChromeMethodPFE: %d::%s::%d)",
+           message.c_str(), method, MethodIDToString(method), -error);
   return Status::IOError(filename, buf);
 }
 
 Status MakeIOError(Slice filename,
-                   const char* message,
-                   MethodID method,
-                   base::File::Error error) {
-  DCHECK_LT(error, 0);
+                   const std::string& message,
+                   MethodID method) {
   char buf[512];
-  snprintf(buf,
-           sizeof(buf),
-           "%s (ChromeMethodPFE: %d::%s::%d)",
-           message,
-           method,
-           MethodIDToString(method),
-           -error);
+  snprintf(buf, sizeof(buf), "%s (ChromeMethodOnly: %d::%s)", message.c_str(),
+           method, MethodIDToString(method));
   return Status::IOError(filename, buf);
 }
 
-Status MakeIOError(Slice filename, const char* message, MethodID method) {
-  char buf[512];
-  snprintf(buf,
-           sizeof(buf),
-           "%s (ChromeMethodOnly: %d::%s)",
-           message,
-           method,
-           MethodIDToString(method));
-  return Status::IOError(filename, buf);
-}
-
 ErrorParsingResult ParseMethodAndError(const char* string,
                                        MethodID* method_param,
                                        int* error) {
   int method;
   if (RE2::PartialMatch(string, "ChromeMethodOnly: (\\d+)", &method)) {
     *method_param = static_cast<MethodID>(method);
     return METHOD_ONLY;
   }
   if (RE2::PartialMatch(
           string, "ChromeMethodPFE: (\\d+)::.*::(\\d+)", &method, error)) {
@@ skipping 233 matching lines @@
        it != backups_only.end();
        ++it) {
     base::FilePath restored_table_name =
         RestoreFromBackup(dir_filepath.Append(*it));
     result->push_back(FilePathToString(restored_table_name.BaseName()));
   }
 }
 
 Status ChromiumEnv::GetChildren(const std::string& dir_string,
                                 std::vector<std::string>* result) {
-  std::vector<base::FilePath> entries;
-  base::File::Error error =
-      GetDirectoryEntries(CreateFilePath(dir_string), &entries);
-  if (error != base::File::FILE_OK) {
-    RecordOSError(kGetChildren, error);
-    return MakeIOError(
-        dir_string, "Could not open/read directory", kGetChildren, error);
+  result->clear();
+  base::FilePath dir_path = CreateFilePath(dir_string);
+  if (!DirectoryExists(dir_path)) {
+    RecordErrorAt(kGetChildren);
+    return MakeIOError(dir_string, "Could not open/read directory",
+                       kGetChildren);
   }
-  result->clear();
-  for (std::vector<base::FilePath>::iterator it = entries.begin();
-       it != entries.end();
-       ++it) {
-    result->push_back(FilePathToString(*it));
+  base::FileEnumerator enumerator(dir_path,

[dgrogan, 2014/11/12 22:45:11]

As Josh hinted, moving back to FileEnumerator will regress the corruption rate.
I recommend not doing so, at least not without explicitly weighing the benefit
against the increased data loss users will experience.

First glance suggests that the benefit (-~50 lines of code) is small. The cost
is that +~50 IDB users and +~200 non-IDB users of LevelDB will suffer data loss
each day[1].

So my vote is to stick with the existing, non-FileEnumerator implementations of
GetChildren.

[1]
https://uma.googleplex.com/histograms/?dayCount=1&endDate=11-11-2014&version=...

[cmumford, 2014/11/12 23:52:41]

So right now LevelDB calls GetChildren in four places. It ignores the status
code for two of them. Of the two that it checks it treats an empty list the same
as an error. But that being said I'm not a fan of ignoring failures. I'm
currently chatting with rvargas to see if he's OK with improving FileEnumerator.
If I can't get buyoff on that then I'll go back to our original implementation.

+                                  false,  // not recursive
+                                  base::FileEnumerator::FILES);
+  for (base::FilePath current = enumerator.Next(); !current.empty();
+       current = enumerator.Next()) {
+    result->push_back(FilePathToString(current.BaseName()));
   }
 
   if (make_backup_)
     RestoreIfNecessary(dir_string, result);
   return Status::OK();
 }
 
 Status ChromiumEnv::DeleteFile(const std::string& fname) {
   Status result;
   base::FilePath fname_filepath = CreateFilePath(fname);
@@ skipping 156 matching lines @@
       RecordErrorAt(kGetTestDirectory);
       return MakeIOError(
           "Could not create temp directory.", "", kGetTestDirectory);
     }
   }
   *path = FilePathToString(test_directory_);
   mu_.Release();
   return Status::OK();
 }
 
+Status ChromiumEnv::NewLogger(const std::string& fname,
+                              leveldb::Logger** result) {
+  base::FilePath path = CreateFilePath(fname);
+  scoped_ptr<base::File> f(new base::File(
+      path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE));
+  if (!f->IsValid()) {
+    *result = NULL;
+    RecordOSError(kNewLogger, f->error_details());
+    return MakeIOError(fname, "Unable to create log file", kNewLogger,
+                       f->error_details());
+  } else {
+    *result = new leveldb::ChromiumLogger(f.release());
+    return Status::OK();
+  }
+}
+
+Status ChromiumEnv::NewSequentialFile(const std::string& fname,
+                                      leveldb::SequentialFile** result) {
+  base::FilePath path = CreateFilePath(fname);
+  scoped_ptr<base::File> f(
+      new base::File(path, base::File::FLAG_OPEN | base::File::FLAG_READ));
+  if (!f->IsValid()) {
+    *result = NULL;
+    RecordOSError(kNewSequentialFile, f->error_details());
+    return MakeIOError(fname, "Unable to create sequential file",
+                       kNewSequentialFile, f->error_details());
+  } else {
+    *result = new ChromiumSequentialFile(fname, f.release(), this);
+    return Status::OK();
+  }
+}
+
+void ChromiumEnv::RecordOpenFilesLimit(const std::string& type) {
+#if defined(OS_POSIX)
+  GetMaxFDHistogram(type)->Add(base::GetMaxFds());
+#elif defined(OS_WIN)
+  // Windows is only limited by available memory
+#else
+#error "Need to determine limit to open files for this OS"
+#endif
+}
+
+Status ChromiumEnv::NewRandomAccessFile(const std::string& fname,
+                                        leveldb::RandomAccessFile** result) {
+  int flags = ::base::File::FLAG_READ | ::base::File::FLAG_OPEN;
+  ::base::File file(ChromiumEnv::CreateFilePath(fname), flags);
+  if (file.IsValid()) {
+    *result = new ChromiumRandomAccessFile(fname, file.Pass(), this);
+    RecordOpenFilesLimit("Success");
+    return Status::OK();
+  }
+  ::base::File::Error error_code = file.error_details();
+  if (error_code == ::base::File::FILE_ERROR_TOO_MANY_OPENED)
+    RecordOpenFilesLimit("TooManyOpened");
+  else
+    RecordOpenFilesLimit("OtherError");
+  *result = NULL;
+  RecordOSError(kNewRandomAccessFile, error_code);
+  return MakeIOError(fname, FileErrorString(error_code), kNewRandomAccessFile,
+                     error_code);
+}
+
+Status ChromiumEnv::NewWritableFile(const std::string& fname,
+                                    leveldb::WritableFile** result) {
+  *result = NULL;
+  base::FilePath path = CreateFilePath(fname);
+  scoped_ptr<base::File> f(new base::File(
+      path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE));
+  if (!f->IsValid()) {
+    RecordErrorAt(kNewWritableFile);
+    return MakeIOError(fname, "Unable to create writable file",
+                       kNewWritableFile, f->error_details());
+  } else {
+    *result =
+        new ChromiumWritableFile(fname, f.release(), this, this, make_backup_);
+    return Status::OK();
+  }
+}
+
 uint64_t ChromiumEnv::NowMicros() {
   return ::base::TimeTicks::Now().ToInternalValue();
 }
 
 void ChromiumEnv::SleepForMicroseconds(int micros) {
   // Round up to the next millisecond.
   ::base::PlatformThread::Sleep(::base::TimeDelta::FromMicroseconds(micros));
 }
 
 void ChromiumEnv::RecordErrorAt(MethodID method) const {
   GetMethodIOErrorHistogram()->Add(method);
 }
 
 void ChromiumEnv::RecordLockFileAncestors(int num_missing_ancestors) const {
   GetLockFileAncestorHistogram()->Add(num_missing_ancestors);
 }
 
 void ChromiumEnv::RecordOSError(MethodID method,
                                 base::File::Error error) const {
   DCHECK_LT(error, 0);
   RecordErrorAt(method);
   GetOSErrorHistogram(method, -base::File::FILE_ERROR_MAX)->Add(-error);
 }
 
-void ChromiumEnv::RecordOSError(MethodID method, int error) const {
-  DCHECK_GT(error, 0);
-  RecordErrorAt(method);
-  GetOSErrorHistogram(method, ERANGE + 1)->Add(error);
-}
-
 void ChromiumEnv::RecordBackupResult(bool result) const {
   std::string uma_name(name_);
   uma_name.append(".TableBackup");
   base::BooleanHistogram::FactoryGet(
       uma_name, base::Histogram::kUmaTargetedHistogramFlag)->AddBoolean(result);
 }
 
 base::HistogramBase* ChromiumEnv::GetOSErrorHistogram(MethodID method,
                                                       int limit) const {
   std::string uma_name(name_);
@@ skipping 60 matching lines @@
 
 class Thread : public ::base::PlatformThread::Delegate {
  public:
   Thread(void (*function)(void* arg), void* arg)
       : function_(function), arg_(arg) {
     ::base::PlatformThreadHandle handle;
     bool success = ::base::PlatformThread::Create(0, this, &handle);
     DCHECK(success);
   }
   virtual ~Thread() {}
-  virtual void ThreadMain() {
+  void ThreadMain() override {
     (*function_)(arg_);
     delete this;
   }
 
  private:
   void (*function_)(void* arg);
   void* arg_;
 };
 
 void ChromiumEnv::Schedule(ScheduleFunc* function, void* arg) {
@@ skipping 69 matching lines @@
 
 Env* IDBEnv() {
   return leveldb_env::idb_env.Pointer();
 }
 
 Env* Env::Default() {
   return leveldb_env::default_env.Pointer();
 }
 
 }  // namespace leveldb