From b337d733b66c4116c203a2f20511fb246137456b Mon Sep 17 00:00:00 2001 From: morningman Date: Mon, 2 Nov 2020 13:41:20 +0800 Subject: [PATCH 1/2] [Compaction][Buf] Fix bug that meta lock need to be held when calucating compaction score --- be/src/olap/base_compaction.cpp | 2 +- be/src/olap/cumulative_compaction.cpp | 2 +- be/src/olap/tablet.cpp | 10 ++++++---- be/src/olap/tablet.h | 5 +++-- be/src/olap/tablet_manager.cpp | 7 +------ be/test/olap/cumulative_compaction_policy_test.cpp | 6 +++--- 6 files changed, 15 insertions(+), 17 deletions(-) diff --git a/be/src/olap/base_compaction.cpp b/be/src/olap/base_compaction.cpp index d938c58aaeb313..03bb6eb106f06c 100644 --- a/be/src/olap/base_compaction.cpp +++ b/be/src/olap/base_compaction.cpp @@ -45,7 +45,7 @@ OLAPStatus BaseCompaction::compact() { TRACE_COUNTER_INCREMENT("input_rowsets_count", _input_rowsets.size()); // 2. do base compaction, merge rowsets - int64_t permits = _tablet->calc_base_compaction_score(); + int64_t permits = _tablet->calc_compaction_score(CompactionType::BASE_COMPACTION); RETURN_NOT_OK(do_compaction(permits)); TRACE("compaction finished"); diff --git a/be/src/olap/cumulative_compaction.cpp b/be/src/olap/cumulative_compaction.cpp index e89e6ed4aa165a..4b26e3fb2cac22 100755 --- a/be/src/olap/cumulative_compaction.cpp +++ b/be/src/olap/cumulative_compaction.cpp @@ -53,7 +53,7 @@ OLAPStatus CumulativeCompaction::compact() { TRACE_COUNTER_INCREMENT("input_rowsets_count", _input_rowsets.size()); // 3. do cumulative compaction, merge rowsets - int64_t permits = _tablet->calc_cumulative_compaction_score(); + int64_t permits = _tablet->calc_compaction_score(CompactionType::CUMULATIVE_COMPACTION); RETURN_NOT_OK(do_compaction(permits)); TRACE("compaction finished"); diff --git a/be/src/olap/tablet.cpp b/be/src/olap/tablet.cpp index 2b7ac865ff11a1..9ad27396c94c51 100644 --- a/be/src/olap/tablet.cpp +++ b/be/src/olap/tablet.cpp @@ -751,22 +751,24 @@ bool Tablet::can_do_compaction() { } const uint32_t Tablet::calc_compaction_score(CompactionType compaction_type) const { + // Need meta lock, because it will iterator "all_rs_metas" of tablet meta. + ReadLock rdlock(&_meta_lock); if (compaction_type == CompactionType::CUMULATIVE_COMPACTION) { - return calc_cumulative_compaction_score(); + return _calc_cumulative_compaction_score(); } else { DCHECK_EQ(compaction_type, CompactionType::BASE_COMPACTION); - return calc_base_compaction_score(); + return _calc_base_compaction_score(); } } -const uint32_t Tablet::calc_cumulative_compaction_score() const { +const uint32_t Tablet::_calc_cumulative_compaction_score() const { uint32_t score = 0; _cumulative_compaction_policy->calc_cumulative_compaction_score( _tablet_meta->all_rs_metas(), cumulative_layer_point(), &score); return score; } -const uint32_t Tablet::calc_base_compaction_score() const { +const uint32_t Tablet::_calc_base_compaction_score() const { uint32_t score = 0; const int64_t point = cumulative_layer_point(); bool base_rowset_exist = false; diff --git a/be/src/olap/tablet.h b/be/src/olap/tablet.h index 07863824702d08..d6cfabcf811d5b 100644 --- a/be/src/olap/tablet.h +++ b/be/src/olap/tablet.h @@ -164,8 +164,6 @@ class Tablet : public BaseTablet { // operation for compaction bool can_do_compaction(); const uint32_t calc_compaction_score(CompactionType compaction_type) const; - const uint32_t calc_cumulative_compaction_score() const; - const uint32_t calc_base_compaction_score() const; static void compute_version_hash_from_rowsets(const std::vector& rowsets, VersionHash* version_hash); @@ -249,6 +247,9 @@ class Tablet : public BaseTablet { OLAPStatus _capture_consistent_rowsets_unlocked(const vector& version_path, vector* rowsets) const; + const uint32_t _calc_cumulative_compaction_score() const; + const uint32_t _calc_base_compaction_score() const; + public: static const int64_t K_INVALID_CUMULATIVE_POINT = -1; diff --git a/be/src/olap/tablet_manager.cpp b/be/src/olap/tablet_manager.cpp index 29f53be487739e..21901eb1490f3f 100644 --- a/be/src/olap/tablet_manager.cpp +++ b/be/src/olap/tablet_manager.cpp @@ -747,12 +747,7 @@ TabletSharedPtr TabletManager::find_best_tablet_to_compaction( uint32_t table_score = 0; { - ReadLock rdlock(tablet_ptr->get_header_lock_ptr()); - if (compaction_type == CompactionType::BASE_COMPACTION) { - table_score = tablet_ptr->calc_base_compaction_score(); - } else if (compaction_type == CompactionType::CUMULATIVE_COMPACTION) { - table_score = tablet_ptr->calc_cumulative_compaction_score(); - } + table_score = tablet_ptr->calc_compaction_score(compaction_type); } if (table_score > highest_score) { highest_score = table_score; diff --git a/be/test/olap/cumulative_compaction_policy_test.cpp b/be/test/olap/cumulative_compaction_policy_test.cpp index 4d5d0d19c6757b..e3310563f36a27 100644 --- a/be/test/olap/cumulative_compaction_policy_test.cpp +++ b/be/test/olap/cumulative_compaction_policy_test.cpp @@ -212,7 +212,7 @@ TEST_F(TestNumBasedCumulativeCompactionPolicy, calc_cumulative_compaction_score) TabletSharedPtr _tablet(new Tablet(_tablet_meta, nullptr, CUMULATIVE_NUM_BASED_POLICY)); _tablet->init(); - const uint32_t score = _tablet->calc_cumulative_compaction_score(); + const uint32_t score = _tablet->calc_compaction_score(CompactionType::CUMULATIVE_COMPACTION); ASSERT_EQ(15, score); } @@ -675,7 +675,7 @@ TEST_F(TestSizeBasedCumulativeCompactionPolicy, calc_cumulative_compaction_score _tablet->init(); _tablet->calculate_cumulative_point(); - const uint32_t score = _tablet->calc_cumulative_compaction_score(); + const uint32_t score = _tablet->calc_compaction_score(CompactionType::CUMULATIVE_COMPACTION); ASSERT_EQ(15, score); } @@ -692,7 +692,7 @@ TEST_F(TestSizeBasedCumulativeCompactionPolicy, calc_cumulative_compaction_score TabletSharedPtr _tablet(new Tablet(_tablet_meta, nullptr, CUMULATIVE_SIZE_BASED_POLICY)); _tablet->init(); _tablet->calculate_cumulative_point(); - const uint32_t score = _tablet->calc_cumulative_compaction_score(); + const uint32_t score = _tablet->calc_compaction_score(CompactionType::CUMULATIVE_COMPACTION); ASSERT_EQ(7, score); } From 57d87d48d87a78266f8863a6ab2023d916041675 Mon Sep 17 00:00:00 2001 From: morningman Date: Mon, 2 Nov 2020 13:49:34 +0800 Subject: [PATCH 2/2] fix --- be/src/olap/tablet_manager.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/be/src/olap/tablet_manager.cpp b/be/src/olap/tablet_manager.cpp index 21901eb1490f3f..72ce994f58ea1e 100644 --- a/be/src/olap/tablet_manager.cpp +++ b/be/src/olap/tablet_manager.cpp @@ -745,10 +745,7 @@ TabletSharedPtr TabletManager::find_best_tablet_to_compaction( } } - uint32_t table_score = 0; - { - table_score = tablet_ptr->calc_compaction_score(compaction_type); - } + uint32_t table_score = tablet_ptr->calc_compaction_score(compaction_type); if (table_score > highest_score) { highest_score = table_score; best_tablet = tablet_ptr;