Fix #3840: only SyncClosedLogs for multiple CFs (#4460)

Summary:
Call `SyncClosedLogs()` only if there are more than one column families.

Update several unit tests (in `fault_injection_test` and `db_flush_test`) correspondingly.

See #3840 for more info.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/4460

Differential Revision: D12896377

Pulled By: riversand963

fbshipit-source-id: f49afdaec32568f12f001219a3aec1dfde3b32bf

db/db_flush_test.cc

@@ -77,7 +77,7 @@ TEST_F(DBFlushTest, SyncFail) {
        {"DBImpl::SyncClosedLogs:Failed", "DBFlushTest::SyncFail:2"}});
   SyncPoint::GetInstance()->EnableProcessing();
 
-  Reopen(options);
+  CreateAndReopenWithCF({"pikachu"}, options);
   Put("key", "value");
   auto* cfd =
       reinterpret_cast<ColumnFamilyHandleImpl*>(db_->DefaultColumnFamily())
@@ -102,6 +102,30 @@ TEST_F(DBFlushTest, SyncFail) {
   ASSERT_EQ(refs_before, cfd->current()->TEST_refs());
   Destroy(options);
 }
+
+TEST_F(DBFlushTest, SyncSkip) {
+  Options options = CurrentOptions();
+
+  SyncPoint::GetInstance()->LoadDependency(
+      {{"DBFlushTest::SyncSkip:1", "DBImpl::SyncClosedLogs:Skip"},
+       {"DBImpl::SyncClosedLogs:Skip", "DBFlushTest::SyncSkip:2"}});
+  SyncPoint::GetInstance()->EnableProcessing();
+
+  Reopen(options);
+  Put("key", "value");
+
+  FlushOptions flush_options;
+  flush_options.wait = false;
+  ASSERT_OK(dbfull()->Flush(flush_options));
+
+  TEST_SYNC_POINT("DBFlushTest::SyncSkip:1");
+  TEST_SYNC_POINT("DBFlushTest::SyncSkip:2");
+
+  // Now the background job will do the flush; wait for it.
+  dbfull()->TEST_WaitForFlushMemTable();
+
+  Destroy(options);
+}
 #endif  // TRAVIS
 
 TEST_F(DBFlushTest, FlushInLowPriThreadPool) {

db/db_impl_compaction_flush.cc

@@ -145,7 +145,7 @@ Status DBImpl::FlushMemTableToOutputFile(
 
   Status s;
   if (logfile_number_ > 0 &&
-      versions_->GetColumnFamilySet()->NumberOfColumnFamilies() > 0) {
+      versions_->GetColumnFamilySet()->NumberOfColumnFamilies() > 1) {
     // If there are more than one column families, we need to make sure that
     // all the log files except the most recent one are synced. Otherwise if
     // the host crashes after flushing and before WAL is persistent, the
@@ -153,6 +153,8 @@ Status DBImpl::FlushMemTableToOutputFile(
     // other column families are missing.
     // SyncClosedLogs() may unlock and re-lock the db_mutex.
     s = SyncClosedLogs(job_context);
+  } else {
+    TEST_SYNC_POINT("DBImpl::SyncClosedLogs:Skip");
   }
 
   // Within flush_job.Run, rocksdb may call event listener to notify

db/db_impl_files.cc

@@ -465,7 +465,13 @@ void DBImpl::PurgeObsoleteFiles(JobContext& state, bool schedule_only) {
     } else {
       dir_to_sync =
           (type == kLogFile) ? immutable_db_options_.wal_dir : dbname_;
-      fname = dir_to_sync + "/" + to_delete;
+      fname = dir_to_sync
+            + (
+                (!dir_to_sync.empty() && dir_to_sync.back() == '/') ||
+                (!to_delete.empty() && to_delete.front() == '/')
+                ? "" : "/"
+              )
+            + to_delete;
     }
 
 #ifndef ROCKSDB_LITE

db/db_wal_test.cc

@@ -89,6 +89,7 @@ class DBWALTestWithEnrichedEnv : public DBTestBase {
     enriched_env_ = new EnrichedSpecialEnv(env_->target());
     auto options = CurrentOptions();
     options.env = enriched_env_;
+    options.allow_2pc = true;
     Reopen(options);
     delete env_;
     // to be deleted by the parent class