feat: add an interface to get read_throttling_reject_count perf counter #940
Conversation
Smityz
changed the title
src/replica/replication_app_base.cpp
Outdated
|
|
||
| dsn::perf_counter *replication_app_base::get_counter_recent_read_throttling_reject_count() | ||
| { | ||
| ; |
| @@ -294,6 +294,8 @@ class replication_app_base : public replica_base | |||
| replica_init_info _info; | |||
|
|
|||
| explicit replication_app_base(::dsn::replication::replica *replica); | |||
|
|
|||
| dsn::perf_counter *get_counter_recent_read_throttling_reject_count(); | |||
There was a problem hiding this comment.
why didn't you return the smart pointer?
There was a problem hiding this comment.
the original perf_counter is a raw point, passed by _counter_recent_read_throttling_reject_count.get()
There was a problem hiding this comment.
I agree with @levy5307, it's better to avoid raw pointer and use smart pointer, although the original value is raw.
There was a problem hiding this comment.
-
It's meaningless to wrap a raw point by shared_ptr
-
I think it's no risk there because
replicahas the longer lifetime than_app
ref: https://github.com/XiaoMi/rdsn/blob/master/src/replica/replica.h#L568
https://github.com/XiaoMi/rdsn/blob/master/src/replica/replica.h#L488 -
If I refactor raw point to shared_ptr here, it would be a big change, due to there are many codes using
define, and I have to replace->to->operator->()
ref: https://github.com/XiaoMi/rdsn/blob/master/src/replica/replica_throttle.cpp#L30
There was a problem hiding this comment.
I have another question, why would you like to pass this perf-counter to pegasus?
There was a problem hiding this comment.
the throttling_controller works on Pegasus, so must use the same perf_counter to collect read reject count
There was a problem hiding this comment.
I have another question, why would you like to pass this perf-counter to pegasus?
+1, the function put here seems to be strange
Related: apache/incubator-pegasus#830