enhance performance for broken tablet checking under multi-core scena…

…rio with a coarse-grained read lock
apache · Dec 18, 2023 · 8e4a7ec · 8e4a7ec
1 parent 03e989b
commit 8e4a7ec
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 21 deletions.
diff --git a/be/src/runtime/tablets_channel.cpp b/be/src/runtime/tablets_channel.cpp
@@ -17,9 +17,11 @@
 
 #include "runtime/tablets_channel.h"
 
+#include <_types/_uint32_t.h>
 #include <fmt/format.h>
 #include <gen_cpp/internal_service.pb.h>
 #include <gen_cpp/types.pb.h>
+#include <sys/_types/_int64_t.h>
 #include <time.h>
 
 #include "common/compiler_util.h" // IWYU pragma: keep
@@ -29,7 +31,9 @@
 #include <initializer_list>
 #include <set>
 #include <thread>
+#include <unordered_map>
 #include <utility>
+#include <vector>
 
 #ifdef DEBUG
 #include <unordered_set>
@@ -513,24 +517,8 @@ Status BaseTabletsChannel::add_batch(const PTabletWriterAddBlockRequest& request
 
     std::unordered_map<int64_t /* tablet_id */, std::vector<uint32_t> /* row index */>
             tablet_to_rowidxs;
-    for (uint32_t i = 0; i < request.tablet_ids_size(); ++i) {
-        if (request.is_single_tablet_block()) {
-            break;
-        }
-        int64_t tablet_id = request.tablet_ids(i);
-        if (_is_broken_tablet(tablet_id)) {
-            // skip broken tablets
-            VLOG_PROGRESS << "skip broken tablet tablet=" << tablet_id;
-            continue;
-        }
-        auto it = tablet_to_rowidxs.find(tablet_id);
-        if (it == tablet_to_rowidxs.end()) {
-            tablet_to_rowidxs.emplace(tablet_id, std::initializer_list<uint32_t> {i});
-        } else {
-            it->second.emplace_back(i);
-        }
-    }
-
+    _build_tablet_to_rowidxs(request, &tablet_to_rowidxs);
+
     vectorized::Block send_data;
     RETURN_IF_ERROR(send_data.deserialize(request.block()));
     CHECK(send_data.rows() == request.tablet_ids_size())
@@ -589,9 +577,30 @@ void BaseTabletsChannel::_add_broken_tablet(int64_t tablet_id) {
     _broken_tablets.insert(tablet_id);
 }
 
-bool BaseTabletsChannel::_is_broken_tablet(int64_t tablet_id) {
+void BaseTabletsChannel::_build_tablet_to_rowidxs(
+        const PTabletWriterAddBlockRequest& request,
+        std::unordered_map<int64_t, std::vector<uint32_t>>* tablet_to_rowidxs) {
+    // just add a coarse-grained read lock here rather than each time when visiting _broken_tablets
+    // tests show that the later way of read lock is too fine-grained that add to the performance overhead,
+    // especially under multicore scenario
     std::shared_lock<std::shared_mutex> rlock(_broken_tablets_lock);
-    return _broken_tablets.find(tablet_id) != _broken_tablets.end();
+    for (uint32_t i = 0; i < request.tablet_ids_size(); ++i) {
+        if (request.is_single_tablet_block()) {
+            break;
+        }
+        int64_t tablet_id = request.tablet_ids(i);
+        if (_broken_tablets.find(tablet_id) != _broken_tablets.end()) {
+            // skip broken tablets
+            VLOG_PROGRESS << "skip broken tablet tablet=" << tablet_id;
+            continue;
+        }
+        auto it = tablet_to_rowidxs->find(tablet_id);
+        if (it == tablet_to_rowidxs->end()) {
+            tablet_to_rowidxs->emplace(tablet_id, std::initializer_list<uint32_t> {i});
+        } else {
+            it->second.emplace_back(i);
+        }
+    }
 }
 
 } // namespace doris
diff --git a/be/src/runtime/tablets_channel.h b/be/src/runtime/tablets_channel.h
@@ -118,7 +118,10 @@ class BaseTabletsChannel {
     void _add_broken_tablet(int64_t tablet_id);
     void _add_error_tablet(google::protobuf::RepeatedPtrField<PTabletError>* tablet_errors,
                            int64_t tablet_id, Status error) const;
-    bool _is_broken_tablet(int64_t tablet_id);
+    void _build_tablet_to_rowidxs(
+            const PTabletWriterAddBlockRequest& request,
+            std::unordered_map<int64_t /* tablet_id */, std::vector<uint32_t> /* row index */>*
+                    tablet_to_rowidxs);
     virtual void _init_profile(RuntimeProfile* profile);
 
     // id of this load channel