From 106a94af1552ba6eb89cfe08fd4f92c2078af463 Mon Sep 17 00:00:00 2001
From: Siying Dong <siying.d@fb.com>
Date: Thu, 28 Mar 2019 13:11:53 -0700
Subject: [PATCH] Improve obsolete_files_test (#5125)

Summary:
We see a failure of obsolete_files_test but aren't able to identify
the issue. Improve the test in following way and hope we can debug
better next time:
1. Place sync point before automatic compaction runs so race condition
   will always trigger.
2. Disable sync point before test finishes.
3. ASSERT_OK() instead of ASSERT_TRUE(status.ok())
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5125

Differential Revision: D14669456

Pulled By: siying

fbshipit-source-id: dccb7648e334501ad651eb212880096eef1f4ab2
---
 db/obsolete_files_test.cc | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/db/obsolete_files_test.cc b/db/obsolete_files_test.cc
index 05cadd6e8..52175a07b 100644
--- a/db/obsolete_files_test.cc
+++ b/db/obsolete_files_test.cc
@@ -158,9 +158,6 @@ class ObsoleteFilesTest : public testing::Test {
 };
 
 TEST_F(ObsoleteFilesTest, RaceForObsoleteFileDeletion) {
-  createLevel0Files(2, 50000);
-  CheckFileTypeCounts(options_.wal_dir, 1, 0, 0);
-
   SyncPoint::GetInstance()->LoadDependency({
       {"DBImpl::BackgroundCallCompaction:FoundObsoleteFiles",
        "ObsoleteFilesTest::RaceForObsoleteFileDeletion:1"},
@@ -170,7 +167,7 @@ TEST_F(ObsoleteFilesTest, RaceForObsoleteFileDeletion) {
   SyncPoint::GetInstance()->SetCallBack(
       "DBImpl::DeleteObsoleteFileImpl:AfterDeletion", [&](void* arg) {
         Status* p_status = reinterpret_cast<Status*>(arg);
-        ASSERT_TRUE(p_status->ok());
+        ASSERT_OK(*p_status);
       });
   SyncPoint::GetInstance()->SetCallBack(
       "DBImpl::CloseHelper:PendingPurgeFinished", [&](void* arg) {
@@ -180,6 +177,9 @@ TEST_F(ObsoleteFilesTest, RaceForObsoleteFileDeletion) {
       });
   SyncPoint::GetInstance()->EnableProcessing();
 
+  createLevel0Files(2, 50000);
+  CheckFileTypeCounts(options_.wal_dir, 1, 0, 0);
+
   DBImpl* dbi = reinterpret_cast<DBImpl*>(db_);
   port::Thread user_thread([&]() {
     JobContext jobCxt(0);
@@ -196,12 +196,10 @@ TEST_F(ObsoleteFilesTest, RaceForObsoleteFileDeletion) {
   user_thread.join();
 
   CloseDB();
+  SyncPoint::GetInstance()->DisableProcessing();
 }
 
 TEST_F(ObsoleteFilesTest, DeleteObsoleteOptionsFile) {
-  createLevel0Files(2, 50000);
-  CheckFileTypeCounts(options_.wal_dir, 1, 0, 0);
-
   std::vector<uint64_t> optsfiles_nums;
   std::vector<bool> optsfiles_keep;
   SyncPoint::GetInstance()->SetCallBack(
@@ -214,6 +212,9 @@ TEST_F(ObsoleteFilesTest, DeleteObsoleteOptionsFile) {
       });
   SyncPoint::GetInstance()->EnableProcessing();
 
+  createLevel0Files(2, 50000);
+  CheckFileTypeCounts(options_.wal_dir, 1, 0, 0);
+
   DBImpl* dbi = static_cast<DBImpl*>(db_);
   ASSERT_OK(dbi->DisableFileDeletions());
   for (int i = 0; i != 4; ++i) {
@@ -245,6 +246,7 @@ TEST_F(ObsoleteFilesTest, DeleteObsoleteOptionsFile) {
     }
   }
   ASSERT_EQ(2, opts_file_count);
+  SyncPoint::GetInstance()->DisableProcessing();
 }
 
 } //namespace rocksdb