Skip to content

Commit f37ce33

Browse files
archang19facebook-github-bot
authored andcommitted
Clean up secondary column families alongside primary column families (#13343)
Summary: This is a continuation of #13338, which aims to address crash test failures caused by #13281. This PR attempts to address the TSAN failures. I searched for wherever we call `column_families_.clear()` and made sure that we also clear the secondary column families as well. I made a helper method since it is easy to forget to clear both sets of column families. Pull Request resolved: #13343 Test Plan: Monitor recurring crash test results. Reviewed By: cbi42 Differential Revision: D68790580 Pulled By: archang19 fbshipit-source-id: 96ed758a21545dd20181b8db71b81dd660546e18
1 parent b8d915c commit f37ce33

File tree

2 files changed

+17
-20
lines changed

2 files changed

+17
-20
lines changed

db_stress_tool/db_stress_test_base.cc

+15-20
Original file line numberDiff line numberDiff line change
@@ -104,22 +104,26 @@ StressTest::StressTest()
104104
}
105105

106106
void StressTest::CleanUp() {
107-
for (auto cf : column_families_) {
108-
delete cf;
109-
}
110-
column_families_.clear();
107+
CleanUpColumnFamilies();
111108
if (db_) {
112109
db_->Close();
113110
}
114111
delete db_;
115112
db_ = nullptr;
116113

114+
delete secondary_db_;
115+
secondary_db_ = nullptr;
116+
}
117+
118+
void StressTest::CleanUpColumnFamilies() {
119+
for (auto cf : column_families_) {
120+
delete cf;
121+
}
122+
column_families_.clear();
117123
for (auto* cf : secondary_cfhs_) {
118124
delete cf;
119125
}
120126
secondary_cfhs_.clear();
121-
delete secondary_db_;
122-
secondary_db_ = nullptr;
123127
}
124128

125129
std::shared_ptr<Cache> StressTest::NewCache(size_t capacity,
@@ -735,10 +739,7 @@ void StressTest::PreloadDbAndReopenAsReadOnly(int64_t number_of_keys,
735739
s = db_->Flush(FlushOptions(), column_families_);
736740
}
737741
if (s.ok()) {
738-
for (auto cf : column_families_) {
739-
delete cf;
740-
}
741-
column_families_.clear();
742+
CleanUpColumnFamilies();
742743
delete db_;
743744
db_ = nullptr;
744745
txn_db_ = nullptr;
@@ -3581,10 +3582,7 @@ void StressTest::Open(SharedState* shared, bool reopen) {
35813582
// clean state before executing queries.
35823583
s = db_->GetRootDB()->WaitForCompact(WaitForCompactOptions());
35833584
if (!s.ok()) {
3584-
for (auto cf : column_families_) {
3585-
delete cf;
3586-
}
3587-
column_families_.clear();
3585+
CleanUpColumnFamilies();
35883586
delete db_;
35893587
db_ = nullptr;
35903588
delete secondary_db_;
@@ -3755,15 +3753,12 @@ void StressTest::Reopen(ThreadState* thread) {
37553753
}
37563754
assert(!write_prepared || bg_canceled);
37573755

3758-
for (auto cf : column_families_) {
3759-
delete cf;
3760-
}
3761-
column_families_.clear();
3756+
CleanUpColumnFamilies();
37623757

37633758
// Currently reopen does not restore expected state
37643759
// with potential data loss in mind like the first open before
3765-
// crash-recovery verification does. Therefore it always expects no data loss
3766-
// and we should ensure no data loss in testing.
3760+
// crash-recovery verification does. Therefore it always expects no data
3761+
// loss and we should ensure no data loss in testing.
37673762
// TODO(hx235): eliminate the FlushWAL(true /* sync */)/SyncWAL() below
37683763
if (!FLAGS_disable_wal) {
37693764
Status s;

db_stress_tool/db_stress_test_base.h

+2
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,8 @@ class StressTest {
391391
std::string& ts_str, Slice& ts_slice,
392392
ReadOptions& read_opts);
393393

394+
void CleanUpColumnFamilies();
395+
394396
std::shared_ptr<Cache> cache_;
395397
std::shared_ptr<Cache> compressed_cache_;
396398
std::shared_ptr<const FilterPolicy> filter_policy_;

0 commit comments

Comments
 (0)