From 94b4faa0f1eafdadd76d6ce5ed98f42a05770216 Mon Sep 17 00:00:00 2001 From: Jay Zhuang Date: Thu, 20 May 2021 09:28:58 -0700 Subject: [PATCH] Deflake ExternalSSTFileTest.PickedLevelBug (#8307) Summary: The test want to make sure these's no compaction during `AddFile` (between `DBImpl::AddFile:MutexLock` and `DBImpl::AddFile:MutexUnlock`) but the mutex could be unlocked by `EnterUnbatched()`. Move the lock start point after bumping the ingest file number. Also fix the dead lock when ASSERT fails. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8307 Reviewed By: ajkr Differential Revision: D28479849 Pulled By: jay-zhuang fbshipit-source-id: b3c50f66aa5d5f59c5c27f815bfea189c4cd06cb --- db/external_sst_file_test.cc | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/db/external_sst_file_test.cc b/db/external_sst_file_test.cc index 552730295..afb8ba459 100644 --- a/db/external_sst_file_test.cc +++ b/db/external_sst_file_test.cc @@ -1291,8 +1291,11 @@ TEST_F(ExternalSSTFileTest, PickedLevelBug) { // We have 2 overlapping files in L0 EXPECT_EQ(FilesPerLevel(), "2"); + ASSERT_OK(dbfull()->TEST_WaitForCompact()); + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependency( - {{"DBImpl::AddFile:MutexLock", "ExternalSSTFileTest::PickedLevelBug:0"}, + {{"DBImpl::IngestExternalFile:AfterIncIngestFileCounter", + "ExternalSSTFileTest::PickedLevelBug:0"}, {"ExternalSSTFileTest::PickedLevelBug:1", "DBImpl::AddFile:MutexUnlock"}, {"ExternalSSTFileTest::PickedLevelBug:2", "DBImpl::RunManualCompaction:0"}, @@ -1333,10 +1336,14 @@ TEST_F(ExternalSSTFileTest, PickedLevelBug) { // wait for 2 seconds to give a chance for compactions to run during // this period, and then make sure that no compactions where able to run env_->SleepForMicroseconds(1000000 * 2); - ASSERT_FALSE(bg_compact_started.load()); + bool bg_compact_started_tmp = bg_compact_started.load(); // Hold AddFile from finishing writing the MANIFEST TEST_SYNC_POINT("ExternalSSTFileTest::PickedLevelBug:1"); + + // check the status at the end, so even if the ASSERT fails the threads + // could be joined and return. + ASSERT_FALSE(bg_compact_started_tmp); } ASSERT_OK(bg_addfile_status);