Shuo-Jia commented on a change in pull request #605:
URL: https://github.com/apache/incubator-pegasus/pull/605#discussion_r491936351



##########
File path: src/client_lib/pegasus_client_impl.cpp
##########
@@ -1302,5 +1303,61 @@ const char *pegasus_client_impl::get_error_string(int 
error_code) const
 {
     return (rocskdb_error == 0) ? 0 : ROCSKDB_ERROR_START - rocskdb_error;
 }
+
+bool pegasus_client_impl::detect_hotkey(const std::string &hotkey_type,
+                                        const std::string &hotkey_action,
+                                        uint64_t partition_hash,
+                                        std::string &err_info)
+{
+    if (partition_hash < 0) {
+        err_info = "error partition_num";
+        return false;
+    }
+
+    dsn::apps::hotkey_detect_request req;
+    std::string hotkey_type_check = hotkey_type;
+    std::transform(
+        hotkey_type_check.begin(), hotkey_type_check.end(), 
hotkey_type_check.begin(), ::tolower);
+    if (hotkey_type_check == "read") {
+        req.type = dsn::apps::hotkey_type::type::READ;
+    } else if (hotkey_type_check == "write") {
+        req.type = dsn::apps::hotkey_type::type::WRITE;
+    } else {
+        err_info = "wrong hotkey type";
+        return false;
+    }
+
+    std::string hotkey_action_check = hotkey_action;
+    std::transform(hotkey_action_check.begin(),
+                   hotkey_action_check.end(),
+                   hotkey_action_check.begin(),
+                   ::tolower);
+    if (hotkey_action_check == "start") {
+        req.action = dsn::apps::hotkey_detect_action::START;
+    } else if (hotkey_action_check == "stop") {
+        req.action = dsn::apps::hotkey_detect_action::STOP;
+    } else {
+        err_info = "wrong action type";
+        return false;
+    }
+

Review comment:
       you‘d better refactor param `check` and `transform` to one method,  such 
as `generate_hotkey_request`. And I think it should be the `command` logic but 
not put into pegasus_client_iml.cpp

##########
File path: src/shell/commands/data_operations.cpp
##########
@@ -2664,3 +2665,45 @@ bool calculate_hash_value(command_executor *e, 
shell_context *sc, arguments args
     tp.output(std::cout);
     return true;
 }
+
+// TODO: (Tangyanzhao) merge 
hotspot_partition_calculator::send_hotkey_detect_request
+bool detect_hotkey(command_executor *e, shell_context *sc, arguments args)
+{
+    static struct option long_options[] = {{"partition_num", 
required_argument, 0, 'p'},
+                                           {"hotkey_type", required_argument, 
0, 't'},
+                                           {"action", required_argument, 0, 
'a'},
+                                           {0, 0, 0, 0}};
+
+    std::string hotkey_type, hotkey_action;
+    int32_t partition_num = 0;
+    optind = 0;
+    while (true) {
+        int option_index = 0;
+        int c;
+        c = getopt_long(args.argc, args.argv, "a:p:t:c", long_options, 
&option_index);
+        if (c == -1)
+            break;
+        switch (c) {
+        case 'p':
+            partition_num = boost::lexical_cast<int32_t>(optarg);
+            break;
+        case 't':
+            hotkey_type = optarg;
+            break;
+        case 'a':
+            hotkey_action = optarg;

Review comment:
       Base previous comment, you should check and transfer param here, and 
that `detect_hotkey` only "detect_hotkey"

##########
File path: src/shell/commands/data_operations.cpp
##########
@@ -2664,3 +2665,45 @@ bool calculate_hash_value(command_executor *e, 
shell_context *sc, arguments args
     tp.output(std::cout);
     return true;
 }
+
+// TODO: (Tangyanzhao) merge 
hotspot_partition_calculator::send_hotkey_detect_request
+bool detect_hotkey(command_executor *e, shell_context *sc, arguments args)
+{
+    static struct option long_options[] = {{"partition_num", 
required_argument, 0, 'p'},
+                                           {"hotkey_type", required_argument, 
0, 't'},
+                                           {"action", required_argument, 0, 
'a'},
+                                           {0, 0, 0, 0}};
+
+    std::string hotkey_type, hotkey_action;
+    int32_t partition_num = 0;
+    optind = 0;
+    while (true) {
+        int option_index = 0;
+        int c;
+        c = getopt_long(args.argc, args.argv, "a:p:t:c", long_options, 
&option_index);
+        if (c == -1)
+            break;
+        switch (c) {
+        case 'p':
+            partition_num = boost::lexical_cast<int32_t>(optarg);
+            break;
+        case 't':
+            hotkey_type = optarg;
+            break;
+        case 'a':
+            hotkey_action = optarg;

Review comment:
       May just as the @acelyc111 comment: 
https://github.com/apache/incubator-pegasus/pull/605#discussion_r492069310, 
you'd better parse the string to `dsn::apps::hotkey_type::type` and 
`dsn::apps::hotkey_detect_action` here, and pass `detect_hotkey`

##########
File path: src/shell/commands/data_operations.cpp
##########
@@ -2664,3 +2665,45 @@ bool calculate_hash_value(command_executor *e, 
shell_context *sc, arguments args
     tp.output(std::cout);
     return true;
 }
+
+// TODO: (Tangyanzhao) merge 
hotspot_partition_calculator::send_hotkey_detect_request
+bool detect_hotkey(command_executor *e, shell_context *sc, arguments args)
+{
+    static struct option long_options[] = {{"partition_num", 
required_argument, 0, 'p'},
+                                           {"hotkey_type", required_argument, 
0, 't'},
+                                           {"action", required_argument, 0, 
'a'},
+                                           {0, 0, 0, 0}};
+
+    std::string hotkey_type, hotkey_action;
+    int32_t partition_num = 0;
+    optind = 0;
+    while (true) {
+        int option_index = 0;
+        int c;
+        c = getopt_long(args.argc, args.argv, "a:p:t:c", long_options, 
&option_index);
+        if (c == -1)
+            break;
+        switch (c) {
+        case 'p':
+            partition_num = boost::lexical_cast<int32_t>(optarg);
+            break;
+        case 't':
+            hotkey_type = optarg;
+            break;
+        case 'a':
+            hotkey_action = optarg;

Review comment:
       May just as the @acelyc111 comment: 
https://github.com/apache/incubator-pegasus/pull/605#discussion_r492069310, 
you'd better parse the string to `dsn::apps::hotkey_type::type` and 
`dsn::apps::hotkey_detect_action` here, and then pass `detect_hotkey`




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to