Make a copy of MutableCFOptions to avoid race condition (#4876)
Summary: If we do not do this, then reading MutableCFOptions may have a race condition with SetOptions which modifies MutableCFOptions. Also reserve space in advance for vectors to avoid reallocation changing the address of its elements. Test plan ``` $make clean && make -j32 all check $make clean && COMPILE_WITH_TSAN=1 make -j32 all check $make clean && COMPILE_WITH_ASAN=1 make -j32 all check ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/4876 Differential Revision: D13644500 Pulled By: riversand963 fbshipit-source-id: 4b8112c5c819d5a2922bb61ad1521b3d2fb2fd47
This commit is contained in:
parent
53f760b8a8
commit
acba14b3d9
@ -222,8 +222,7 @@ Status DBImpl::FlushMemTablesToOutputFiles(
|
|||||||
Status status;
|
Status status;
|
||||||
for (auto& arg : bg_flush_args) {
|
for (auto& arg : bg_flush_args) {
|
||||||
ColumnFamilyData* cfd = arg.cfd_;
|
ColumnFamilyData* cfd = arg.cfd_;
|
||||||
const MutableCFOptions& mutable_cf_options =
|
MutableCFOptions mutable_cf_options = *cfd->GetLatestMutableCFOptions();
|
||||||
*cfd->GetLatestMutableCFOptions();
|
|
||||||
SuperVersionContext* superversion_context = arg.superversion_context_;
|
SuperVersionContext* superversion_context = arg.superversion_context_;
|
||||||
Status s = FlushMemTableToOutputFile(cfd, mutable_cf_options, made_progress,
|
Status s = FlushMemTableToOutputFile(cfd, mutable_cf_options, made_progress,
|
||||||
job_context, superversion_context,
|
job_context, superversion_context,
|
||||||
@ -276,7 +275,9 @@ Status DBImpl::AtomicFlushMemTablesToOutputFiles(
|
|||||||
}
|
}
|
||||||
autovector<Directory*> distinct_output_dirs;
|
autovector<Directory*> distinct_output_dirs;
|
||||||
std::vector<FlushJob> jobs;
|
std::vector<FlushJob> jobs;
|
||||||
|
std::vector<MutableCFOptions> all_mutable_cf_options;
|
||||||
int num_cfs = static_cast<int>(cfds.size());
|
int num_cfs = static_cast<int>(cfds.size());
|
||||||
|
all_mutable_cf_options.reserve(num_cfs);
|
||||||
for (int i = 0; i < num_cfs; ++i) {
|
for (int i = 0; i < num_cfs; ++i) {
|
||||||
auto cfd = cfds[i];
|
auto cfd = cfds[i];
|
||||||
Directory* data_dir = GetDataDir(cfd, 0U);
|
Directory* data_dir = GetDataDir(cfd, 0U);
|
||||||
@ -295,8 +296,8 @@ Status DBImpl::AtomicFlushMemTablesToOutputFiles(
|
|||||||
distinct_output_dirs.emplace_back(data_dir);
|
distinct_output_dirs.emplace_back(data_dir);
|
||||||
}
|
}
|
||||||
|
|
||||||
const MutableCFOptions& mutable_cf_options =
|
all_mutable_cf_options.emplace_back(*cfd->GetLatestMutableCFOptions());
|
||||||
*cfd->GetLatestMutableCFOptions();
|
const MutableCFOptions& mutable_cf_options = all_mutable_cf_options.back();
|
||||||
const uint64_t* max_memtable_id = &(bg_flush_args[i].max_memtable_id_);
|
const uint64_t* max_memtable_id = &(bg_flush_args[i].max_memtable_id_);
|
||||||
jobs.emplace_back(
|
jobs.emplace_back(
|
||||||
dbname_, cfds[i], immutable_db_options_, mutable_cf_options,
|
dbname_, cfds[i], immutable_db_options_, mutable_cf_options,
|
||||||
@ -432,8 +433,7 @@ Status DBImpl::AtomicFlushMemTablesToOutputFiles(
|
|||||||
if (!cfds[i]->IsDropped() && !mems.empty()) {
|
if (!cfds[i]->IsDropped() && !mems.empty()) {
|
||||||
tmp_cfds.emplace_back(cfds[i]);
|
tmp_cfds.emplace_back(cfds[i]);
|
||||||
mems_list.emplace_back(&mems);
|
mems_list.emplace_back(&mems);
|
||||||
mutable_cf_options_list.emplace_back(
|
mutable_cf_options_list.emplace_back(&all_mutable_cf_options[i]);
|
||||||
cfds[i]->GetLatestMutableCFOptions());
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -1460,6 +1460,7 @@ Status DBImpl::RunManualCompaction(ColumnFamilyData* cfd, int input_level,
|
|||||||
void DBImpl::GenerateFlushRequest(const autovector<ColumnFamilyData*>& cfds,
|
void DBImpl::GenerateFlushRequest(const autovector<ColumnFamilyData*>& cfds,
|
||||||
FlushRequest* req) {
|
FlushRequest* req) {
|
||||||
assert(req != nullptr);
|
assert(req != nullptr);
|
||||||
|
req->reserve(cfds.size());
|
||||||
for (const auto cfd : cfds) {
|
for (const auto cfd : cfds) {
|
||||||
if (nullptr == cfd) {
|
if (nullptr == cfd) {
|
||||||
// cfd may be null, see DBImpl::ScheduleFlushes
|
// cfd may be null, see DBImpl::ScheduleFlushes
|
||||||
|
Loading…
Reference in New Issue
Block a user