You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by "empiredan (via GitHub)" <gi...@apache.org> on 2023/02/14 10:25:24 UTC

[GitHub] [incubator-pegasus] empiredan opened a new pull request, #1351: feat(new_metrics): migrate replica-level metrics for write service

empiredan opened a new pull request, #1351:
URL: https://github.com/apache/incubator-pegasus/pull/1351

   This PR is to resolve https://github.com/apache/incubator-pegasus/issues/1336.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] empiredan commented on a diff in pull request #1351: feat(new_metrics): migrate replica-level metrics for write service

Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #1351:
URL: https://github.com/apache/incubator-pegasus/pull/1351#discussion_r1115282563


##########
src/server/pegasus_write_service.cpp:
##########
@@ -36,111 +124,36 @@ pegasus_write_service::pegasus_write_service(pegasus_server_impl *server)
       _server(server),
       _impl(new impl(server)),
       _batch_start_time(0),
-      _cu_calculator(server->_cu_calculator.get())
+      _cu_calculator(server->_cu_calculator.get()),
+      _put_counter(METRIC_put_requests.instantiate(replica_metric_entity())),

Review Comment:
   Normally instantiations for all metric types will not lead to error. Once instantiation fails, the process will crash which means there must have been some bug. For example, the metric entity is uninitialized. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] empiredan commented on a diff in pull request #1351: feat(new_metrics): migrate replica-level metrics for write service

Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #1351:
URL: https://github.com/apache/incubator-pegasus/pull/1351#discussion_r1115277561


##########
src/utils/time_utils.h:
##########
@@ -126,5 +130,29 @@ inline int64_t hh_mm_today_to_unix_sec(string_view hhmm_of_day)
     return get_unix_sec_today_midnight() + sec_of_day;
 }
 
+class chronograph
+{
+public:
+    chronograph() : chronograph(dsn_now_ns()) {}
+    chronograph(uint64_t start_time_ns) : _start_time_ns(start_time_ns) {}
+    ~chronograph() = default;
+
+    inline void reset_start_time() { _start_time_ns = dsn_now_ns(); }
+
+    template <typename T, typename = typename std::enable_if<std::is_integral<T>::value>::type>

Review Comment:
   OK, since internally we use `dsn_now_ns()` to save timestamp, I'll change the type to `uint64_t`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] acelyc111 merged pull request #1351: feat(new_metrics): migrate replica-level metrics for write service

Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 merged PR #1351:
URL: https://github.com/apache/incubator-pegasus/pull/1351


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1351: feat(new_metrics): migrate replica-level metrics for write service

Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1351:
URL: https://github.com/apache/incubator-pegasus/pull/1351#discussion_r1111057278


##########
src/server/pegasus_write_service.h:
##########
@@ -199,28 +197,29 @@ class pegasus_write_service : dsn::replication::replica_base
     capacity_unit_calculator *_cu_calculator;
     int64_t _dup_lagging_write_threshold_ms;
 
-    ::dsn::perf_counter_wrapper _pfc_put_qps;
-    ::dsn::perf_counter_wrapper _pfc_multi_put_qps;
-    ::dsn::perf_counter_wrapper _pfc_remove_qps;
-    ::dsn::perf_counter_wrapper _pfc_multi_remove_qps;
-    ::dsn::perf_counter_wrapper _pfc_incr_qps;
-    ::dsn::perf_counter_wrapper _pfc_check_and_set_qps;
-    ::dsn::perf_counter_wrapper _pfc_check_and_mutate_qps;
-    ::dsn::perf_counter_wrapper _pfc_duplicate_qps;
-    ::dsn::perf_counter_wrapper _pfc_dup_time_lag;
-    ::dsn::perf_counter_wrapper _pfc_dup_lagging_writes;
-
-    ::dsn::perf_counter_wrapper _pfc_put_latency;
-    ::dsn::perf_counter_wrapper _pfc_multi_put_latency;
-    ::dsn::perf_counter_wrapper _pfc_remove_latency;
-    ::dsn::perf_counter_wrapper _pfc_multi_remove_latency;
-    ::dsn::perf_counter_wrapper _pfc_incr_latency;
-    ::dsn::perf_counter_wrapper _pfc_check_and_set_latency;
-    ::dsn::perf_counter_wrapper _pfc_check_and_mutate_latency;
-
-    // Records all requests.
-    std::vector<::dsn::perf_counter *> _batch_qps_perfcounters;
-    std::vector<::dsn::perf_counter *> _batch_latency_perfcounters;
+    dsn::counter_ptr<> _put_counter;
+    dsn::counter_ptr<> _multi_put_counter;
+    dsn::counter_ptr<> _remove_counter;
+    dsn::counter_ptr<> _multi_remove_counter;
+    dsn::counter_ptr<> _incr_counter;
+    dsn::counter_ptr<> _check_and_set_counter;
+    dsn::counter_ptr<> _check_and_mutate_counter;
+
+    dsn::percentile_ptr<int64_t> _put_latency_ns;
+    dsn::percentile_ptr<int64_t> _multi_put_latency_ns;
+    dsn::percentile_ptr<int64_t> _remove_latency_ns;
+    dsn::percentile_ptr<int64_t> _multi_remove_latency_ns;
+    dsn::percentile_ptr<int64_t> _incr_latency_ns;
+    dsn::percentile_ptr<int64_t> _check_and_set_latency_ns;
+    dsn::percentile_ptr<int64_t> _check_and_mutate_latency_ns;
+
+    dsn::counter_ptr<> _dup_counter;
+    dsn::percentile_ptr<int64_t> _dup_time_lag_ms;
+    dsn::counter_ptr<> _dup_lagging_write_counter;
+
+    // Record batch size for put and remove requests.
+    uint32_t _put_batch_size;

Review Comment:
   nit: Use atomic?



##########
src/server/pegasus_write_service.cpp:
##########
@@ -36,111 +124,36 @@ pegasus_write_service::pegasus_write_service(pegasus_server_impl *server)
       _server(server),
       _impl(new impl(server)),
       _batch_start_time(0),
-      _cu_calculator(server->_cu_calculator.get())
+      _cu_calculator(server->_cu_calculator.get()),
+      _put_counter(METRIC_put_requests.instantiate(replica_metric_entity())),

Review Comment:
   Will the counter_ptr/percentile_ptr get any error if not initialized in c'tor? It will be helpful to prevent missing initialized.



##########
src/server/pegasus_write_service.cpp:
##########
@@ -36,111 +124,36 @@ pegasus_write_service::pegasus_write_service(pegasus_server_impl *server)
       _server(server),
       _impl(new impl(server)),
       _batch_start_time(0),
-      _cu_calculator(server->_cu_calculator.get())
+      _cu_calculator(server->_cu_calculator.get()),

Review Comment:
   How about use macros to format/simplify these codes? (Maybe the member variables and metrics should have some similar naming?)



##########
src/server/pegasus_write_service.cpp:
##########
@@ -296,15 +307,23 @@ void pegasus_write_service::set_default_ttl(uint32_t ttl) { _impl->set_default_t
 
 void pegasus_write_service::clear_up_batch_states()
 {
-    uint64_t latency = dsn_now_ns() - _batch_start_time;
-    for (dsn::perf_counter *pfc : _batch_qps_perfcounters)
-        pfc->increment();
-    for (dsn::perf_counter *pfc : _batch_latency_perfcounters)
-        pfc->set(latency);
-
-    _batch_qps_perfcounters.clear();
-    _batch_latency_perfcounters.clear();
+#define PROCESS_WRITE_BATCH(op)                                                                    \
+    do {                                                                                           \
+        _##op##_counter->increment_by(static_cast<int64_t>(_##op##_batch_size));                   \
+        for (uint32_t i = 0; i < _##op##_batch_size; ++i) {                                        \
+            _##op##_latency_ns->set(latency_ns);                                                   \

Review Comment:
   How about encapsulate another function like `set(count, value)` for percentile metrics?



##########
src/utils/time_utils.h:
##########
@@ -126,5 +130,29 @@ inline int64_t hh_mm_today_to_unix_sec(string_view hhmm_of_day)
     return get_unix_sec_today_midnight() + sec_of_day;
 }
 
+class chronograph
+{
+public:
+    chronograph() : chronograph(dsn_now_ns()) {}
+    chronograph(uint64_t start_time_ns) : _start_time_ns(start_time_ns) {}
+    ~chronograph() = default;
+
+    inline void reset_start_time() { _start_time_ns = dsn_now_ns(); }
+
+    template <typename T, typename = typename std::enable_if<std::is_integral<T>::value>::type>

Review Comment:
   How about set a default type `uint64_t` for `T`?



##########
src/server/pegasus_write_service.h:
##########
@@ -199,28 +197,29 @@ class pegasus_write_service : dsn::replication::replica_base
     capacity_unit_calculator *_cu_calculator;
     int64_t _dup_lagging_write_threshold_ms;
 
-    ::dsn::perf_counter_wrapper _pfc_put_qps;
-    ::dsn::perf_counter_wrapper _pfc_multi_put_qps;
-    ::dsn::perf_counter_wrapper _pfc_remove_qps;
-    ::dsn::perf_counter_wrapper _pfc_multi_remove_qps;
-    ::dsn::perf_counter_wrapper _pfc_incr_qps;
-    ::dsn::perf_counter_wrapper _pfc_check_and_set_qps;
-    ::dsn::perf_counter_wrapper _pfc_check_and_mutate_qps;
-    ::dsn::perf_counter_wrapper _pfc_duplicate_qps;
-    ::dsn::perf_counter_wrapper _pfc_dup_time_lag;
-    ::dsn::perf_counter_wrapper _pfc_dup_lagging_writes;
-
-    ::dsn::perf_counter_wrapper _pfc_put_latency;
-    ::dsn::perf_counter_wrapper _pfc_multi_put_latency;
-    ::dsn::perf_counter_wrapper _pfc_remove_latency;
-    ::dsn::perf_counter_wrapper _pfc_multi_remove_latency;
-    ::dsn::perf_counter_wrapper _pfc_incr_latency;
-    ::dsn::perf_counter_wrapper _pfc_check_and_set_latency;
-    ::dsn::perf_counter_wrapper _pfc_check_and_mutate_latency;
-
-    // Records all requests.
-    std::vector<::dsn::perf_counter *> _batch_qps_perfcounters;
-    std::vector<::dsn::perf_counter *> _batch_latency_perfcounters;

Review Comment:
   Have we lost some metrics?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] empiredan commented on a diff in pull request #1351: feat(new_metrics): migrate replica-level metrics for write service

Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #1351:
URL: https://github.com/apache/incubator-pegasus/pull/1351#discussion_r1115278575


##########
src/server/pegasus_write_service.cpp:
##########
@@ -296,15 +307,23 @@ void pegasus_write_service::set_default_ttl(uint32_t ttl) { _impl->set_default_t
 
 void pegasus_write_service::clear_up_batch_states()
 {
-    uint64_t latency = dsn_now_ns() - _batch_start_time;
-    for (dsn::perf_counter *pfc : _batch_qps_perfcounters)
-        pfc->increment();
-    for (dsn::perf_counter *pfc : _batch_latency_perfcounters)
-        pfc->set(latency);
-
-    _batch_qps_perfcounters.clear();
-    _batch_latency_perfcounters.clear();
+#define PROCESS_WRITE_BATCH(op)                                                                    \
+    do {                                                                                           \
+        _##op##_counter->increment_by(static_cast<int64_t>(_##op##_batch_size));                   \
+        for (uint32_t i = 0; i < _##op##_batch_size; ++i) {                                        \
+            _##op##_latency_ns->set(latency_ns);                                                   \

Review Comment:
   Good idea, I'll encapsulate this interface which is actually needed by unit-tests.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] empiredan commented on a diff in pull request #1351: feat(new_metrics): migrate replica-level metrics for write service

Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #1351:
URL: https://github.com/apache/incubator-pegasus/pull/1351#discussion_r1115816768


##########
src/utils/metrics.h:
##########
@@ -553,6 +592,31 @@ enum class metric_unit
     kInvalidUnit,
 };
 
+#define METRIC_ASSERT_UNIT_LATENCY(unit, index)                                                    \
+    static_assert(static_cast<size_t>(metric_unit::unit) == index,                                 \
+                  #unit " should be at index " #index)
+
+METRIC_ASSERT_UNIT_LATENCY(kNanoSeconds, 0);
+METRIC_ASSERT_UNIT_LATENCY(kMicroSeconds, 1);
+METRIC_ASSERT_UNIT_LATENCY(kMilliSeconds, 2);
+METRIC_ASSERT_UNIT_LATENCY(kSeconds, 3);
+
+const std::vector<uint64_t> kMetricLatencyConverterFromNS = {
+    1, 1000, 1000 * 1000, 1000 * 1000 * 1000};
+
+inline uint64_t convert_metric_latency_from_ns(uint64_t latency_ns, metric_unit target_unit)
+{
+    if (target_unit == metric_unit::kNanoSeconds) {
+        // Since nanoseconds are used as the latency unit more frequently, eliminate unnecessary

Review Comment:
   OK.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] empiredan commented on a diff in pull request #1351: feat(new_metrics): migrate replica-level metrics for write service

Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #1351:
URL: https://github.com/apache/incubator-pegasus/pull/1351#discussion_r1115273755


##########
src/server/pegasus_write_service.h:
##########
@@ -199,28 +197,29 @@ class pegasus_write_service : dsn::replication::replica_base
     capacity_unit_calculator *_cu_calculator;
     int64_t _dup_lagging_write_threshold_ms;
 
-    ::dsn::perf_counter_wrapper _pfc_put_qps;
-    ::dsn::perf_counter_wrapper _pfc_multi_put_qps;
-    ::dsn::perf_counter_wrapper _pfc_remove_qps;
-    ::dsn::perf_counter_wrapper _pfc_multi_remove_qps;
-    ::dsn::perf_counter_wrapper _pfc_incr_qps;
-    ::dsn::perf_counter_wrapper _pfc_check_and_set_qps;
-    ::dsn::perf_counter_wrapper _pfc_check_and_mutate_qps;
-    ::dsn::perf_counter_wrapper _pfc_duplicate_qps;
-    ::dsn::perf_counter_wrapper _pfc_dup_time_lag;
-    ::dsn::perf_counter_wrapper _pfc_dup_lagging_writes;
-
-    ::dsn::perf_counter_wrapper _pfc_put_latency;
-    ::dsn::perf_counter_wrapper _pfc_multi_put_latency;
-    ::dsn::perf_counter_wrapper _pfc_remove_latency;
-    ::dsn::perf_counter_wrapper _pfc_multi_remove_latency;
-    ::dsn::perf_counter_wrapper _pfc_incr_latency;
-    ::dsn::perf_counter_wrapper _pfc_check_and_set_latency;
-    ::dsn::perf_counter_wrapper _pfc_check_and_mutate_latency;
-
-    // Records all requests.
-    std::vector<::dsn::perf_counter *> _batch_qps_perfcounters;
-    std::vector<::dsn::perf_counter *> _batch_latency_perfcounters;
+    dsn::counter_ptr<> _put_counter;
+    dsn::counter_ptr<> _multi_put_counter;
+    dsn::counter_ptr<> _remove_counter;
+    dsn::counter_ptr<> _multi_remove_counter;
+    dsn::counter_ptr<> _incr_counter;
+    dsn::counter_ptr<> _check_and_set_counter;
+    dsn::counter_ptr<> _check_and_mutate_counter;
+
+    dsn::percentile_ptr<int64_t> _put_latency_ns;
+    dsn::percentile_ptr<int64_t> _multi_put_latency_ns;
+    dsn::percentile_ptr<int64_t> _remove_latency_ns;
+    dsn::percentile_ptr<int64_t> _multi_remove_latency_ns;
+    dsn::percentile_ptr<int64_t> _incr_latency_ns;
+    dsn::percentile_ptr<int64_t> _check_and_set_latency_ns;
+    dsn::percentile_ptr<int64_t> _check_and_mutate_latency_ns;
+
+    dsn::counter_ptr<> _dup_counter;
+    dsn::percentile_ptr<int64_t> _dup_time_lag_ms;
+    dsn::counter_ptr<> _dup_lagging_write_counter;
+
+    // Record batch size for put and remove requests.
+    uint32_t _put_batch_size;

Review Comment:
   There's only one thread serving all write operations on one replica; besides, previously `std::vector` is used to store pointers of perf-counters which is not thread-safe. Therefore I think `uint32_t` is enough.



##########
src/server/pegasus_write_service.h:
##########
@@ -199,28 +197,29 @@ class pegasus_write_service : dsn::replication::replica_base
     capacity_unit_calculator *_cu_calculator;
     int64_t _dup_lagging_write_threshold_ms;
 
-    ::dsn::perf_counter_wrapper _pfc_put_qps;
-    ::dsn::perf_counter_wrapper _pfc_multi_put_qps;
-    ::dsn::perf_counter_wrapper _pfc_remove_qps;
-    ::dsn::perf_counter_wrapper _pfc_multi_remove_qps;
-    ::dsn::perf_counter_wrapper _pfc_incr_qps;
-    ::dsn::perf_counter_wrapper _pfc_check_and_set_qps;
-    ::dsn::perf_counter_wrapper _pfc_check_and_mutate_qps;
-    ::dsn::perf_counter_wrapper _pfc_duplicate_qps;
-    ::dsn::perf_counter_wrapper _pfc_dup_time_lag;
-    ::dsn::perf_counter_wrapper _pfc_dup_lagging_writes;
-
-    ::dsn::perf_counter_wrapper _pfc_put_latency;
-    ::dsn::perf_counter_wrapper _pfc_multi_put_latency;
-    ::dsn::perf_counter_wrapper _pfc_remove_latency;
-    ::dsn::perf_counter_wrapper _pfc_multi_remove_latency;
-    ::dsn::perf_counter_wrapper _pfc_incr_latency;
-    ::dsn::perf_counter_wrapper _pfc_check_and_set_latency;
-    ::dsn::perf_counter_wrapper _pfc_check_and_mutate_latency;
-
-    // Records all requests.
-    std::vector<::dsn::perf_counter *> _batch_qps_perfcounters;
-    std::vector<::dsn::perf_counter *> _batch_latency_perfcounters;

Review Comment:
   Actually there are only pointers of `put` and `remove` perf-counters in the `std::vector`, which are heavy for the problem. We can just use 2 counters instead, and there will be no metric lost.



##########
src/server/pegasus_write_service.cpp:
##########
@@ -36,111 +124,36 @@ pegasus_write_service::pegasus_write_service(pegasus_server_impl *server)
       _server(server),
       _impl(new impl(server)),
       _batch_start_time(0),
-      _cu_calculator(server->_cu_calculator.get())
+      _cu_calculator(server->_cu_calculator.get()),

Review Comment:
   OK, I'll add a new macro system to simplify/standardize operations for metric variables.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1351: feat(new_metrics): migrate replica-level metrics for write service

Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1351:
URL: https://github.com/apache/incubator-pegasus/pull/1351#discussion_r1115276683


##########
src/utils/metrics.h:
##########
@@ -133,6 +136,42 @@
 #define METRIC_DECLARE_percentile_double(name)                                                     \
     extern dsn::floating_percentile_prototype<double> METRIC_##name
 
+// Following METRIC_*VAR* macros are introduced so that:
+// * only need to use prototype name to operate each metric variable;
+// * uniformly name each variable in user class;
+// * differentiate operations on metrics significantly from main logic, improving code readability.
+
+// Declare a metric variable in user class.
+//
+// Since a type tends to be a class template where there might be commas, use variadic arguments
+// instead of a single fixed argumen to represent a type.

Review Comment:
   ```suggestion
   // instead of a single fixed argument to represent a type.
   ```



##########
src/utils/metrics.h:
##########
@@ -553,6 +592,31 @@ enum class metric_unit
     kInvalidUnit,
 };
 
+#define METRIC_ASSERT_UNIT_LATENCY(unit, index)                                                    \
+    static_assert(static_cast<size_t>(metric_unit::unit) == index,                                 \
+                  #unit " should be at index " #index)
+
+METRIC_ASSERT_UNIT_LATENCY(kNanoSeconds, 0);
+METRIC_ASSERT_UNIT_LATENCY(kMicroSeconds, 1);
+METRIC_ASSERT_UNIT_LATENCY(kMilliSeconds, 2);
+METRIC_ASSERT_UNIT_LATENCY(kSeconds, 3);
+
+const std::vector<uint64_t> kMetricLatencyConverterFromNS = {
+    1, 1000, 1000 * 1000, 1000 * 1000 * 1000};
+
+inline uint64_t convert_metric_latency_from_ns(uint64_t latency_ns, metric_unit target_unit)
+{
+    if (target_unit == metric_unit::kNanoSeconds) {
+        // Since nanoseconds are used as the latency unit more frequently, eliminate unnecessary

Review Comment:
   How about add `dsn_likely`?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org