Skip to content

Commit

Permalink
Fix race in multiops txn stress test (#12847)
Browse files Browse the repository at this point in the history
Summary:
`MultiOpsTxnsStressListener::OnCompactionCompleted()` access `db_` and can be called while db_ is being destroyed in ~StressTest(). This causes TSAN to complain about data race. This PR fixes this issue by calling db_->Close() first to stop all background work. Also moved the cleanup out of StressTest destructor to avoid race between the listener and  ~StressTest().

Pull Request resolved: #12847

Test Plan: monitor crash test failure.

Reviewed By: hx235

Differential Revision: D59492691

Pulled By: cbi42

fbshipit-source-id: afcbab084cc9ac0904d6b04809b0888498ca8e66
  • Loading branch information
cbi42 authored and facebook-github-bot committed Jul 9, 2024
1 parent ebe2116 commit d6f265f
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 7 deletions.
6 changes: 5 additions & 1 deletion db_stress_tool/db_stress_test_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,16 @@ StressTest::StressTest()
}
}

StressTest::~StressTest() {
void StressTest::CleanUp() {
for (auto cf : column_families_) {
delete cf;
}
column_families_.clear();
if (db_) {
db_->Close();
}
delete db_;
db_ = nullptr;

for (auto* cf : cmp_cfhs_) {
delete cf;
Expand Down
4 changes: 3 additions & 1 deletion db_stress_tool/db_stress_test_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class StressTest {
public:
StressTest();

virtual ~StressTest();
virtual ~StressTest() {}

std::shared_ptr<Cache> NewCache(size_t capacity, int32_t num_shard_bits);

Expand All @@ -49,6 +49,8 @@ class StressTest {
FLAGS_manual_wal_flush_one_in > 0;
}

void CleanUp();

protected:
static int GetMinInjectedErrorCount(int error_count_1, int error_count_2) {
if (error_count_1 > 0 && error_count_2 > 0) {
Expand Down
10 changes: 5 additions & 5 deletions db_stress_tool/db_stress_tool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -362,11 +362,11 @@ int db_stress_tool(int argc, char** argv) {
// Initialize the Zipfian pre-calculated array
InitializeHotKeyGenerator(FLAGS_hot_key_alpha);
shared.reset(new SharedState(db_stress_env, stress.get()));
if (RunStressTest(shared.get())) {
return 0;
} else {
return 1;
}
bool run_stress_test = RunStressTest(shared.get());
// Close DB in CleanUp() before destructor to prevent race between destructor
// and operations in listener callbacks (e.g. MultiOpsTxnsStressListener).
stress->CleanUp();
return run_stress_test ? 0 : 1;
}

} // namespace ROCKSDB_NAMESPACE
Expand Down

0 comments on commit d6f265f

Please sign in to comment.