[refactor][store] Optimize region metrics collection for region_size.

ketor · astor-oss · commit ab85985068bf · 2024-01-22T01:51:45.000Z
Skip region size calcuation when the region is not modified too much.
This will remarkablely reduct the performace penalty of region size
collection.

Signed-off-by: Ketor &lt;d.ketor@gmail.com&gt;
diff --git a/conf/index.template.yaml b/conf/index.template.yaml
@@ -7,6 +7,7 @@ server:
   port: $SERVER_PORT$
   heartbeat_interval_s: 6
   metrics_collect_interval_s: 300
+  approximate_size_metrics_collect_interval_s: 300
   scrub_vector_index_interval_s: 60
   worker_thread_num: 128 # must >4, worker_thread_num priority worker_thread_ratio
   # worker_thread_ratio: 2 # cpu core * ratio
@@ -18,6 +19,7 @@ server:
 region:
   region_max_size: 536870912 # 512MB
   enable_auto_split: true
+  split_check_interval_s: 120
 raft:
   host: $RAFT_HOST$
   port: $RAFT_PORT$
diff --git a/conf/store.template.yaml b/conf/store.template.yaml
@@ -7,6 +7,7 @@ server:
   port: $SERVER_PORT$
   heartbeat_interval_s: 6
   metrics_collect_interval_s: 300
+  approximate_size_metrics_collect_interval_s: 300
   worker_thread_num: 128 # must >4, worker_thread_num priority worker_thread_ratio
   # worker_thread_ratio: 1 # cpu core * ratio
   read_worker_num: 48 # read_worker_num + write_worker_num + raft_apply_worker_num must < server.worker_thread_num
@@ -17,6 +18,7 @@ server:
 region:
   region_max_size: 268435456 # 256MB
   enable_auto_split: true
+  split_check_interval_s: 120
 raft:
   host: $RAFT_HOST$
   port: $RAFT_PORT$
diff --git a/proto/common.proto b/proto/common.proto
@@ -694,10 +694,13 @@ message RegionMetrics {
   RegionDefinition region_definition = 5;     // region definition
   VectorIndexStatus vector_index_status = 6;  // vector index status
 
-  int64 row_count = 11;    // row count of this region
-  bytes min_key = 12;      // the min key of this region now exist
-  bytes max_key = 13;      // the max key of this region now exist
-  int64 region_size = 14;  // the bytes size of this region
+  int64 row_count = 11;                      // row count of this region
+  bytes min_key = 12;                        // the min key of this region now exist
+  bytes max_key = 13;                        // the max key of this region now exist
+  int64 region_size = 14;                    // the bytes size of this region
+  int64 last_update_metrics_log_index = 15;  // last update metrics log index
+  int64 last_update_metrics_version = 16;    // last update metrics version
+  int64 last_update_metrics_timestamp = 17;  // last update metrics timestamp
 
   // bool is_hold_vector_index = 29;                // is hold vector index
   VectorIndexMetrics vector_index_metrics = 20;  // vector index  metrics
diff --git a/src/common/constant.h b/src/common/constant.h
@@ -189,26 +189,6 @@ class Constant {
   // hnsw max elements expand number
   static const uint32_t kHnswMaxElementsExpandNum = 10000;
 
-  // crontab default interval
-  static const int32_t kHeartbeatIntervalS = 10;
-  static const int32_t kScanIntervalS = 30;
-  static const int32_t kPushIntervalS = 1;
-  static const int32_t kUpdateStateIntervalS = 10;
-  static const int32_t kTaskListIntervalS = 1;
-  static const int32_t kCalcMetricsIntervalS = 60;
-  static const int32_t kRecycleOrphanIntervalS = 60;
-  static const int32_t kRemoveWatchIntervalS = 60;
-  static const int32_t kLeaseIntervalS = 60;
-  static const int32_t kCompactionIntervalS = 300;
-  static const int32_t kScrubVectorIndexIntervalS = 60;
-  static const int32_t kApproximateSizeMetricsCollectIntervalS = 50;
-  static const int32_t kStoreMetricsCollectIntervalS = 30;
-  static const int32_t kRegionMetricsCollectIntervalS = 300;
-  static const int32_t kDefaultSplitCheckIntervalS = 120;
-  static const int32_t kRaftSnapshotIntervalS = 120;
-  static constexpr int32_t kGcUpdateSafePointIntervalS = 60;
-  static constexpr int32_t kGcDoGcPointIntervalS = 60;
-
   // raft snapshot
   inline static const std::string kRaftSnapshotRegionMetaFileName = "region_meta";
   inline static const std::string kRaftSnapshotRegionDateFileNameSuffix = ".dingo_sst";
diff --git a/src/crontab/crontab.cc b/src/crontab/crontab.cc
@@ -15,10 +15,10 @@
 #include "crontab/crontab.h"
 
 #include "bthread/bthread.h"
+#include "bthread/unstable.h"
 #include "common/logging.h"
 #include "common/role.h"
 #include "fmt/core.h"
-#include "server/server.h"
 
 namespace dingodb {
 
diff --git a/src/crontab/crontab.h b/src/crontab/crontab.h
@@ -20,11 +20,10 @@
 #include <functional>
 #include <map>
 #include <memory>
-#include <shared_mutex>
 #include <string>
 #include <vector>
 
-#include "bthread/unstable.h"
+#include "bthread/types.h"
 #include "proto/common.pb.h"
 
 namespace dingodb {
@@ -39,35 +38,24 @@ struct CrontabConfig {
 
 class Crontab {
  public:
-  Crontab()
-      : id(0),
-        interval(0),
-        max_times(0),
-        immediately(false),
-        run_count(0),
-        pause(false),
-        timer_id(0),
-        func(nullptr),
-        arg(nullptr) {}
-
-  uint32_t id;
+  uint32_t id{0};
   std::string name;
   // unit ms
-  int64_t interval;
+  int64_t interval{0};
   // 0 is no limit
-  uint32_t max_times;
+  uint32_t max_times{0};
   // Is immediately run
-  bool immediately;
+  bool immediately{false};
   // Already run count
-  int run_count;
+  int run_count{0};
   // Is pause crontab
-  bool pause;
+  bool pause{false};
   // bthread_timer_t handler
-  bthread_timer_t timer_id;
+  bthread_timer_t timer_id{0};
   // For run target function
   std::function<void(void*)> func;
   // Delivery to func_'s argument
-  void* arg;
+  void* arg{nullptr};
 };
 
 // Manage crontab use brpc::bthread_timer_add
diff --git a/src/engine/bdb_raw_engine.h b/src/engine/bdb_raw_engine.h
@@ -24,7 +24,6 @@
 #include <unordered_map>
 #include <vector>
 
-#include "bthread/types.h"
 #include "config/config.h"
 #include "db_cxx.h"
 #include "engine/iterator.h"
diff --git a/src/handler/raft_apply_handler.cc b/src/handler/raft_apply_handler.cc
@@ -624,8 +624,9 @@ int SplitHandler::Handle(std::shared_ptr<Context>, store::RegionPtr from_region,
   }
 
   // Update region metrics min/max key policy
+  // Update region_size in next collect region metrics
   if (region_metrics != nullptr) {
-    region_metrics->UpdateMaxAndMinKeyPolicy();
+    region_metrics->ResetMetricsForRegionVersionUpdate();
   }
 
   return 0;
@@ -787,7 +788,8 @@ int PrepareMergeHandler::Handle(std::shared_ptr<Context>, store::RegionPtr sourc
 }
 
 int CommitMergeHandler::Handle(std::shared_ptr<Context>, store::RegionPtr target_region, std::shared_ptr<RawEngine>,
-                               const pb::raft::Request &req, store::RegionMetricsPtr, int64_t, int64_t /*log_id*/) {
+                               const pb::raft::Request &req, store::RegionMetricsPtr region_metrics, int64_t,
+                               int64_t /*log_id*/) {
   assert(target_region != nullptr);
   const auto &request = req.commit_merge();
   auto store_region_meta = GET_STORE_REGION_META;
@@ -934,6 +936,12 @@ int CommitMergeHandler::Handle(std::shared_ptr<Context>, store::RegionPtr target
       request.entries().size(), actual_apply_log_count, target_region->EpochToString(), target_region->RangeToString(),
       Helper::TimestampMs() - start_time);
 
+  // Update region metrics min/max key policy
+  // Update region_size in next collect region metrics
+  if (region_metrics != nullptr) {
+    region_metrics->ResetMetricsForRegionVersionUpdate();
+  }
+
   return 0;
 }
 
diff --git a/src/metrics/store_metrics_manager.cc b/src/metrics/store_metrics_manager.cc
@@ -24,12 +24,11 @@
 #include "common/constant.h"
 #include "common/helper.h"
 #include "common/logging.h"
-#include "common/role.h"
 #include "config/config_manager.h"
 #include "fmt/core.h"
+#include "gflags/gflags.h"
 #include "proto/common.pb.h"
 #include "server/server.h"
-#include "vector/vector_index_manager.h"
 
 namespace dingodb {
 
@@ -312,20 +311,67 @@ std::vector<std::vector<store::RegionPtr>> GenBatchRegion(std::vector<store::Reg
   return result;
 }
 
+DEFINE_int64(collect_approximate_size_log_index_interval, 10,
+             "Collecting approximate size log index interval, the region has raft log index greater than this value "
+             "will be collected region size");
+
 bool StoreRegionMetrics::CollectApproximateSizeMetrics() {
   auto store_region_meta = GET_STORE_REGION_META;
   auto region_metricses = GetAllMetrics();
 
   std::vector<store::RegionPtr> need_collect_rocks_regions;
   std::vector<store::RegionPtr> need_collect_bdb_regions;
+
   for (const auto& region_metrics : region_metricses) {
+    DINGO_LOG(DEBUG) << fmt::format(
+        "[metrics.region] collect approximate size metrics start, region({}) log_index_id({})", region_metrics->Id(),
+        region_metrics->LastLogIndex());
+
     auto region = store_region_meta->GetRegion(region_metrics->Id());
     if (region == nullptr) {
+      DINGO_LOG(INFO) << fmt::format("[metrics.region] skip collect approximate size metrics, region({}) not exist",
+                                     region_metrics->Id());
       continue;
     }
+
     if (region->State() != pb::common::NORMAL) {
+      DINGO_LOG(INFO) << fmt::format(
+          "[metrics.region] skip collect approximate size metrics for state not NORMAL, region({}) state({})",
+          region->Id(), pb::common::StoreRegionState_Name(region->State()));
       continue;
     }
+
+    // first update region_version in region metrics
+    auto region_version = region->Epoch().version();
+    if (region_version > region_metrics->RegionVersion()) {
+      region_metrics->SetRegionVersion(region_version);
+    }
+
+    // get region log index id
+    auto log_index_id = region_metrics->LastLogIndex();
+
+    // get last update metrics log index and version
+    auto last_update_metrics_log_index = region_metrics->LastUpdateMetricsLogIndex();
+    auto last_update_metrics_version = region_metrics->LastUpdateMetricsVersion();
+
+    // if the region version has been updated, update the last update metrics version, this is done in SetRegionSize
+    // else if the region version is equal and the difference between the current log index and the last update
+    // metrics log index is less than the interval, skip it
+    if (region_version <= last_update_metrics_version &&
+        (log_index_id - last_update_metrics_log_index < FLAGS_collect_approximate_size_log_index_interval)) {
+      DINGO_LOG(INFO) << fmt::format(
+          "[metrics.region] skip collect approximate size metrics, region({}) log_index_id({}) region_version({}) "
+          "last_update_metrics_log_index({}) last_update_metrics_version({})",
+          region->Id(), log_index_id, region_version, last_update_metrics_log_index, last_update_metrics_version);
+
+      continue;
+    }
+
+    DINGO_LOG(INFO) << fmt::format(
+        "[metrics.region] collect approximate size metrics, region({}) log_index_id({}) version({}) "
+        "last_update_metrics_log_index({}) last_update_metrics_version({})",
+        region->Id(), log_index_id, region_version, last_update_metrics_log_index, last_update_metrics_version);
+
     if (region->GetRawEngineType() == pb::common::RAW_ENG_ROCKSDB) {
       need_collect_rocks_regions.push_back(region);
     } else if (region->GetRawEngineType() == pb::common::RAW_ENG_BDB) {
@@ -346,6 +392,11 @@ bool StoreRegionMetrics::CollectApproximateSizeMetrics() {
         auto region_metrics = GetMetrics(region_id);
         if (region_metrics != nullptr) {
           region_metrics->SetRegionSize(size);
+          region_metrics->UpdateLastUpdateMetricsLogIndex();
+
+          DINGO_LOG(INFO) << fmt::format(
+              "[metrics.region] get rocksdb region approximate size region({}) size({}) elapsed time[{} ms]", region_id,
+              size, Helper::TimestampMs() - start_time);
         }
       }
     }
@@ -368,6 +419,11 @@ bool StoreRegionMetrics::CollectApproximateSizeMetrics() {
         auto region_metrics = GetMetrics(region_id);
         if (region_metrics != nullptr) {
           region_metrics->SetRegionSize(size);
+          region_metrics->UpdateLastUpdateMetricsLogIndex();
+
+          DINGO_LOG(INFO) << fmt::format(
+              "[metrics.region] get bdb region approximate size region({}) size({}) elapsed time[{} ms]", region_id,
+              size, Helper::TimestampMs() - start_time);
         }
       }
     }
diff --git a/src/metrics/store_metrics_manager.h b/src/metrics/store_metrics_manager.h
diff --git a/src/server/server.cc b/src/server/server.cc
diff --git a/src/split/split_checker.cc b/src/split/split_checker.cc
