-
Notifications
You must be signed in to change notification settings - Fork 312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(hotkey): build a fundamental framework of hotkey detection #603
Conversation
b0db511
to
6986e58
Compare
_read_hotkey_collector->capture_raw_key(key, key.size() + value.size()); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's not needed to add capture_raw_key in this PR, since only this is just a 'fundamental framework' PR, and some othe read/write paths like add_write_cu missing call it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
capture_raw_key
is a fundamental interface of hotkey_collector
, I want to show how to use this function
Co-authored-by: Yingchun Lai <[email protected]>
@@ -122,21 +134,25 @@ void capacity_unit_calculator::add_multi_get_cu(int32_t status, | |||
} | |||
|
|||
if (status == rocksdb::Status::kNotFound) { | |||
_read_hotkey_collector->capture_hash_key(hash_key, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe you can move this into func add_read_cu
same with add_write_cu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
capture_hash_key
and add_read_cu
are two independent function. It will destroy the original semantics.
@@ -122,21 +134,25 @@ void capacity_unit_calculator::add_multi_get_cu(int32_t status, | |||
} | |||
|
|||
if (status == rocksdb::Status::kNotFound) { | |||
_read_hotkey_collector->capture_hash_key(hash_key, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice that you try to capture not-found key for multi_get, but not for single get, could you please explain why you design it?
void add_scan_cu(int32_t status, const std::vector<::dsn::apps::key_value> &kvs); | ||
void add_scan_cu(int32_t status, | ||
const std::vector<::dsn::apps::key_value> &kvs, | ||
const dsn::blob &hash_key_filter_pattern = dsn::blob()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the usage of hash_key_filter_pattern
?
@@ -55,6 +64,9 @@ class capacity_unit_calculator : public dsn::replication::replica_base | |||
#endif | |||
|
|||
private: | |||
void count_read_data(const dsn::blob &key, key_type type, int64_t size); | |||
void count_write_data(const dsn::blob &key, key_type type, int64_t size); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is function count_wirte_data
used for add cu and capture key for hotkey detection?
Hotkey detection process
What problem does this PR solve?
Rely PR: XiaoMi/rdsn#634
In this PR, I define two interfaces of
hotkey_collector
.capture_raw_key
is to capture data incapacity_unit_calculator
, in the future PR, I will analyze these data to find the hotkey.handle_operation
is to receive the control RPC of hotkey detection.capture_raw_key
is executed incapacity_unit_calculator
, I will implentment it in the next PR