hycdong commented on a change in pull request #605:
URL: https://github.com/apache/incubator-pegasus/pull/605#discussion_r493221631
##########
File path: src/shell/commands/detect_hotkey.cpp
##########
@@ -0,0 +1,116 @@
+#include "shell/commands.h"
+#include "shell/argh.h"
+#include "server/pegasus_read_service.h"
+
+bool validate_cmd(const argh::parser &cmd, const std::set<std::string> ¶ms)
+{
+ 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;
+ }
+ }
+
+ return true;
+}
+
+bool generate_hotkey_request(dsn::apps::hotkey_detect_request &req,
+ const std::string &hotkey_action,
+ const std::string &hotkey_type)
+{
+ 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::apps::hotkey_type::type::READ;
+ } else if (hotkey_type_check == "write") {
+ req.type = dsn::apps::hotkey_type::type::WRITE;
+ } else {
+ fmt::print(stderr, "\"{}\" is a invalid hotkey type (read or write)
\n", hotkey_type);
+ return false;
+ }
+ 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 {
+ fmt::print(
+ stderr, "\"{}\" is a invalid hotkey detect action (start or stop)
\n", hotkey_action);
+ return false;
+ }
+ return true;
+}
+
+// TODO: (Tangyanzhao) merge
hotspot_partition_calculator::send_hotkey_detect_request
+bool detect_hotkey(command_executor *e, shell_context *sc, arguments args)
+{
+ // detect_hotkey [-a|--app_name]
[-p|--partition_index][-c|--hotkey_action][-t|--hotkey_type]
+ const std::set<std::string> ¶ms = {
+ "a", "app_name", "p", "partition_index", "c", "hotkey_action", "t",
"hotkey_type"};
+ argh::parser cmd(args.argc, args.argv,
argh::parser::PREFER_PARAM_FOR_UNREG_OPTION);
+ if (!validate_cmd(cmd, params)) {
+ return false;
+ }
+
+ std::string app_name = cmd({"-a", "--app_name"}).str();
+ uint64_t partition_index;
+ try {
+ partition_index = boost::lexical_cast<uint64_t>(cmd({"-p",
"--partition_index"}).str());
+ } catch (boost::bad_lexical_cast &e) {
+ fmt::print(stderr,
+ "\"{}\" is a invalid partition index \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::apps::hotkey_detect_request req;
+ if (!generate_hotkey_request(req, hotkey_action, hotkey_type)) {
+ return false;
+ }
+ auto request_ptr = std::make_unique<dsn::apps::hotkey_detect_request>(req);
+ auto resolver = partition_resolver::get_resolver(
+ sc->current_cluster_name.c_str(), sc->meta_list, app_name.c_str());
+ bool is_successful = true;
+ dsn::task_tracker tracker;
+ pegasus::server::detect_hotkey_rpc rpc(
+ std::move(request_ptr), RPC_DETECT_HOTKEY, std::chrono::seconds(10),
partition_index);
+ rpc.call(resolver,
+ &tracker,
+ [app_name, partition_index, &is_successful](dsn::error_code
error) {
+ if (error != dsn::ERR_OK) {
+ is_successful = false;
+ fmt::print(stderr,
+ "Hotkey detect rpc sending failed, in {}.{},
error_hint:{} \n",
+ app_name,
+ partition_index,
+ error.to_string());
+ }
+ })
+ ->wait();
Review comment:
If partition_index is an invalid parameter, will it get_resolver succeed
and send rpc succeed?
For example, this app partiion_count is 16, but partition_index is 18.
##########
File path: src/shell/main.cpp
##########
@@ -474,6 +474,15 @@ 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_name str]"
+ "[-p|--partition_index num]"
+ "[-t|--hotkey_type read|write]"
+ "[-c|--detect_action start|stop]",
Review comment:
`<>` to show required parameters, and `[]` is for optional parameters. I
suggest using `<>`.
##########
File path: src/server/pegasus_read_service.h
##########
@@ -79,6 +81,7 @@ class pegasus_read_service : public
dsn::replication::replication_app_base,
register_rpc_handler_with_rpc_holder(dsn::apps::RPC_RRDB_RRDB_SCAN,
"scan", on_scan);
register_async_rpc_handler(
dsn::apps::RPC_RRDB_RRDB_CLEAR_SCANNER, "clear_scanner",
on_clear_scanner);
+ register_rpc_handler_with_rpc_holder(RPC_DETECT_HOTKEY,
"detect_hotkey", on_detect_hotkey);
Review comment:
I still don't recommend that the `detect_hotkey` rpc handler is
registered in `pegasus_read_service`.
----------------------------------------------------------------
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]
