Simplify TEST_F(DBWALTest, FixSyncWalOnObseletedWalWithNewManifestCausingMissingWAL) (#11186)

Summary:
**Context/Summary**:
Simplify `TEST_F(DBWALTest, FixSyncWalOnObseletedWalWithNewManifestCausingMissingWAL)` based on https://github.com/facebook/rocksdb/pull/11016#pullrequestreview-1205020134 and delete unused sync points.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/11186

Test Plan:
- UT failed before fix in https://github.com/facebook/rocksdb/pull/10892 and passes after
- Check UT not flaky when running with https://app.circleci.com/pipelines/github/facebook/rocksdb/21985/workflows/5f6cc355-78c1-46d8-89ee-0fd679725a8a/jobs/540878

Reviewed By: ajkr

Differential Revision: D43034723

Pulled By: hx235

fbshipit-source-id: f503774987b8f3718505f99e95080a7fad28ac66

db/db_impl/db_impl_files.cc

@@ -361,8 +361,6 @@ void DBImpl::DeleteObsoleteFileImpl(int job_id, const std::string& fname,
   }
   TEST_SYNC_POINT_CALLBACK("DBImpl::DeleteObsoleteFileImpl:AfterDeletion",
                            &file_deletion_status);
-  TEST_SYNC_POINT_CALLBACK("DBImpl::DeleteObsoleteFileImpl:AfterDeletion2",
-                           const_cast<std::string*>(&fname));
   if (file_deletion_status.ok()) {
     ROCKS_LOG_DEBUG(immutable_db_options_.info_log,
                     "[JOB %d] Delete %s type=%d #%" PRIu64 " -- %s\n", job_id,

db/db_wal_test.cc

@@ -1633,6 +1633,8 @@ TEST_F(DBWALTest, RaceInstallFlushResultsWithWalObsoletion) {
 
 TEST_F(DBWALTest, FixSyncWalOnObseletedWalWithNewManifestCausingMissingWAL) {
   Options options = CurrentOptions();
+  // Small size to force manifest creation
+  options.max_manifest_file_size = 1;
   options.track_and_verify_wals_in_manifest = true;
   DestroyAndReopen(options);
 
@@ -1649,53 +1651,33 @@ TEST_F(DBWALTest, FixSyncWalOnObseletedWalWithNewManifestCausingMissingWAL) {
   // (2) SyncWAL() proceeds with the lock. It
   // creates a new manifest and syncs all the inactive wals before the latest
   // (i.e, active log), which is 4.log. Note that SyncWAL() is not aware of the
-  // fact that 4.log has marked as to be obseleted. Prior to the fix, such wal
+  // fact that 4.log has marked as to be obseleted. Such wal
   // sync will then add a WAL addition record of 4.log to the new manifest
-  // without any special treatment.
-  // (3) BackgroundFlush() will eventually purge 4.log.
+  // without any special treatment. Prior to the fix, there is no WAL deletion
+  // record to offset it. (3) BackgroundFlush() will eventually purge 4.log.
+
   bool wal_synced = false;
   SyncPoint::GetInstance()->SetCallBack(
       "FindObsoleteFiles::PostMutexUnlock", [&](void*) {
         ASSERT_OK(env_->FileExists(wal_file_path));
-
-        SyncPoint::GetInstance()->SetCallBack(
-            "VersionSet::ProcessManifestWrites:"
-            "PostDecidingCreateNewManifestOrNot",
-            [&](void* arg) {
-              bool* new_descriptor_log = (bool*)arg;
-              *new_descriptor_log = true;
-            });
-
+        uint64_t pre_sync_wal_manifest_no =
+            dbfull()->TEST_Current_Manifest_FileNo();
         ASSERT_OK(db_->SyncWAL());
+        uint64_t post_sync_wal_manifest_no =
+            dbfull()->TEST_Current_Manifest_FileNo();
+        bool new_manifest_created =
+            post_sync_wal_manifest_no == pre_sync_wal_manifest_no + 1;
+        ASSERT_TRUE(new_manifest_created);
         wal_synced = true;
       });
 
-  SyncPoint::GetInstance()->SetCallBack(
-      "DBImpl::DeleteObsoleteFileImpl:AfterDeletion2", [&](void* arg) {
-        std::string* file_name = (std::string*)arg;
-        if (*file_name == wal_file_path) {
-          TEST_SYNC_POINT(
-              "DBWALTest::"
-              "FixSyncWalOnObseletedWalWithNewManifestCausingMissingWAL::"
-              "PostDeleteWAL");
-        }
-      });
-
-  SyncPoint::GetInstance()->LoadDependency(
-      {{"DBImpl::BackgroundCallFlush:FilesFound",
-        "PreConfrimObsoletedWALSynced"},
-       {"DBWALTest::FixSyncWalOnObseletedWalWithNewManifestCausingMissingWAL::"
-        "PostDeleteWAL",
-        "PreConfrimWALDeleted"}});
 
   SyncPoint::GetInstance()->EnableProcessing();
 
   ASSERT_OK(Flush());
+  ASSERT_OK(dbfull()->TEST_WaitForBackgroundWork());
 
-  TEST_SYNC_POINT("PreConfrimObsoletedWALSynced");
   ASSERT_TRUE(wal_synced);
-
-  TEST_SYNC_POINT("PreConfrimWALDeleted");
   // BackgroundFlush() purged 4.log
   // because the memtable associated with the WAL was flushed and new WAL was
   // created (i.e, 8.log)

db/version_set.cc

@@ -5077,15 +5077,10 @@ Status VersionSet::ProcessManifestWrites(
   if (!descriptor_log_ ||
       manifest_file_size_ > db_options_->max_manifest_file_size) {
     TEST_SYNC_POINT("VersionSet::ProcessManifestWrites:BeforeNewManifest");
-    TEST_SYNC_POINT_CALLBACK(
-        "VersionSet::ProcessManifestWrites:BeforeNewManifest", nullptr);
     new_descriptor_log = true;
   } else {
     pending_manifest_file_number_ = manifest_file_number_;
   }
-  TEST_SYNC_POINT_CALLBACK(
-      "VersionSet::ProcessManifestWrites:PostDecidingCreateNewManifestOrNot",
-      &new_descriptor_log);
 
   // Local cached copy of state variable(s). WriteCurrentStateToManifest()
   // reads its content after releasing db mutex to avoid race with