From f25f1ec60b1bd1de48a4db42f7ceda660fb419bb Mon Sep 17 00:00:00 2001 From: Siying Dong Date: Thu, 26 Jan 2017 16:25:19 -0800 Subject: [PATCH] Add test DBTest2.GetRaceFlush which can expose a data race bug Summary: A current data race issue in Get() and Flush() can cause a Get() to return wrong results when a flush happened in the middle. Disable the test for now. Closes https://github.com/facebook/rocksdb/pull/1813 Differential Revision: D4472310 Pulled By: siying fbshipit-source-id: 5755ebd --- db/db_impl.cc | 3 +++ db/db_test2.cc | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/db/db_impl.cc b/db/db_impl.cc index b68694721b..cbe052a952 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -4007,6 +4007,9 @@ Status DBImpl::GetImpl(const ReadOptions& read_options, } else { snapshot = versions_->LastSequence(); } + + TEST_SYNC_POINT("DBImpl::GetImpl:1"); + TEST_SYNC_POINT("DBImpl::GetImpl:2"); // Acquire SuperVersion SuperVersion* sv = GetAndRefSuperVersion(cfd); // Prepare to store a list of merge operations if merge occurs. diff --git a/db/db_test2.cc b/db/db_test2.cc index ccbed04979..313270184f 100644 --- a/db/db_test2.cc +++ b/db/db_test2.cc @@ -2228,6 +2228,29 @@ TEST_F(DBTest2, OptimizeForPointLookup) { Flush(); ASSERT_EQ("v1", Get("foo")); } + +// Disable the test before we fix the bug +TEST_F(DBTest2, DISABLED_GetRaceFlush) { + ASSERT_OK(Put("foo", "v1")); + + rocksdb::SyncPoint::GetInstance()->LoadDependency( + {{"DBImpl::GetImpl:1", "DBTest2::GetRaceFlush:1"}, + {"DBTest2::GetRaceFlush:2", "DBImpl::GetImpl:2"}}); + + rocksdb::SyncPoint::GetInstance()->EnableProcessing(); + + std::thread threads([&] { + TEST_SYNC_POINT("DBTest2::GetRaceFlush:1"); + ASSERT_OK(Put("foo", "v2")); + Flush(); + TEST_SYNC_POINT("DBTest2::GetRaceFlush:2"); + }); + + // Get() is issued after the first Put(), so it should see either + // "v1" or "v2". + ASSERT_NE("NOT_FOUND", Get("foo")); + rocksdb::SyncPoint::GetInstance()->DisableProcessing(); +} #endif // ROCKSDB_LITE } // namespace rocksdb