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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
