From 6054cfb35fd23b1397379cbec3391cd44f34623a Mon Sep 17 00:00:00 2001 From: tangyanzhao Date: Mon, 28 Sep 2020 15:54:34 +0800 Subject: [PATCH 01/13] refactor --- rdsn | 2 +- src/shell/command_helper.h | 3 +- src/shell/commands.h | 4 + src/shell/commands/detect_hotkey.cpp | 136 ++++++++++++++++++++++++++ src/shell/commands/disk_rebalance.cpp | 33 +------ src/shell/main.cpp | 10 ++ src/shell/validate_utils.cpp | 62 ++++++++++++ src/shell/validate_utils.h | 12 +++ 8 files changed, 228 insertions(+), 34 deletions(-) create mode 100644 src/shell/commands/detect_hotkey.cpp create mode 100644 src/shell/validate_utils.cpp create mode 100644 src/shell/validate_utils.h diff --git a/rdsn b/rdsn index fafbd599d1..69102a786f 160000 --- a/rdsn +++ b/rdsn @@ -1 +1 @@ -Subproject commit fafbd599d1df48b36bade4d08940ab79837aaf3b +Subproject commit 69102a786f3b888155bc18b8b6c58031c7d2fd98 diff --git a/src/shell/command_helper.h b/src/shell/command_helper.h index 4f7c280d28..f4969b930f 100644 --- a/src/shell/command_helper.h +++ b/src/shell/command_helper.h @@ -35,6 +35,7 @@ #include "command_executor.h" #include "command_utils.h" +#include "validate_utils.h" using namespace dsn::replication; @@ -1077,4 +1078,4 @@ inline bool get_storage_size_stat(shell_context *sc, app_storage_size_stat &st_s dsn::utils::time_ms_to_date_time(dsn_now_ms(), buf, sizeof(buf)); st_stat.timestamp = buf; return true; -} +} \ No newline at end of file diff --git a/src/shell/commands.h b/src/shell/commands.h index 27618f026b..92ede4c06a 100644 --- a/src/shell/commands.h +++ b/src/shell/commands.h @@ -262,3 +262,7 @@ bool pause_bulk_load(command_executor *e, shell_context *sc, arguments args); bool restart_bulk_load(command_executor *e, shell_context *sc, arguments args); bool cancel_bulk_load(command_executor *e, shell_context *sc, arguments args); + +// == detect hotkey (see 'commands/detect_hotkey.cpp') == // + +bool detect_hotkey(command_executor *e, shell_context *sc, arguments args); \ No newline at end of file diff --git a/src/shell/commands/detect_hotkey.cpp b/src/shell/commands/detect_hotkey.cpp new file mode 100644 index 0000000000..57e6daa44e --- /dev/null +++ b/src/shell/commands/detect_hotkey.cpp @@ -0,0 +1,136 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "shell/commands.h" +#include "shell/validate_utils.h" +#include + +bool generate_hotkey_request(dsn::replication::detect_hotkey_request &req, + const std::string &hotkey_action, + const std::string &hotkey_type, + int app_id, + int partition_index, + std::string &err_info) +{ + std::string hotkey_action_check = hotkey_action; + std::string hotkey_type_check = hotkey_type; + std::transform( + hotkey_type_check.begin(), hotkey_type_check.end(), hotkey_type_check.begin(), tolower); + std::transform(hotkey_action_check.begin(), + hotkey_action_check.end(), + hotkey_action_check.begin(), + ::tolower); + if (hotkey_type_check == "read") { + req.type = dsn::replication::hotkey_type::type::READ; + } else if (hotkey_type_check == "write") { + req.type = dsn::replication::hotkey_type::type::WRITE; + } else { + err_info = fmt::format("\"{}\" is an invalid hotkey type (should be 'read' or 'write')\n", + hotkey_type); + return false; + } + if (hotkey_action_check == "start") { + req.action = dsn::replication::detect_action::START; + } else if (hotkey_action_check == "stop") { + req.action = dsn::replication::detect_action::STOP; + } else { + err_info = + fmt::format("\"{}\" is an invalid hotkey detect action (should be 'start' or 'stop')\n", + hotkey_action); + return false; + } + req.pid = dsn::gpid(app_id, partition_index); + return true; +} + +// TODO: (Tangyanzhao) merge hotspot_partition_calculator::send_detect_hotkey_request +bool detect_hotkey(command_executor *e, shell_context *sc, arguments args) +{ + // detect_hotkey [-a|--app_id] [-p|--partition_index][-c|--hotkey_action][-t|--hotkey_type] + const std::set ¶ms = {"a", + "app_id", + "p", + "partition_index", + "c", + "hotkey_action", + "t", + "hotkey_type", + "d", + "address"}; + const std::set flags = {}; + argh::parser cmd(args.argc, args.argv, argh::parser::PREFER_PARAM_FOR_UNREG_OPTION); + if (!validate_cmd(cmd, params, flags)) { + return false; + } + int app_id; + if (!dsn::buf2int32(cmd({"-a", "--app_id"}).str(), app_id)) { + fmt::print( + stderr, + "\"{}\" is a invalid app_id, you should use `app` to check related information\n", + cmd({"-a", "--app_id"}).str()); + return false; + } + int partition_index; + if (!dsn::buf2int32(cmd({"-p", "--partition_index"}).str(), partition_index)) { + fmt::print(stderr, + "\"{}\" is a invalid partition index, you should use `app` to check related " + "information\n", + cmd({"-p", "--partition_index"}).str()); + return false; + } + std::string hotkey_action = cmd({"-c", "--hotkey_action"}).str(); + std::string hotkey_type = cmd({"-t", "--hotkey_type"}).str(); + dsn::rpc_address target_address; + std::string err_info; + + if (!validate_ip(sc, cmd({"-d", "--address"}).str(), target_address, err_info)) { + fmt::print(stderr, err_info); + return false; + } + + dsn::replication::detect_hotkey_request req; + if (!generate_hotkey_request( + req, hotkey_action, hotkey_type, app_id, partition_index, err_info)) { + fmt::print(stderr, err_info); + return false; + } + + detect_hotkey_response resp; + auto err = sc->ddl_client->detect_hotkey(dsn::rpc_address(target_address), req, resp); + if (err != dsn::ERR_OK) { + fmt::print(stderr, + "Hotkey detect rpc sending failed, in {}.{}, error_hint:{}\n", + app_id, + partition_index, + err.to_string()); + return false; + } + + if (resp.err != dsn::ERR_OK) { + fmt::print(stderr, + "Hotkey detect rpc performed failed, in {}.{}, error_hint:{} {}\n", + app_id, + partition_index, + resp.err, + resp.err_hint); + return false; + } + + fmt::print(stderr, "YES\n"); + + return true; +} \ No newline at end of file diff --git a/src/shell/commands/disk_rebalance.cpp b/src/shell/commands/disk_rebalance.cpp index 97462c9f8e..3bcc81a2d7 100644 --- a/src/shell/commands/disk_rebalance.cpp +++ b/src/shell/commands/disk_rebalance.cpp @@ -5,45 +5,14 @@ #include "shell/commands.h" #include "shell/argh.h" #include "shell/command_output.h" +#include "shell/validate_utils.h" #include #include #include #include -#include #include -bool validate_cmd(const argh::parser &cmd, - const std::set ¶ms, - const std::set &flags) -{ - if (cmd.size() > 1) { - fmt::print(stderr, "too many params!\n"); - return false; - } - - for (const auto ¶m : cmd.params()) { - if (params.find(param.first) == params.end()) { - fmt::print(stderr, "unknown param {} = {}\n", param.first, param.second); - return false; - } - } - - for (const auto &flag : cmd.flags()) { - if (params.find(flag) != params.end()) { - fmt::print(stderr, "missing value of {}\n", flag); - return false; - } - - if (flags.find(flag) == flags.end()) { - fmt::print(stderr, "unknown flag {}\n", flag); - return false; - } - } - - return true; -} - bool query_disk_info( shell_context *sc, const argh::parser &cmd, diff --git a/src/shell/main.cpp b/src/shell/main.cpp index ea2688b2ba..094b0b3837 100644 --- a/src/shell/main.cpp +++ b/src/shell/main.cpp @@ -474,6 +474,16 @@ static command_executor commands[] = { "<-a --app_name str> [-f --forced]", cancel_bulk_load, }, + { + "detect_hotkey", + "start or stop hotkey detection on the replica", + "<-a|--app_id str> " + "<-p|--partition_index num> " + "<-t|--hotkey_type read|write> " + "<-c|--detect_action start|stop> " + "<-d|--address str>", + detect_hotkey, + }, { "exit", "exit shell", "", exit_shell, }, diff --git a/src/shell/validate_utils.cpp b/src/shell/validate_utils.cpp new file mode 100644 index 0000000000..5b00bdbfe7 --- /dev/null +++ b/src/shell/validate_utils.cpp @@ -0,0 +1,62 @@ +// +// Created by smilencer on 2020/9/28. +// + +#include "shell/validate_utils.h" + +bool validate_cmd(const argh::parser &cmd, + const std::set ¶ms, + const std::set &flags) +{ + if (cmd.size() > 1) { + fmt::print(stderr, "too many params!\n"); + return false; + } + + for (const auto ¶m : cmd.params()) { + if (params.find(param.first) == params.end()) { + fmt::print(stderr, "unknown param {} = {}\n", param.first, param.second); + return false; + } + } + + for (const auto &flag : cmd.flags()) { + if (params.find(flag) != params.end()) { + fmt::print(stderr, "missing value of {}\n", flag); + return false; + } + + if (flags.find(flag) == flags.end()) { + fmt::print(stderr, "unknown flag {}\n", flag); + return false; + } + } + + return true; +} + +bool validate_ip(shell_context *sc, + const std::string &ip_str, + /*out*/ dsn::rpc_address &target_address, + /*out*/ std::string &err_info) +{ + std::map nodes; + auto error = sc->ddl_client->list_nodes(::dsn::replication::node_status::NS_INVALID, nodes); + if (error != dsn::ERR_OK) { + err_info = fmt::format("list nodes failed, error={} \n", error.to_string()); + return false; + } + + bool not_find_ip = true; + for (const auto &node : nodes) { + if (ip_str == node.first.to_std_string()) { + target_address = node.first; + not_find_ip = false; + } + } + if (not_find_ip) { + err_info = fmt::format("invalid ip, error={} \n", ip_str); + return false; + } + return true; +} \ No newline at end of file diff --git a/src/shell/validate_utils.h b/src/shell/validate_utils.h new file mode 100644 index 0000000000..26909a2a79 --- /dev/null +++ b/src/shell/validate_utils.h @@ -0,0 +1,12 @@ +#include "shell/argh.h" +#include +#include "command_executor.h" + +bool validate_cmd(const argh::parser &cmd, + const std::set ¶ms, + const std::set &flags); + +bool validate_ip(shell_context *sc, + const std::string &ip_str, + /*out*/ dsn::rpc_address &target_address, + /*out*/ std::string &err_info); \ No newline at end of file From 10895e6c330a88861ec2d3b9d722117f89cd1a0e Mon Sep 17 00:00:00 2001 From: tangyanzhao Date: Mon, 28 Sep 2020 16:09:34 +0800 Subject: [PATCH 02/13] test --- rdsn | 2 +- src/shell/command_helper.h | 2 +- src/shell/commands.h | 2 +- src/shell/commands/detect_hotkey.cpp | 4 ++-- src/shell/validate_utils.cpp | 17 +++++++++++++++-- src/shell/validate_utils.h | 19 ++++++++++++++++++- 6 files changed, 38 insertions(+), 8 deletions(-) diff --git a/rdsn b/rdsn index 69102a786f..fafbd599d1 160000 --- a/rdsn +++ b/rdsn @@ -1 +1 @@ -Subproject commit 69102a786f3b888155bc18b8b6c58031c7d2fd98 +Subproject commit fafbd599d1df48b36bade4d08940ab79837aaf3b diff --git a/src/shell/command_helper.h b/src/shell/command_helper.h index f4969b930f..1eeb51f4c2 100644 --- a/src/shell/command_helper.h +++ b/src/shell/command_helper.h @@ -1078,4 +1078,4 @@ inline bool get_storage_size_stat(shell_context *sc, app_storage_size_stat &st_s dsn::utils::time_ms_to_date_time(dsn_now_ms(), buf, sizeof(buf)); st_stat.timestamp = buf; return true; -} \ No newline at end of file +} diff --git a/src/shell/commands.h b/src/shell/commands.h index 92ede4c06a..dfe85301f3 100644 --- a/src/shell/commands.h +++ b/src/shell/commands.h @@ -265,4 +265,4 @@ bool cancel_bulk_load(command_executor *e, shell_context *sc, arguments args); // == detect hotkey (see 'commands/detect_hotkey.cpp') == // -bool detect_hotkey(command_executor *e, shell_context *sc, arguments args); \ No newline at end of file +bool detect_hotkey(command_executor *e, shell_context *sc, arguments args); diff --git a/src/shell/commands/detect_hotkey.cpp b/src/shell/commands/detect_hotkey.cpp index 57e6daa44e..c66aedc8ee 100644 --- a/src/shell/commands/detect_hotkey.cpp +++ b/src/shell/commands/detect_hotkey.cpp @@ -33,7 +33,7 @@ bool generate_hotkey_request(dsn::replication::detect_hotkey_request &req, std::transform(hotkey_action_check.begin(), hotkey_action_check.end(), hotkey_action_check.begin(), - ::tolower); + tolower); if (hotkey_type_check == "read") { req.type = dsn::replication::hotkey_type::type::READ; } else if (hotkey_type_check == "write") { @@ -133,4 +133,4 @@ bool detect_hotkey(command_executor *e, shell_context *sc, arguments args) fmt::print(stderr, "YES\n"); return true; -} \ No newline at end of file +} diff --git a/src/shell/validate_utils.cpp b/src/shell/validate_utils.cpp index 5b00bdbfe7..ee7739b1cd 100644 --- a/src/shell/validate_utils.cpp +++ b/src/shell/validate_utils.cpp @@ -1,6 +1,19 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at // -// Created by smilencer on 2020/9/28. +// http://www.apache.org/licenses/LICENSE-2.0 // +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. #include "shell/validate_utils.h" @@ -59,4 +72,4 @@ bool validate_ip(shell_context *sc, return false; } return true; -} \ No newline at end of file +} diff --git a/src/shell/validate_utils.h b/src/shell/validate_utils.h index 26909a2a79..8cf39b386b 100644 --- a/src/shell/validate_utils.h +++ b/src/shell/validate_utils.h @@ -1,3 +1,20 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + #include "shell/argh.h" #include #include "command_executor.h" @@ -9,4 +26,4 @@ bool validate_cmd(const argh::parser &cmd, bool validate_ip(shell_context *sc, const std::string &ip_str, /*out*/ dsn::rpc_address &target_address, - /*out*/ std::string &err_info); \ No newline at end of file + /*out*/ std::string &err_info); From be33ccc6fa59a4e92bc257d4fc7e59e13aa14975 Mon Sep 17 00:00:00 2001 From: Smilencer <527646889@qq.com> Date: Mon, 28 Sep 2020 16:21:39 +0800 Subject: [PATCH 03/13] Update command_helper.h --- src/shell/command_helper.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/shell/command_helper.h b/src/shell/command_helper.h index 1eeb51f4c2..4f7c280d28 100644 --- a/src/shell/command_helper.h +++ b/src/shell/command_helper.h @@ -35,7 +35,6 @@ #include "command_executor.h" #include "command_utils.h" -#include "validate_utils.h" using namespace dsn::replication; From 673f51747e64787853c78c5f88248f9a08f980ba Mon Sep 17 00:00:00 2001 From: tangyanzhao Date: Mon, 28 Sep 2020 16:46:07 +0800 Subject: [PATCH 04/13] test --- src/shell/commands/detect_hotkey.cpp | 16 ++---- src/shell/validate_utils.cpp | 75 ---------------------------- src/shell/validate_utils.h | 54 +++++++++++++++++++- 3 files changed, 56 insertions(+), 89 deletions(-) delete mode 100644 src/shell/validate_utils.cpp diff --git a/src/shell/commands/detect_hotkey.cpp b/src/shell/commands/detect_hotkey.cpp index c66aedc8ee..42fad136d3 100644 --- a/src/shell/commands/detect_hotkey.cpp +++ b/src/shell/commands/detect_hotkey.cpp @@ -26,26 +26,18 @@ bool generate_hotkey_request(dsn::replication::detect_hotkey_request &req, int partition_index, std::string &err_info) { - std::string hotkey_action_check = hotkey_action; - std::string hotkey_type_check = hotkey_type; - std::transform( - hotkey_type_check.begin(), hotkey_type_check.end(), hotkey_type_check.begin(), tolower); - std::transform(hotkey_action_check.begin(), - hotkey_action_check.end(), - hotkey_action_check.begin(), - tolower); - if (hotkey_type_check == "read") { + if (std::strcasecmp(hotkey_type, "read")) { req.type = dsn::replication::hotkey_type::type::READ; - } else if (hotkey_type_check == "write") { + } else if (std::strcasecmp(hotkey_type, "write")) { req.type = dsn::replication::hotkey_type::type::WRITE; } else { err_info = fmt::format("\"{}\" is an invalid hotkey type (should be 'read' or 'write')\n", hotkey_type); return false; } - if (hotkey_action_check == "start") { + if (std::strcasecmp(hotkey_action, "start")) { req.action = dsn::replication::detect_action::START; - } else if (hotkey_action_check == "stop") { + } else if (std::strcasecmp(hotkey_action, "stop")) { req.action = dsn::replication::detect_action::STOP; } else { err_info = diff --git a/src/shell/validate_utils.cpp b/src/shell/validate_utils.cpp deleted file mode 100644 index ee7739b1cd..0000000000 --- a/src/shell/validate_utils.cpp +++ /dev/null @@ -1,75 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -#include "shell/validate_utils.h" - -bool validate_cmd(const argh::parser &cmd, - const std::set ¶ms, - const std::set &flags) -{ - if (cmd.size() > 1) { - fmt::print(stderr, "too many params!\n"); - return false; - } - - for (const auto ¶m : cmd.params()) { - if (params.find(param.first) == params.end()) { - fmt::print(stderr, "unknown param {} = {}\n", param.first, param.second); - return false; - } - } - - for (const auto &flag : cmd.flags()) { - if (params.find(flag) != params.end()) { - fmt::print(stderr, "missing value of {}\n", flag); - return false; - } - - if (flags.find(flag) == flags.end()) { - fmt::print(stderr, "unknown flag {}\n", flag); - return false; - } - } - - return true; -} - -bool validate_ip(shell_context *sc, - const std::string &ip_str, - /*out*/ dsn::rpc_address &target_address, - /*out*/ std::string &err_info) -{ - std::map nodes; - auto error = sc->ddl_client->list_nodes(::dsn::replication::node_status::NS_INVALID, nodes); - if (error != dsn::ERR_OK) { - err_info = fmt::format("list nodes failed, error={} \n", error.to_string()); - return false; - } - - bool not_find_ip = true; - for (const auto &node : nodes) { - if (ip_str == node.first.to_std_string()) { - target_address = node.first; - not_find_ip = false; - } - } - if (not_find_ip) { - err_info = fmt::format("invalid ip, error={} \n", ip_str); - return false; - } - return true; -} diff --git a/src/shell/validate_utils.h b/src/shell/validate_utils.h index 8cf39b386b..a93e300a10 100644 --- a/src/shell/validate_utils.h +++ b/src/shell/validate_utils.h @@ -15,15 +15,65 @@ // specific language governing permissions and limitations // under the License. +#pragma once + #include "shell/argh.h" #include #include "command_executor.h" bool validate_cmd(const argh::parser &cmd, const std::set ¶ms, - const std::set &flags); + const std::set &flags) +{ + if (cmd.size() > 1) { + fmt::print(stderr, "too many params!\n"); + return false; + } + + for (const auto ¶m : cmd.params()) { + if (params.find(param.first) == params.end()) { + fmt::print(stderr, "unknown param {} = {}\n", param.first, param.second); + return false; + } + } + + for (const auto &flag : cmd.flags()) { + if (params.find(flag) != params.end()) { + fmt::print(stderr, "missing value of {}\n", flag); + return false; + } + + if (flags.find(flag) == flags.end()) { + fmt::print(stderr, "unknown flag {}\n", flag); + return false; + } + } + + return true; +} bool validate_ip(shell_context *sc, const std::string &ip_str, /*out*/ dsn::rpc_address &target_address, - /*out*/ std::string &err_info); + /*out*/ std::string &err_info) +{ + std::map nodes; + auto error = sc->ddl_client->list_nodes(::dsn::replication::node_status::NS_INVALID, nodes); + if (error != dsn::ERR_OK) { + err_info = fmt::format("list nodes failed, error={} \n", error.to_string()); + return false; + } + + bool not_find_ip = true; + for (const auto &node : nodes) { + if (ip_str == node.first.to_std_string()) { + target_address = node.first; + not_find_ip = false; + } + } + if (not_find_ip) { + err_info = fmt::format("invalid ip, error={} \n", ip_str); + return false; + } + return true; +} From 30553b9eeb6224856fac1bf1f523589bb67c5dbe Mon Sep 17 00:00:00 2001 From: tangyanzhao Date: Mon, 28 Sep 2020 16:53:47 +0800 Subject: [PATCH 05/13] test --- src/shell/commands/detect_hotkey.cpp | 6 ++++-- src/shell/validate_utils.h | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/shell/commands/detect_hotkey.cpp b/src/shell/commands/detect_hotkey.cpp index 42fad136d3..8e07835c4a 100644 --- a/src/shell/commands/detect_hotkey.cpp +++ b/src/shell/commands/detect_hotkey.cpp @@ -76,6 +76,7 @@ bool detect_hotkey(command_executor *e, shell_context *sc, arguments args) cmd({"-a", "--app_id"}).str()); return false; } + int partition_index; if (!dsn::buf2int32(cmd({"-p", "--partition_index"}).str(), partition_index)) { fmt::print(stderr, @@ -84,8 +85,7 @@ bool detect_hotkey(command_executor *e, shell_context *sc, arguments args) cmd({"-p", "--partition_index"}).str()); return false; } - std::string hotkey_action = cmd({"-c", "--hotkey_action"}).str(); - std::string hotkey_type = cmd({"-t", "--hotkey_type"}).str(); + dsn::rpc_address target_address; std::string err_info; @@ -94,6 +94,8 @@ bool detect_hotkey(command_executor *e, shell_context *sc, arguments args) return false; } + std::string hotkey_action = cmd({"-c", "--hotkey_action"}).str(); + std::string hotkey_type = cmd({"-t", "--hotkey_type"}).str(); dsn::replication::detect_hotkey_request req; if (!generate_hotkey_request( req, hotkey_action, hotkey_type, app_id, partition_index, err_info)) { diff --git a/src/shell/validate_utils.h b/src/shell/validate_utils.h index a93e300a10..55cd4de76a 100644 --- a/src/shell/validate_utils.h +++ b/src/shell/validate_utils.h @@ -69,6 +69,7 @@ bool validate_ip(shell_context *sc, if (ip_str == node.first.to_std_string()) { target_address = node.first; not_find_ip = false; + break; } } if (not_find_ip) { From 5c84a520aac93ea968fa144e722d13f637237fb5 Mon Sep 17 00:00:00 2001 From: tangyanzhao Date: Mon, 28 Sep 2020 19:14:45 +0800 Subject: [PATCH 06/13] update --- rdsn | 2 +- src/shell/CMakeLists.txt | 1 + src/shell/command_utils.h | 37 +++++++++++++ src/shell/commands.h | 1 - src/shell/commands/detect_hotkey.cpp | 22 ++++---- src/shell/commands/disk_rebalance.cpp | 2 +- src/shell/validate_utils.h | 80 --------------------------- 7 files changed, 52 insertions(+), 93 deletions(-) delete mode 100644 src/shell/validate_utils.h diff --git a/rdsn b/rdsn index fafbd599d1..69102a786f 160000 --- a/rdsn +++ b/rdsn @@ -1 +1 @@ -Subproject commit fafbd599d1df48b36bade4d08940ab79837aaf3b +Subproject commit 69102a786f3b888155bc18b8b6c58031c7d2fd98 diff --git a/src/shell/CMakeLists.txt b/src/shell/CMakeLists.txt index a92302faf6..0ed4cf21fc 100644 --- a/src/shell/CMakeLists.txt +++ b/src/shell/CMakeLists.txt @@ -43,6 +43,7 @@ set(MY_BOOST_LIBS SET(CMAKE_INSTALL_RPATH ".") SET(CMAKE_BUILD_WITH_INSTALL_RPATH TRUE) + add_definitions(-Wno-attributes) dsn_add_executable() diff --git a/src/shell/command_utils.h b/src/shell/command_utils.h index ea534a6f85..d576d6d99e 100644 --- a/src/shell/command_utils.h +++ b/src/shell/command_utils.h @@ -5,6 +5,43 @@ #pragma once #include +#include +#include + +#include "shell/argh.h" +#include +#include "command_executor.h" + +inline bool validate_cmd(const argh::parser &cmd, + const std::set ¶ms, + const std::set &flags) +{ + if (cmd.size() > 1) { + fmt::print(stderr, "too many params!\n"); + return false; + } + + for (const auto ¶m : cmd.params()) { + if (params.find(param.first) == params.end()) { + fmt::print(stderr, "unknown param {} = {}\n", param.first, param.second); + return false; + } + } + + for (const auto &flag : cmd.flags()) { + if (params.find(flag) != params.end()) { + fmt::print(stderr, "missing value of {}\n", flag); + return false; + } + + if (flags.find(flag) == flags.end()) { + fmt::print(stderr, "unknown flag {}\n", flag); + return false; + } + } + + return true; +} #define verify_logged(exp, ...) \ do { \ diff --git a/src/shell/commands.h b/src/shell/commands.h index dfe85301f3..af29c2cc34 100644 --- a/src/shell/commands.h +++ b/src/shell/commands.h @@ -25,7 +25,6 @@ #include #include "command_executor.h" -#include "command_utils.h" #include "command_helper.h" #include "args.h" diff --git a/src/shell/commands/detect_hotkey.cpp b/src/shell/commands/detect_hotkey.cpp index 8e07835c4a..1e703728d9 100644 --- a/src/shell/commands/detect_hotkey.cpp +++ b/src/shell/commands/detect_hotkey.cpp @@ -16,7 +16,8 @@ // under the License. #include "shell/commands.h" -#include "shell/validate_utils.h" +#include "shell/argh.h" +#include "shell/command_helper.h" #include bool generate_hotkey_request(dsn::replication::detect_hotkey_request &req, @@ -26,18 +27,18 @@ bool generate_hotkey_request(dsn::replication::detect_hotkey_request &req, int partition_index, std::string &err_info) { - if (std::strcasecmp(hotkey_type, "read")) { + if (strcasecmp(hotkey_type.c_str(), "read")) { req.type = dsn::replication::hotkey_type::type::READ; - } else if (std::strcasecmp(hotkey_type, "write")) { + } else if (strcasecmp(hotkey_type.c_str(), "write")) { req.type = dsn::replication::hotkey_type::type::WRITE; } else { err_info = fmt::format("\"{}\" is an invalid hotkey type (should be 'read' or 'write')\n", hotkey_type); return false; } - if (std::strcasecmp(hotkey_action, "start")) { + if (strcasecmp(hotkey_action.c_str(), "start")) { req.action = dsn::replication::detect_action::START; - } else if (std::strcasecmp(hotkey_action, "stop")) { + } else if (strcasecmp(hotkey_action.c_str(), "stop")) { req.action = dsn::replication::detect_action::STOP; } else { err_info = @@ -52,7 +53,8 @@ bool generate_hotkey_request(dsn::replication::detect_hotkey_request &req, // TODO: (Tangyanzhao) merge hotspot_partition_calculator::send_detect_hotkey_request bool detect_hotkey(command_executor *e, shell_context *sc, arguments args) { - // detect_hotkey [-a|--app_id] [-p|--partition_index][-c|--hotkey_action][-t|--hotkey_type] + // detect_hotkey + // [-a|--app_id][-p|--partition_index][-c|--hotkey_action][-t|--hotkey_type][-d|--address] const std::set ¶ms = {"a", "app_id", "p", @@ -87,13 +89,13 @@ bool detect_hotkey(command_executor *e, shell_context *sc, arguments args) } dsn::rpc_address target_address; - std::string err_info; - - if (!validate_ip(sc, cmd({"-d", "--address"}).str(), target_address, err_info)) { - fmt::print(stderr, err_info); + std::string ip_str = cmd({"-d", "--address"}).str(); + if (!target_address.from_string_ipv4(ip_str.c_str())) { + fmt::print("invalid ip, error={} \n", ip_str); return false; } + std::string err_info; std::string hotkey_action = cmd({"-c", "--hotkey_action"}).str(); std::string hotkey_type = cmd({"-t", "--hotkey_type"}).str(); dsn::replication::detect_hotkey_request req; diff --git a/src/shell/commands/disk_rebalance.cpp b/src/shell/commands/disk_rebalance.cpp index 3bcc81a2d7..164c4536b0 100644 --- a/src/shell/commands/disk_rebalance.cpp +++ b/src/shell/commands/disk_rebalance.cpp @@ -5,7 +5,7 @@ #include "shell/commands.h" #include "shell/argh.h" #include "shell/command_output.h" -#include "shell/validate_utils.h" +#include "shell/command_helper.h" #include #include diff --git a/src/shell/validate_utils.h b/src/shell/validate_utils.h deleted file mode 100644 index 55cd4de76a..0000000000 --- a/src/shell/validate_utils.h +++ /dev/null @@ -1,80 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -#pragma once - -#include "shell/argh.h" -#include -#include "command_executor.h" - -bool validate_cmd(const argh::parser &cmd, - const std::set ¶ms, - const std::set &flags) -{ - if (cmd.size() > 1) { - fmt::print(stderr, "too many params!\n"); - return false; - } - - for (const auto ¶m : cmd.params()) { - if (params.find(param.first) == params.end()) { - fmt::print(stderr, "unknown param {} = {}\n", param.first, param.second); - return false; - } - } - - for (const auto &flag : cmd.flags()) { - if (params.find(flag) != params.end()) { - fmt::print(stderr, "missing value of {}\n", flag); - return false; - } - - if (flags.find(flag) == flags.end()) { - fmt::print(stderr, "unknown flag {}\n", flag); - return false; - } - } - - return true; -} - -bool validate_ip(shell_context *sc, - const std::string &ip_str, - /*out*/ dsn::rpc_address &target_address, - /*out*/ std::string &err_info) -{ - std::map nodes; - auto error = sc->ddl_client->list_nodes(::dsn::replication::node_status::NS_INVALID, nodes); - if (error != dsn::ERR_OK) { - err_info = fmt::format("list nodes failed, error={} \n", error.to_string()); - return false; - } - - bool not_find_ip = true; - for (const auto &node : nodes) { - if (ip_str == node.first.to_std_string()) { - target_address = node.first; - not_find_ip = false; - break; - } - } - if (not_find_ip) { - err_info = fmt::format("invalid ip, error={} \n", ip_str); - return false; - } - return true; -} From aca2321d8955104db46e926bd9fbed8234bd21c8 Mon Sep 17 00:00:00 2001 From: Smilencer <527646889@qq.com> Date: Tue, 29 Sep 2020 10:16:18 +0800 Subject: [PATCH 07/13] Update CMakeLists.txt --- src/shell/CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/src/shell/CMakeLists.txt b/src/shell/CMakeLists.txt index 0ed4cf21fc..a92302faf6 100644 --- a/src/shell/CMakeLists.txt +++ b/src/shell/CMakeLists.txt @@ -43,7 +43,6 @@ set(MY_BOOST_LIBS SET(CMAKE_INSTALL_RPATH ".") SET(CMAKE_BUILD_WITH_INSTALL_RPATH TRUE) - add_definitions(-Wno-attributes) dsn_add_executable() From 120b123cf0e01020b77ae11da591321767cbebba Mon Sep 17 00:00:00 2001 From: tangyanzhao Date: Tue, 29 Sep 2020 10:58:01 +0800 Subject: [PATCH 08/13] fix fix --- src/shell/commands/detect_hotkey.cpp | 31 ++++++++++++++-------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/shell/commands/detect_hotkey.cpp b/src/shell/commands/detect_hotkey.cpp index 1e703728d9..d4f6c47d4b 100644 --- a/src/shell/commands/detect_hotkey.cpp +++ b/src/shell/commands/detect_hotkey.cpp @@ -27,18 +27,18 @@ bool generate_hotkey_request(dsn::replication::detect_hotkey_request &req, int partition_index, std::string &err_info) { - if (strcasecmp(hotkey_type.c_str(), "read")) { + if (!strcasecmp(hotkey_type.c_str(), "read")) { req.type = dsn::replication::hotkey_type::type::READ; - } else if (strcasecmp(hotkey_type.c_str(), "write")) { + } else if (!strcasecmp(hotkey_type.c_str(), "write")) { req.type = dsn::replication::hotkey_type::type::WRITE; } else { err_info = fmt::format("\"{}\" is an invalid hotkey type (should be 'read' or 'write')\n", hotkey_type); return false; } - if (strcasecmp(hotkey_action.c_str(), "start")) { + if (!strcasecmp(hotkey_action.c_str(), "start")) { req.action = dsn::replication::detect_action::START; - } else if (strcasecmp(hotkey_action.c_str(), "stop")) { + } else if (!strcasecmp(hotkey_action.c_str(), "stop")) { req.action = dsn::replication::detect_action::STOP; } else { err_info = @@ -55,21 +55,22 @@ bool detect_hotkey(command_executor *e, shell_context *sc, arguments args) { // detect_hotkey // [-a|--app_id][-p|--partition_index][-c|--hotkey_action][-t|--hotkey_type][-d|--address] - const std::set ¶ms = {"a", - "app_id", - "p", - "partition_index", - "c", - "hotkey_action", - "t", - "hotkey_type", - "d", - "address"}; + const std::set params = {"a", + "app_id", + "p", + "partition_index", + "c", + "hotkey_action", + "t", + "hotkey_type", + "d", + "address"}; const std::set flags = {}; argh::parser cmd(args.argc, args.argv, argh::parser::PREFER_PARAM_FOR_UNREG_OPTION); if (!validate_cmd(cmd, params, flags)) { return false; } + int app_id; if (!dsn::buf2int32(cmd({"-a", "--app_id"}).str(), app_id)) { fmt::print( @@ -126,7 +127,5 @@ bool detect_hotkey(command_executor *e, shell_context *sc, arguments args) return false; } - fmt::print(stderr, "YES\n"); - return true; } From 5ed432c30510ff6a2d78c5fcf92f3cb490e0eae0 Mon Sep 17 00:00:00 2001 From: tangyanzhao Date: Tue, 29 Sep 2020 13:45:39 +0800 Subject: [PATCH 09/13] format format --- src/shell/command_utils.h | 1 - src/shell/commands/detect_hotkey.cpp | 4 ++-- src/shell/commands/disk_rebalance.cpp | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/shell/command_utils.h b/src/shell/command_utils.h index d576d6d99e..2aa480b9f3 100644 --- a/src/shell/command_utils.h +++ b/src/shell/command_utils.h @@ -10,7 +10,6 @@ #include "shell/argh.h" #include -#include "command_executor.h" inline bool validate_cmd(const argh::parser &cmd, const std::set ¶ms, diff --git a/src/shell/commands/detect_hotkey.cpp b/src/shell/commands/detect_hotkey.cpp index d4f6c47d4b..0f565cfa4e 100644 --- a/src/shell/commands/detect_hotkey.cpp +++ b/src/shell/commands/detect_hotkey.cpp @@ -17,7 +17,6 @@ #include "shell/commands.h" #include "shell/argh.h" -#include "shell/command_helper.h" #include bool generate_hotkey_request(dsn::replication::detect_hotkey_request &req, @@ -36,6 +35,7 @@ bool generate_hotkey_request(dsn::replication::detect_hotkey_request &req, hotkey_type); return false; } + if (!strcasecmp(hotkey_action.c_str(), "start")) { req.action = dsn::replication::detect_action::START; } else if (!strcasecmp(hotkey_action.c_str(), "stop")) { @@ -54,7 +54,7 @@ bool generate_hotkey_request(dsn::replication::detect_hotkey_request &req, bool detect_hotkey(command_executor *e, shell_context *sc, arguments args) { // detect_hotkey - // [-a|--app_id][-p|--partition_index][-c|--hotkey_action][-t|--hotkey_type][-d|--address] + // <-a|--app_id><-p|--partition_index><-c|--hotkey_action><-t|--hotkey_type><-d|--address> const std::set params = {"a", "app_id", "p", diff --git a/src/shell/commands/disk_rebalance.cpp b/src/shell/commands/disk_rebalance.cpp index 164c4536b0..6ced3ddbb0 100644 --- a/src/shell/commands/disk_rebalance.cpp +++ b/src/shell/commands/disk_rebalance.cpp @@ -5,7 +5,6 @@ #include "shell/commands.h" #include "shell/argh.h" #include "shell/command_output.h" -#include "shell/command_helper.h" #include #include From 91e5bbdb507f58025fcc36c52f548f875ff66316 Mon Sep 17 00:00:00 2001 From: Smilencer <527646889@qq.com> Date: Tue, 29 Sep 2020 01:22:16 -0500 Subject: [PATCH 10/13] Update src/shell/main.cpp Co-authored-by: Yingchun Lai <405403881@qq.com> --- src/shell/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shell/main.cpp b/src/shell/main.cpp index 094b0b3837..d733f4a098 100644 --- a/src/shell/main.cpp +++ b/src/shell/main.cpp @@ -476,7 +476,7 @@ static command_executor commands[] = { }, { "detect_hotkey", - "start or stop hotkey detection on the replica", + "start or stop hotkey detection on a replica of a replica server", "<-a|--app_id str> " "<-p|--partition_index num> " "<-t|--hotkey_type read|write> " From fa1ce688a099853cedbea171400553fa6dc72af5 Mon Sep 17 00:00:00 2001 From: Smilencer <527646889@qq.com> Date: Tue, 29 Sep 2020 01:22:27 -0500 Subject: [PATCH 11/13] Update src/shell/commands/detect_hotkey.cpp Co-authored-by: Yingchun Lai <405403881@qq.com> --- src/shell/commands/detect_hotkey.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shell/commands/detect_hotkey.cpp b/src/shell/commands/detect_hotkey.cpp index 0f565cfa4e..1b8951172e 100644 --- a/src/shell/commands/detect_hotkey.cpp +++ b/src/shell/commands/detect_hotkey.cpp @@ -92,7 +92,7 @@ bool detect_hotkey(command_executor *e, shell_context *sc, arguments args) dsn::rpc_address target_address; std::string ip_str = cmd({"-d", "--address"}).str(); if (!target_address.from_string_ipv4(ip_str.c_str())) { - fmt::print("invalid ip, error={} \n", ip_str); + fmt::print("invalid ip, error={}\n", ip_str); return false; } From a545601733ef407a70c1953e3a04f94f683ac82b Mon Sep 17 00:00:00 2001 From: Smilencer <527646889@qq.com> Date: Tue, 29 Sep 2020 01:22:42 -0500 Subject: [PATCH 12/13] Update src/shell/main.cpp Co-authored-by: Yingchun Lai <405403881@qq.com> --- src/shell/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shell/main.cpp b/src/shell/main.cpp index d733f4a098..c747556615 100644 --- a/src/shell/main.cpp +++ b/src/shell/main.cpp @@ -477,7 +477,7 @@ static command_executor commands[] = { { "detect_hotkey", "start or stop hotkey detection on a replica of a replica server", - "<-a|--app_id str> " + "<-a|--app_id num> " "<-p|--partition_index num> " "<-t|--hotkey_type read|write> " "<-c|--detect_action start|stop> " From 2fcc752ea1514d88dc1fa9ddc91a85fd3a118c3a Mon Sep 17 00:00:00 2001 From: tangyanzhao Date: Tue, 29 Sep 2020 14:23:43 +0800 Subject: [PATCH 13/13] update --- src/shell/commands/detect_hotkey.cpp | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/shell/commands/detect_hotkey.cpp b/src/shell/commands/detect_hotkey.cpp index 0f565cfa4e..ed302b4bec 100644 --- a/src/shell/commands/detect_hotkey.cpp +++ b/src/shell/commands/detect_hotkey.cpp @@ -54,7 +54,8 @@ bool generate_hotkey_request(dsn::replication::detect_hotkey_request &req, bool detect_hotkey(command_executor *e, shell_context *sc, arguments args) { // detect_hotkey - // <-a|--app_id><-p|--partition_index><-c|--hotkey_action><-t|--hotkey_type><-d|--address> + // <-a|--app_id str><-p|--partition_index num><-t|--hotkey_type read|write> + // <-c|--detect_action start|stop><-d|--address str> const std::set params = {"a", "app_id", "p", @@ -73,19 +74,13 @@ bool detect_hotkey(command_executor *e, shell_context *sc, arguments args) int app_id; if (!dsn::buf2int32(cmd({"-a", "--app_id"}).str(), app_id)) { - fmt::print( - stderr, - "\"{}\" is a invalid app_id, you should use `app` to check related information\n", - cmd({"-a", "--app_id"}).str()); + fmt::print(stderr, "\"{}\" is an invalid num\n", cmd({"-a", "--app_id"}).str()); return false; } int partition_index; if (!dsn::buf2int32(cmd({"-p", "--partition_index"}).str(), partition_index)) { - fmt::print(stderr, - "\"{}\" is a invalid partition index, you should use `app` to check related " - "information\n", - cmd({"-p", "--partition_index"}).str()); + fmt::print(stderr, "\"{}\" is an invalid num\n", cmd({"-p", "--partition_index"}).str()); return false; }