[Enhancement] Avoid multiple lock acquire in the same stack for table…

…t report ## Why I'm doing: In current impl, the longest lock acquire chain in tablet report will be looks like: tablet shard lock -> tablet meta lock -> pk relative lock which may make other thread stuck easliy because too many lock acquire at the same time. ## What I'm doing: Remove the lock dependency. Signed-off-by: srlch <[email protected]>
StarRocks · Feb 11, 2025 · 85890ad · 85890ad
1 parent d925914
commit 85890ad
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 20 deletions.
diff --git a/be/src/storage/tablet.cpp b/be/src/storage/tablet.cpp
@@ -1534,6 +1534,13 @@ bool Tablet::_contains_rowset(const RowsetId rowset_id) {
 }
 
 void Tablet::build_tablet_report_info(TTabletInfo* tablet_info) {
+    // primary key specified info will protected by independent lock context
+    // in TabletUpdates which means that those lock context can be sperated from
+    // the tablet meta_lock when trying to get the extra info
+    if (_updates) {
+        _updates->get_tablet_info_extra(tablet_info);
+    }
+
     std::shared_lock rdlock(_meta_lock);
     tablet_info->__set_tablet_id(_tablet_meta->tablet_id());
     tablet_info->__set_schema_hash(_tablet_meta->schema_hash());
@@ -1546,9 +1553,7 @@ void Tablet::build_tablet_report_info(TTabletInfo* tablet_info) {
     if (_tablet_meta->get_binlog_config() != nullptr) {
         tablet_info->__set_binlog_config_version(_tablet_meta->get_binlog_config()->version);
     }
-    if (_updates) {
-        _updates->get_tablet_info_extra(tablet_info);
-    } else {
+    if (_updates == nullptr) {
         int64_t max_version = _timestamped_version_tracker.get_max_continuous_version();
         auto max_rowset = rowset_with_max_version();
         if (max_rowset != nullptr) {

diff --git a/be/src/storage/tablet_manager.cpp b/be/src/storage/tablet_manager.cpp
@@ -983,28 +983,35 @@ Status TabletManager::report_all_tablets_info(std::map<TTabletId, TTablet>* tabl
 
     StarRocksMetrics::instance()->report_all_tablets_requests_total.increment(1);
 
-    size_t max_tablet_rowset_num = 0;
+    std::vector<TabletSharedPtr> all_tablets;
     for (const auto& tablets_shard : _tablets_shards) {
         std::shared_lock rlock(tablets_shard.lock);
-        for (const auto& [tablet_id, tablet_ptr] : tablets_shard.tablet_map) {
-            TTablet t_tablet;
-            TTabletInfo tablet_info;
-            tablet_ptr->build_tablet_report_info(&tablet_info);
-            max_tablet_rowset_num = std::max(max_tablet_rowset_num, tablet_ptr->version_count());
-            // find expired transaction corresponding to this tablet
-            TabletInfo tinfo(tablet_id, tablet_ptr->schema_hash(), tablet_ptr->tablet_uid());
-            auto find = expire_txn_map.find(tinfo);
-            if (find != expire_txn_map.end()) {
-                tablet_info.__set_transaction_ids(find->second);
-                expire_txn_map.erase(find);
-            }
-            t_tablet.tablet_infos.push_back(tablet_info);
+        for (const auto& [_, tablet_ptr] : tablets_shard.tablet_map) {
+            all_tablets.push_back(tablet_ptr);
+        }
+    }
 
-            if (!t_tablet.tablet_infos.empty()) {
-                tablets_info->emplace(tablet_id, t_tablet);
-            }
+    size_t max_tablet_rowset_num = 0;
+    for (auto& tablet_ptr : all_tablets) {
+        TTablet t_tablet;
+        TTabletInfo tablet_info;
+        tablet_ptr->build_tablet_report_info(&tablet_info);
+        max_tablet_rowset_num = std::max(max_tablet_rowset_num, tablet_ptr->version_count());
+        // find expired transaction corresponding to this tablet
+        TabletInfo tinfo(tablet_ptr->tablet_id(), tablet_ptr->schema_hash(), tablet_ptr->tablet_uid());
+        auto find = expire_txn_map.find(tinfo);
+        if (find != expire_txn_map.end()) {
+            tablet_info.__set_transaction_ids(find->second);
+            expire_txn_map.erase(find);
+        }
+        t_tablet.tablet_infos.push_back(tablet_info);
+
+        if (!t_tablet.tablet_infos.empty()) {
+            tablets_info->emplace(tablet_ptr->tablet_id(), t_tablet);
         }
     }
+    all_tablets.clear();
+
     LOG(INFO) << "Report all " << tablets_info->size()
               << " tablets info. max_tablet_rowset_num:" << max_tablet_rowset_num;
     StarRocksMetrics::instance()->max_tablet_rowset_num.set_value(max_tablet_rowset_num);