You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by GitBox <gi...@apache.org> on 2020/09/21 14:02:47 UTC

[GitHub] [incubator-pegasus] acelyc111 commented on a change in pull request #605: feat(hotspot): add a function to start hotkey detecting in shell commands

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



##########
File path: src/client_lib/pegasus_client_impl.h
##########
@@ -221,6 +221,10 @@ class pegasus_client_impl : public pegasus_client
                                  const scan_options &options,
                                  async_get_unordered_scanners_callback_t &&callback) override;
 
+    virtual bool detect_hotkey(const std::string &hotkey_type,

Review comment:
       `override` is present, `virtual ` can be removed.

##########
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'},

Review comment:
       Better to use argh::parser, refer to src/shell/commands/disk_rebalance.cpp

##########
File path: src/client_lib/pegasus_client_impl.h
##########
@@ -221,6 +221,10 @@ class pegasus_client_impl : public pegasus_client
                                  const scan_options &options,
                                  async_get_unordered_scanners_callback_t &&callback) override;
 
+    virtual bool detect_hotkey(const std::string &hotkey_type,

Review comment:
       Better to use dsn::apps::hotkey_type::type and dsn::apps::hotkey_detect_action as parameter, then no need to parse from string.

##########
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";

Review comment:
       append hotkey_type's value in error message.

##########
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;
+            break;
+        default:
+            return false;
+        }
+    }
+
+    std::string err_info;
+    bool state = sc->pg_client->detect_hotkey(hotkey_type, hotkey_action, partition_num, err_info);
+    if (!state) {
+        fprintf(stderr, "%s\n", err_info.c_str());
+    } else {
+        std::cout << hotkey_action << " detecting hotkey succeed" << std::endl;

Review comment:
       Use std::cout and std::cerr in pair.

##########
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";

Review comment:
       partition_hash is an uint64_t

##########
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";

Review comment:
       append hotkey_action's value in error message.




----------------------------------------------------------------
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:
users@infra.apache.org



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