From 90ca39faeb224a98f4bbb8849a545e0e8329d327 Mon Sep 17 00:00:00 2001 From: Yanqin Jin Date: Sat, 5 Dec 2020 23:55:01 -0800 Subject: [PATCH 1/4] Refactor ProcessManifestWrites a little bit Summary: This PR removes a nested loop inside ProcessManifestWrites. The new implementation has the same behavior as the old code with simpler logic and lower complexity. Test Plan: make check Run make crash_test on devserver and succeeds 3 times. --- db/version_set.cc | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/db/version_set.cc b/db/version_set.cc index 2f5348cfb91..c89da7d6e66 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -4229,31 +4229,21 @@ Status VersionSet::ProcessManifestWrites( // Each version in versions corresponds to a column family. // For each column family, update its log number indicating that logs // with number smaller than this should be ignored. - // TODO (yanqin): remove the nested loop if possible. - for (const auto version : versions) { - uint64_t max_log_number_in_batch = 0; - assert(version->cfd_); - uint32_t cf_id = version->cfd_->GetID(); - std::string full_history_ts_low; - for (const auto& e : batch_edits) { - if (e->column_family_ == cf_id) { - if (e->has_log_number_) { - max_log_number_in_batch = - std::max(max_log_number_in_batch, e->log_number_); - } - if (e->HasFullHistoryTsLow()) { - version->cfd_->SetFullHistoryTsLow(e->GetFullHistoryTsLow()); - } - } + uint64_t last_min_log_number_to_keep = 0; + for (const auto& e : batch_edits) { + ColumnFamilyData* cfd = nullptr; + if (!e->IsColumnFamilyManipulation()) { + cfd = column_family_set_->GetColumnFamily(e->column_family_); + assert(cfd); } - if (max_log_number_in_batch != 0) { - assert(version->cfd_->GetLogNumber() <= max_log_number_in_batch); - version->cfd_->SetLogNumber(max_log_number_in_batch); + if (cfd) { + if (e->has_log_number_ && e->log_number_ > cfd->GetLogNumber()) { + cfd->SetLogNumber(e->log_number_); + } + if (e->HasFullHistoryTsLow()) { + cfd->SetFullHistoryTsLow(e->GetFullHistoryTsLow()); + } } - } - - uint64_t last_min_log_number_to_keep = 0; - for (auto& e : batch_edits) { if (e->has_min_log_number_to_keep_) { last_min_log_number_to_keep = std::max(last_min_log_number_to_keep, e->min_log_number_to_keep_); From 206821d33834cbdaa9ff1b397d76c1726b269a32 Mon Sep 17 00:00:00 2001 From: Yanqin Jin Date: Mon, 7 Dec 2020 18:57:47 -0800 Subject: [PATCH 2/4] Remove an assertion --- db/version_set.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/db/version_set.cc b/db/version_set.cc index c89da7d6e66..299503034bc 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -4234,7 +4234,6 @@ Status VersionSet::ProcessManifestWrites( ColumnFamilyData* cfd = nullptr; if (!e->IsColumnFamilyManipulation()) { cfd = column_family_set_->GetColumnFamily(e->column_family_); - assert(cfd); } if (cfd) { if (e->has_log_number_ && e->log_number_ > cfd->GetLogNumber()) { From e9f5c578fee44ea1cc47b175a698f55ae1c95fa9 Mon Sep 17 00:00:00 2001 From: Yanqin Jin Date: Mon, 7 Dec 2020 19:00:59 -0800 Subject: [PATCH 3/4] Update assertion --- db/version_set.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/db/version_set.cc b/db/version_set.cc index 299503034bc..9d2db74fcdf 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -4234,6 +4234,7 @@ Status VersionSet::ProcessManifestWrites( ColumnFamilyData* cfd = nullptr; if (!e->IsColumnFamilyManipulation()) { cfd = column_family_set_->GetColumnFamily(e->column_family_); + assert(!cfd || !cfd->IsDropped()); } if (cfd) { if (e->has_log_number_ && e->log_number_ > cfd->GetLogNumber()) { From 131da910b42b7cd2fc738af9040161119359b15e Mon Sep 17 00:00:00 2001 From: Yanqin Jin Date: Mon, 7 Dec 2020 19:25:20 -0800 Subject: [PATCH 4/4] Add comments --- db/version_set.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/db/version_set.cc b/db/version_set.cc index 9d2db74fcdf..c77a85e5988 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -4234,7 +4234,9 @@ Status VersionSet::ProcessManifestWrites( ColumnFamilyData* cfd = nullptr; if (!e->IsColumnFamilyManipulation()) { cfd = column_family_set_->GetColumnFamily(e->column_family_); - assert(!cfd || !cfd->IsDropped()); + // e would not have been added to batch_edits if its corresponding + // column family is dropped. + assert(cfd); } if (cfd) { if (e->has_log_number_ && e->log_number_ > cfd->GetLogNumber()) {