leveldb: Env no longer implicitly syncs dir for all newly created files.

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_POSIX)
 #include <dirent.h>
 #include <sys/types.h>
 #endif
@@ skipping 198 matching lines @@
   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) {
   FilePath path = FilePath::FromUTF8Unsafe(fname);
   if (path.BaseName().AsUTF8Unsafe().find("MANIFEST") == 0)
     file_type_ = kManifest;
   else if (path.MatchesExtension(table_extension))
     file_type_ = kTable;
-  if (file_type_ != kManifest)
-    tracker_->DidCreateNewFile(filename_);
   parent_dir_ = FilePath::FromUTF8Unsafe(fname).DirName().AsUTF8Unsafe();
 }
 
 Status ChromiumWritableFile::SyncParent() {
   TRACE_EVENT0("leveldb", "SyncParent");
 #if defined(OS_POSIX)
   FilePath path = 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();
 }
@@ skipping 15 matching lines @@
   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();
+  // leveldb's implicit contract for Sync() is that if this instance is for a
+  // manifest file then the directory is also sync'ed. See leveldb's
+  // env_posix.cc.
+  if (file_type_ == kManifest)
+    return SyncParent();
+  else

[jsbell, 2015/03/23 22:42:46]

Nit: Remove else since there's an early return in the if clause.

+    return Status::OK();
 }
 
 class IDBEnv : public ChromiumEnv {
  public:
   IDBEnv() : ChromiumEnv() {
     name_ = "LevelDBEnv.IDB";
     make_backup_ = true;
   }
 };
 
@@ skipping 548 matching lines @@
                                     leveldb::WritableFile** result) {
   *result = NULL;
   FilePath path = FilePath::FromUTF8Unsafe(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_);
+    *result = new ChromiumWritableFile(fname, f.release(), this, make_backup_);
     return Status::OK();
   }
 }
 
 Status ChromiumEnv::NewAppendableFile(const std::string& fname,
                                       leveldb::WritableFile** result) {
   *result = NULL;
   FilePath path = FilePath::FromUTF8Unsafe(fname);
   scoped_ptr<base::File> f(new base::File(
       path, base::File::FLAG_OPEN_ALWAYS | base::File::FLAG_APPEND));
   if (!f->IsValid()) {
     RecordErrorAt(kNewAppendableFile);
     return MakeIOError(fname, "Unable to create appendable file",
                        kNewAppendableFile, f->error_details());
   }
-  *result =
-      new ChromiumWritableFile(fname, f.release(), this, this, make_backup_);
+  *result = new ChromiumWritableFile(fname, f.release(), 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));
@@ skipping 144 matching lines @@
     mu_.Release();
     TRACE_EVENT0("leveldb", "ChromiumEnv::BGThread-Task");
     (*function)(arg);
   }
 }
 
 void ChromiumEnv::StartThread(void (*function)(void* arg), void* arg) {
   new Thread(function, arg);  // Will self-delete.
 }
 
-static std::string GetDirName(const std::string& filename) {
-  return FilePath::FromUTF8Unsafe(filename).DirName().AsUTF8Unsafe();
-}
-
-void ChromiumEnv::DidCreateNewFile(const std::string& filename) {
-  base::AutoLock auto_lock(directory_sync_lock_);
-  directories_needing_sync_.insert(GetDirName(filename));
-}
-
-bool ChromiumEnv::DoesDirNeedSync(const std::string& filename) {
-  base::AutoLock auto_lock(directory_sync_lock_);
-  return ContainsKey(directories_needing_sync_, GetDirName(filename));
-}
-
-void ChromiumEnv::DidSyncDir(const std::string& filename) {
-  base::AutoLock auto_lock(directory_sync_lock_);
-  directories_needing_sync_.erase(GetDirName(filename));
-}
-
 }  // namespace leveldb_env
 
 namespace leveldb {
 
 Env* IDBEnv() {
   return leveldb_env::idb_env.Pointer();
 }
 
 Env* Env::Default() {
   return leveldb_env::default_env.Pointer();
 }
 
 }  // namespace leveldb