You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pegasus.apache.org by la...@apache.org on 2022/11/17 02:44:05 UTC
[incubator-pegasus] branch master updated: refactor: use command_deregister to auto deregister from command_manager (#1241)
This is an automated email from the ASF dual-hosted git repository.
laiyingchun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pegasus.git
The following commit(s) were added to refs/heads/master by this push:
new 405ce5fef refactor: use command_deregister to auto deregister from command_manager (#1241)
405ce5fef is described below
commit 405ce5fefadb53ff1d5f47110ddf9e1e81fdcc48
Author: Yingchun Lai <la...@apache.org>
AuthorDate: Thu Nov 17 10:43:59 2022 +0800
refactor: use command_deregister to auto deregister from command_manager (#1241)
---
.licenserc.yaml | 1 -
src/common/api_common.h | 2 -
src/failure_detector/failure_detector.cpp | 2 +-
src/failure_detector/failure_detector.h | 2 +-
src/meta/app_balance_policy.cpp | 72 +++++----------
src/meta/app_balance_policy.h | 11 +--
src/meta/cluster_balance_policy.h | 2 +-
src/meta/greedy_load_balancer.cpp | 8 +-
src/meta/greedy_load_balancer.h | 2 +-
src/meta/load_balance_policy.cpp | 32 +++----
src/meta/load_balance_policy.h | 7 +-
src/meta/meta_service.cpp | 7 +-
src/meta/meta_service.h | 4 +-
src/meta/partition_guardian.cpp | 14 +--
src/meta/partition_guardian.h | 3 +-
src/meta/server_state.cpp | 28 ++----
src/meta/server_state.h | 4 +-
src/nfs/nfs_client_impl.cpp | 6 +-
src/nfs/nfs_client_impl.h | 2 +-
src/nfs/nfs_server_impl.h | 5 +-
src/perf_counter/perf_counters.cpp | 22 ++---
src/perf_counter/perf_counters.h | 11 +--
src/replica/replica_stub.cpp | 148 ++++++++++--------------------
src/replica/replica_stub.h | 17 +---
src/runtime/service_engine.cpp | 16 +---
src/runtime/service_engine.h | 4 +-
src/runtime/task/task_engine.h | 8 +-
src/utils/command_manager.cpp | 91 +++++++++---------
src/utils/command_manager.h | 52 +++++++----
src/utils/test/command_manager.cpp | 80 ----------------
src/utils/test/command_manager_test.cpp | 69 ++++++++++++++
src/utils/test/main.cpp | 7 +-
32 files changed, 305 insertions(+), 434 deletions(-)
diff --git a/.licenserc.yaml b/.licenserc.yaml
index e7b302fed..093392404 100644
--- a/.licenserc.yaml
+++ b/.licenserc.yaml
@@ -610,7 +610,6 @@ header:
- 'src/utils/test/CMakeLists.txt'
- 'src/utils/test/address.cpp'
- 'src/utils/test/clear.sh'
- - 'src/utils/test/command_manager.cpp'
- 'src/utils/test/config-bad-section.ini'
- 'src/utils/test/config-dup-key.ini'
- 'src/utils/test/config-dup-section.ini'
diff --git a/src/common/api_common.h b/src/common/api_common.h
index 441309d82..1a1105fa7 100644
--- a/src/common/api_common.h
+++ b/src/common/api_common.h
@@ -88,5 +88,3 @@
#define DSN_MAX_CALLBAC_COUNT 32
#define DSN_MAX_APP_COUNT_IN_SAME_PROCESS 256
#define DSN_MAX_PATH 1024
-
-typedef void *dsn_handle_t;
diff --git a/src/failure_detector/failure_detector.cpp b/src/failure_detector/failure_detector.cpp
index 7c828fa72..2c165963b 100644
--- a/src/failure_detector/failure_detector.cpp
+++ b/src/failure_detector/failure_detector.cpp
@@ -68,7 +68,7 @@ void failure_detector::register_ctrl_commands()
});
}
-void failure_detector::unregister_ctrl_commands() { UNREGISTER_VALID_HANDLER(_get_allow_list); }
+void failure_detector::unregister_ctrl_commands() { _get_allow_list.reset(); }
error_code failure_detector::start(uint32_t check_interval_seconds,
uint32_t beacon_interval_seconds,
diff --git a/src/failure_detector/failure_detector.h b/src/failure_detector/failure_detector.h
index ee572d52c..b344f0b4c 100644
--- a/src/failure_detector/failure_detector.h
+++ b/src/failure_detector/failure_detector.h
@@ -221,7 +221,7 @@ private:
perf_counter_wrapper _recent_beacon_fail_count;
- dsn_handle_t _get_allow_list = nullptr;
+ std::unique_ptr<command_deregister> _get_allow_list;
protected:
mutable zlock _lock;
diff --git a/src/meta/app_balance_policy.cpp b/src/meta/app_balance_policy.cpp
index 9775e74ac..3e61ad168 100644
--- a/src/meta/app_balance_policy.cpp
+++ b/src/meta/app_balance_policy.cpp
@@ -23,11 +23,7 @@
namespace dsn {
namespace replication {
-app_balance_policy::app_balance_policy(meta_service *svc)
- : load_balance_policy(svc),
- _ctrl_balancer_in_turn(nullptr),
- _ctrl_only_primary_balancer(nullptr),
- _ctrl_only_move_primary(nullptr)
+app_balance_policy::app_balance_policy(meta_service *svc) : load_balance_policy(svc)
{
if (_svc != nullptr) {
_balancer_in_turn = _svc->get_meta_options()._lb_opts.balancer_in_turn;
@@ -38,11 +34,32 @@ app_balance_policy::app_balance_policy(meta_service *svc)
_only_primary_balancer = false;
_only_move_primary = false;
}
- register_ctrl_commands();
+ _cmds.emplace_back(dsn::command_manager::instance().register_command(
+ {"meta.lb.balancer_in_turn"},
+ "meta.lb.balancer_in_turn <true|false>",
+ "control whether do app balancer in turn",
+ [this](const std::vector<std::string> &args) {
+ return remote_command_set_bool_flag(_balancer_in_turn, "lb.balancer_in_turn", args);
+ }));
+
+ _cmds.emplace_back(dsn::command_manager::instance().register_command(
+ {"meta.lb.only_primary_balancer"},
+ "meta.lb.only_primary_balancer <true|false>",
+ "control whether do only primary balancer",
+ [this](const std::vector<std::string> &args) {
+ return remote_command_set_bool_flag(
+ _only_primary_balancer, "lb.only_primary_balancer", args);
+ }));
+
+ _cmds.emplace_back(dsn::command_manager::instance().register_command(
+ {"meta.lb.only_move_primary"},
+ "meta.lb.only_move_primary <true|false>",
+ "control whether only move primary in balancer",
+ [this](const std::vector<std::string> &args) {
+ return remote_command_set_bool_flag(_only_move_primary, "lb.only_move_primary", args);
+ }));
}
-app_balance_policy::~app_balance_policy() { unregister_ctrl_commands(); }
-
void app_balance_policy::balance(bool checker, const meta_view *global_view, migration_list *list)
{
init(global_view, list);
@@ -76,45 +93,6 @@ void app_balance_policy::balance(bool checker, const meta_view *global_view, mig
std::placeholders::_2));
}
-void app_balance_policy::register_ctrl_commands()
-{
- static std::once_flag flag;
- std::call_once(flag, [&]() {
- _ctrl_balancer_in_turn = dsn::command_manager::instance().register_command(
- {"meta.lb.balancer_in_turn"},
- "meta.lb.balancer_in_turn <true|false>",
- "control whether do app balancer in turn",
- [this](const std::vector<std::string> &args) {
- return remote_command_set_bool_flag(_balancer_in_turn, "lb.balancer_in_turn", args);
- });
-
- _ctrl_only_primary_balancer = dsn::command_manager::instance().register_command(
- {"meta.lb.only_primary_balancer"},
- "meta.lb.only_primary_balancer <true|false>",
- "control whether do only primary balancer",
- [this](const std::vector<std::string> &args) {
- return remote_command_set_bool_flag(
- _only_primary_balancer, "lb.only_primary_balancer", args);
- });
-
- _ctrl_only_move_primary = dsn::command_manager::instance().register_command(
- {"meta.lb.only_move_primary"},
- "meta.lb.only_move_primary <true|false>",
- "control whether only move primary in balancer",
- [this](const std::vector<std::string> &args) {
- return remote_command_set_bool_flag(
- _only_move_primary, "lb.only_move_primary", args);
- });
- });
-}
-
-void app_balance_policy::unregister_ctrl_commands()
-{
- UNREGISTER_VALID_HANDLER(_ctrl_balancer_in_turn);
- UNREGISTER_VALID_HANDLER(_ctrl_only_primary_balancer);
- UNREGISTER_VALID_HANDLER(_ctrl_only_move_primary);
-}
-
bool app_balance_policy::need_balance_secondaries(bool balance_checker)
{
if (!balance_checker && !_migration_result->empty()) {
diff --git a/src/meta/app_balance_policy.h b/src/meta/app_balance_policy.h
index 7dfb7a589..d7e7cffb7 100644
--- a/src/meta/app_balance_policy.h
+++ b/src/meta/app_balance_policy.h
@@ -25,20 +25,15 @@ class app_balance_policy : public load_balance_policy
{
public:
app_balance_policy(meta_service *svc);
- ~app_balance_policy();
+ ~app_balance_policy() = default;
- void balance(bool checker, const meta_view *global_view, migration_list *list);
+ void balance(bool checker, const meta_view *global_view, migration_list *list) override;
private:
bool need_balance_secondaries(bool balance_checker);
bool copy_secondary(const std::shared_ptr<app_state> &app, bool place_holder);
- void register_ctrl_commands();
- void unregister_ctrl_commands();
-
- dsn_handle_t _ctrl_balancer_in_turn;
- dsn_handle_t _ctrl_only_primary_balancer;
- dsn_handle_t _ctrl_only_move_primary;
+ std::vector<std::unique_ptr<command_deregister>> _cmds;
// options
bool _balancer_in_turn;
diff --git a/src/meta/cluster_balance_policy.h b/src/meta/cluster_balance_policy.h
index abd3741bd..6976b221b 100644
--- a/src/meta/cluster_balance_policy.h
+++ b/src/meta/cluster_balance_policy.h
@@ -33,7 +33,7 @@ public:
cluster_balance_policy(meta_service *svc);
~cluster_balance_policy() = default;
- void balance(bool checker, const meta_view *global_view, migration_list *list);
+ void balance(bool checker, const meta_view *global_view, migration_list *list) override;
private:
struct cluster_migration_info;
diff --git a/src/meta/greedy_load_balancer.cpp b/src/meta/greedy_load_balancer.cpp
index cc2261bc5..6ed30925c 100644
--- a/src/meta/greedy_load_balancer.cpp
+++ b/src/meta/greedy_load_balancer.cpp
@@ -44,8 +44,7 @@ DSN_TAG_VARIABLE(balance_cluster, FT_MUTABLE);
DSN_DECLARE_uint64(min_live_node_count_for_unfreeze);
-greedy_load_balancer::greedy_load_balancer(meta_service *_svc)
- : server_load_balancer(_svc), _get_balance_operation_count(nullptr)
+greedy_load_balancer::greedy_load_balancer(meta_service *_svc) : server_load_balancer(_svc)
{
_app_balance_policy = dsn::make_unique<app_balance_policy>(_svc);
_cluster_balance_policy = dsn::make_unique<cluster_balance_policy>(_svc);
@@ -85,10 +84,7 @@ void greedy_load_balancer::register_ctrl_commands()
[this](const std::vector<std::string> &args) { return get_balance_operation_count(args); });
}
-void greedy_load_balancer::unregister_ctrl_commands()
-{
- UNREGISTER_VALID_HANDLER(_get_balance_operation_count);
-}
+void greedy_load_balancer::unregister_ctrl_commands() { _get_balance_operation_count.reset(); }
std::string greedy_load_balancer::get_balance_operation_count(const std::vector<std::string> &args)
{
diff --git a/src/meta/greedy_load_balancer.h b/src/meta/greedy_load_balancer.h
index 9da861cf2..201e045a7 100644
--- a/src/meta/greedy_load_balancer.h
+++ b/src/meta/greedy_load_balancer.h
@@ -74,7 +74,7 @@ private:
std::unique_ptr<load_balance_policy> _app_balance_policy;
std::unique_ptr<load_balance_policy> _cluster_balance_policy;
- dsn_handle_t _get_balance_operation_count;
+ std::unique_ptr<command_deregister> _get_balance_operation_count;
// perf counters
perf_counter_wrapper _balance_operation_count;
diff --git a/src/meta/load_balance_policy.cpp b/src/meta/load_balance_policy.cpp
index 431470025..a6002bb2d 100644
--- a/src/meta/load_balance_policy.cpp
+++ b/src/meta/load_balance_policy.cpp
@@ -170,10 +170,19 @@ generate_balancer_request(const app_mapper &apps,
load_balance_policy::load_balance_policy(meta_service *svc)
: _svc(svc), _ctrl_balancer_ignored_apps(nullptr)
{
- register_ctrl_commands();
+ static std::once_flag flag;
+ std::call_once(flag, [&]() {
+ _ctrl_balancer_ignored_apps = dsn::command_manager::instance().register_command(
+ {"meta.lb.ignored_app_list"},
+ "meta.lb.ignored_app_list <get|set|clear> [app_id1,app_id2..]",
+ "get, set and clear balancer ignored_app_list",
+ [this](const std::vector<std::string> &args) {
+ return remote_command_balancer_ignored_app_ids(args);
+ });
+ });
}
-load_balance_policy::~load_balance_policy() { unregister_ctrl_commands(); }
+load_balance_policy::~load_balance_policy() {}
void load_balance_policy::init(const meta_view *global_view, migration_list *list)
{
@@ -373,25 +382,6 @@ bool load_balance_policy::execute_balance(
return true;
}
-void load_balance_policy::register_ctrl_commands()
-{
- static std::once_flag flag;
- std::call_once(flag, [&]() {
- _ctrl_balancer_ignored_apps = dsn::command_manager::instance().register_command(
- {"meta.lb.ignored_app_list"},
- "meta.lb.ignored_app_list <get|set|clear> [app_id1,app_id2..]",
- "get, set and clear balancer ignored_app_list",
- [this](const std::vector<std::string> &args) {
- return remote_command_balancer_ignored_app_ids(args);
- });
- });
-}
-
-void load_balance_policy::unregister_ctrl_commands()
-{
- UNREGISTER_VALID_HANDLER(_ctrl_balancer_ignored_apps);
-}
-
std::string
load_balance_policy::remote_command_balancer_ignored_app_ids(const std::vector<std::string> &args)
{
diff --git a/src/meta/load_balance_policy.h b/src/meta/load_balance_policy.h
index 25a22d77f..b8574959b 100644
--- a/src/meta/load_balance_policy.h
+++ b/src/meta/load_balance_policy.h
@@ -57,7 +57,7 @@ class load_balance_policy
{
public:
load_balance_policy(meta_service *svc);
- virtual ~load_balance_policy() = 0;
+ virtual ~load_balance_policy();
virtual void balance(bool checker, const meta_view *global_view, migration_list *list) = 0;
@@ -88,7 +88,7 @@ protected:
dsn::zrwlock_nr _balancer_ignored_apps_lock; // {
std::set<app_id> _balancer_ignored_apps;
// }
- dsn_handle_t _ctrl_balancer_ignored_apps;
+ std::unique_ptr<command_deregister> _ctrl_balancer_ignored_apps;
private:
void start_moving_primary(const std::shared_ptr<app_state> &app,
@@ -112,9 +112,6 @@ private:
std::string get_balancer_ignored_app_ids();
std::string clear_balancer_ignored_app_ids();
- void register_ctrl_commands();
- void unregister_ctrl_commands();
-
FRIEND_TEST(cluster_balance_policy, calc_potential_moving);
};
diff --git a/src/meta/meta_service.cpp b/src/meta/meta_service.cpp
index 58f011f86..e7a754349 100644
--- a/src/meta/meta_service.cpp
+++ b/src/meta/meta_service.cpp
@@ -237,13 +237,18 @@ void meta_service::register_ctrl_commands()
void meta_service::unregister_ctrl_commands()
{
- UNREGISTER_VALID_HANDLER(_ctrl_node_live_percentage_threshold_for_update);
+ // TODO(yingchun): the commands can be unregister automatiically, maybe we can remove the manual
+ // unregister code later
+ _ctrl_node_live_percentage_threshold_for_update.reset();
if (_partition_guardian != nullptr) {
_partition_guardian->unregister_ctrl_commands();
}
if (_balancer != nullptr) {
_balancer->unregister_ctrl_commands();
}
+ if (_failure_detector != nullptr) {
+ _failure_detector->unregister_ctrl_commands();
+ }
}
void meta_service::start_service()
diff --git a/src/meta/meta_service.h b/src/meta/meta_service.h
index 77e0aece1..833446b57 100644
--- a/src/meta/meta_service.h
+++ b/src/meta/meta_service.h
@@ -293,7 +293,7 @@ private:
replication_options _opts;
meta_options _meta_opts;
uint64_t _node_live_percentage_threshold_for_update;
- dsn_handle_t _ctrl_node_live_percentage_threshold_for_update = nullptr;
+ std::unique_ptr<command_deregister> _ctrl_node_live_percentage_threshold_for_update;
std::shared_ptr<server_state> _state;
std::shared_ptr<meta_server_failure_detector> _failure_detector;
@@ -303,7 +303,7 @@ private:
std::shared_ptr<server_load_balancer> _balancer;
std::shared_ptr<backup_service> _backup_handler;
- std::shared_ptr<partition_guardian> _partition_guardian = nullptr;
+ std::shared_ptr<partition_guardian> _partition_guardian;
std::unique_ptr<meta_duplication_service> _dup_svc;
diff --git a/src/meta/partition_guardian.cpp b/src/meta/partition_guardian.cpp
index 0f5cf1349..11c010faa 100644
--- a/src/meta/partition_guardian.cpp
+++ b/src/meta/partition_guardian.cpp
@@ -667,26 +667,22 @@ void partition_guardian::finish_cure_proposal(meta_view &view,
void partition_guardian::register_ctrl_commands()
{
- _ctrl_assign_delay_ms = dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(dsn::command_manager::instance().register_command(
{"meta.lb.assign_delay_ms"},
"lb.assign_delay_ms [num | DEFAULT]",
"control the replica_assign_delay_ms_for_dropouts config",
- [this](const std::vector<std::string> &args) { return ctrl_assign_delay_ms(args); });
+ [this](const std::vector<std::string> &args) { return ctrl_assign_delay_ms(args); }));
- _ctrl_assign_secondary_black_list = dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(dsn::command_manager::instance().register_command(
{"meta.lb.assign_secondary_black_list"},
"lb.assign_secondary_black_list [<ip:port,ip:port,ip:port>|clear]",
"control the assign secondary black list",
[this](const std::vector<std::string> &args) {
return ctrl_assign_secondary_black_list(args);
- });
+ }));
}
-void partition_guardian::unregister_ctrl_commands()
-{
- UNREGISTER_VALID_HANDLER(_ctrl_assign_delay_ms);
- UNREGISTER_VALID_HANDLER(_ctrl_assign_secondary_black_list);
-}
+void partition_guardian::unregister_ctrl_commands() { _cmds.clear(); }
std::string partition_guardian::ctrl_assign_delay_ms(const std::vector<std::string> &args)
{
diff --git a/src/meta/partition_guardian.h b/src/meta/partition_guardian.h
index 73efef250..59a06653f 100644
--- a/src/meta/partition_guardian.h
+++ b/src/meta/partition_guardian.h
@@ -89,9 +89,8 @@ private:
dsn::zrwlock_nr _black_list_lock; // [
std::set<dsn::rpc_address> _assign_secondary_black_list;
// ]
- dsn_handle_t _ctrl_assign_secondary_black_list = nullptr;
- dsn_handle_t _ctrl_assign_delay_ms = nullptr;
+ std::vector<std::unique_ptr<command_deregister>> _cmds;
uint64_t _replica_assign_delay_ms_for_dropouts;
friend class meta_partition_guardian_test;
diff --git a/src/meta/server_state.cpp b/src/meta/server_state.cpp
index 4148a037a..558acb53b 100644
--- a/src/meta/server_state.cpp
+++ b/src/meta/server_state.cpp
@@ -93,24 +93,15 @@ static const char *unlock_state = "unlock";
server_state::server_state()
: _meta_svc(nullptr),
_add_secondary_enable_flow_control(false),
- _add_secondary_max_count_for_one_node(0),
- _cli_dump_handle(nullptr),
- _ctrl_add_secondary_enable_flow_control(nullptr),
- _ctrl_add_secondary_max_count_for_one_node(nullptr)
+ _add_secondary_max_count_for_one_node(0)
{
}
-server_state::~server_state()
-{
- _tracker.cancel_outstanding_tasks();
- UNREGISTER_VALID_HANDLER(_cli_dump_handle);
- UNREGISTER_VALID_HANDLER(_ctrl_add_secondary_enable_flow_control);
- UNREGISTER_VALID_HANDLER(_ctrl_add_secondary_max_count_for_one_node);
-}
+server_state::~server_state() { _tracker.cancel_outstanding_tasks(); }
void server_state::register_cli_commands()
{
- _cli_dump_handle = dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(dsn::command_manager::instance().register_command(
{"meta.dump"},
"meta.dump - dump app_states of meta server to local file",
"meta.dump -t|--target target_file",
@@ -132,20 +123,18 @@ void server_state::register_cli_commands()
}
}
return std::string(err.to_string());
- });
- CHECK_NOTNULL(_cli_dump_handle, "register cli handler failed");
+ }));
- _ctrl_add_secondary_enable_flow_control = dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(dsn::command_manager::instance().register_command(
{"meta.lb.add_secondary_enable_flow_control"},
"meta.lb.add_secondary_enable_flow_control <true|false>",
"control whether enable add secondary flow control",
[this](const std::vector<std::string> &args) {
return remote_command_set_bool_flag(
_add_secondary_enable_flow_control, "lb.add_secondary_enable_flow_control", args);
- });
- CHECK(_ctrl_add_secondary_enable_flow_control, "register cli handler failed");
+ }));
- _ctrl_add_secondary_max_count_for_one_node = dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(dsn::command_manager::instance().register_command(
{"meta.lb.add_secondary_max_count_for_one_node"},
"meta.lb.add_secondary_max_count_for_one_node [num | DEFAULT]",
"control the max count to add secondary for one node",
@@ -167,8 +156,7 @@ void server_state::register_cli_commands()
}
}
return result;
- });
- CHECK(_ctrl_add_secondary_max_count_for_one_node, "register cli handler failed");
+ }));
}
void server_state::initialize(meta_service *meta_svc, const std::string &apps_root)
diff --git a/src/meta/server_state.h b/src/meta/server_state.h
index 256154d7c..374823a92 100644
--- a/src/meta/server_state.h
+++ b/src/meta/server_state.h
@@ -406,9 +406,7 @@ private:
bool _add_secondary_enable_flow_control;
int32_t _add_secondary_max_count_for_one_node;
- dsn_handle_t _cli_dump_handle;
- dsn_handle_t _ctrl_add_secondary_enable_flow_control;
- dsn_handle_t _ctrl_add_secondary_max_count_for_one_node;
+ std::vector<std::unique_ptr<command_deregister>> _cmds;
perf_counter_wrapper _dead_partition_count;
perf_counter_wrapper _unreadable_partition_count;
diff --git a/src/nfs/nfs_client_impl.cpp b/src/nfs/nfs_client_impl.cpp
index 304783d3c..c34728878 100644
--- a/src/nfs/nfs_client_impl.cpp
+++ b/src/nfs/nfs_client_impl.cpp
@@ -119,11 +119,7 @@ nfs_client_impl::nfs_client_impl()
register_cli_commands();
}
-nfs_client_impl::~nfs_client_impl()
-{
- _tracker.cancel_outstanding_tasks();
- UNREGISTER_VALID_HANDLER(_nfs_max_copy_rate_megabytes_cmd);
-}
+nfs_client_impl::~nfs_client_impl() { _tracker.cancel_outstanding_tasks(); }
void nfs_client_impl::begin_remote_copy(std::shared_ptr<remote_copy_request> &rci,
aio_task *nfs_task)
diff --git a/src/nfs/nfs_client_impl.h b/src/nfs/nfs_client_impl.h
index 81f9f8a40..6816c1aed 100644
--- a/src/nfs/nfs_client_impl.h
+++ b/src/nfs/nfs_client_impl.h
@@ -299,7 +299,7 @@ private:
perf_counter_wrapper _recent_write_data_size;
perf_counter_wrapper _recent_write_fail_count;
- dsn_handle_t _nfs_max_copy_rate_megabytes_cmd;
+ std::unique_ptr<command_deregister> _nfs_max_copy_rate_megabytes_cmd;
dsn::task_tracker _tracker;
};
diff --git a/src/nfs/nfs_server_impl.h b/src/nfs/nfs_server_impl.h
index ae9057596..f5bc3ac8c 100644
--- a/src/nfs/nfs_server_impl.h
+++ b/src/nfs/nfs_server_impl.h
@@ -54,11 +54,12 @@ public:
void register_cli_commands();
+ // TODO(yingchun): seems nobody call it, can be removed?
void close_service()
{
unregister_rpc_handler(RPC_NFS_COPY);
unregister_rpc_handler(RPC_NFS_GET_FILE_SIZE);
- UNREGISTER_VALID_HANDLER(_nfs_max_send_rate_megabytes_cmd);
+ _nfs_max_send_rate_megabytes_cmd.reset();
}
protected:
@@ -128,7 +129,7 @@ private:
perf_counter_wrapper _recent_copy_data_size;
perf_counter_wrapper _recent_copy_fail_count;
- dsn_handle_t _nfs_max_send_rate_megabytes_cmd;
+ std::unique_ptr<command_deregister> _nfs_max_send_rate_megabytes_cmd;
dsn::task_tracker _tracker;
};
diff --git a/src/perf_counter/perf_counters.cpp b/src/perf_counter/perf_counters.cpp
index 32a815217..5bbceb6d3 100644
--- a/src/perf_counter/perf_counters.cpp
+++ b/src/perf_counter/perf_counters.cpp
@@ -50,14 +50,14 @@ perf_counters::perf_counters()
// perf_counters
tools::shared_io_service::instance();
- _perf_counters_cmd = command_manager::instance().register_command(
+ _cmds.emplace_back(command_manager::instance().register_command(
{"perf-counters"},
"perf-counters - query perf counters, filtered by OR of POSIX basic regular expressions",
"perf-counters [regexp]...",
[](const std::vector<std::string> &args) {
return perf_counters::instance().list_snapshot_by_regexp(args);
- });
- _perf_counters_by_substr_cmd = command_manager::instance().register_command(
+ }));
+ _cmds.emplace_back(command_manager::instance().register_command(
{"perf-counters-by-substr"},
"perf-counters-by-substr - query perf counters, filtered by OR of substrs",
"perf-counters-by-substr [substr]...",
@@ -66,8 +66,8 @@ perf_counters::perf_counters()
args, [](const std::string &arg, const counter_snapshot &cs) {
return cs.name.find(arg) != std::string::npos;
});
- });
- _perf_counters_by_prefix_cmd = command_manager::instance().register_command(
+ }));
+ _cmds.emplace_back(command_manager::instance().register_command(
{"perf-counters-by-prefix"},
"perf-counters-by-prefix - query perf counters, filtered by OR of prefix strings",
"perf-counters-by-prefix [prefix]...",
@@ -77,8 +77,8 @@ perf_counters::perf_counters()
return cs.name.size() >= arg.size() &&
::memcmp(cs.name.c_str(), arg.c_str(), arg.size()) == 0;
});
- });
- _perf_counters_by_postfix_cmd = command_manager::instance().register_command(
+ }));
+ _cmds.emplace_back(command_manager::instance().register_command(
{"perf-counters-by-postfix"},
"perf-counters-by-postfix - query perf counters, filtered by OR of postfix strings",
"perf-counters-by-postfix [postfix]...",
@@ -90,16 +90,14 @@ perf_counters::perf_counters()
arg.c_str(),
arg.size()) == 0;
});
- });
+ }));
}
perf_counters::~perf_counters()
{
+ // TODO(yingchun): can we use default deconstructor?
_counters.clear();
- UNREGISTER_VALID_HANDLER(_perf_counters_cmd);
- UNREGISTER_VALID_HANDLER(_perf_counters_by_substr_cmd);
- UNREGISTER_VALID_HANDLER(_perf_counters_by_prefix_cmd);
- UNREGISTER_VALID_HANDLER(_perf_counters_by_postfix_cmd);
+ _cmds.clear();
}
perf_counter_ptr perf_counters::get_app_counter(const char *section,
diff --git a/src/perf_counter/perf_counters.h b/src/perf_counter/perf_counters.h
index 4ae09d891..9aee6d512 100644
--- a/src/perf_counter/perf_counters.h
+++ b/src/perf_counter/perf_counters.h
@@ -39,6 +39,8 @@
namespace dsn {
+class command_deregister;
+
/// Registry of all perf counters, users can get/create a specific perf counter
/// via `get_app_counter` and `get_global_counter`.
/// To push metrics to some monitoring systems (e.g Prometheus), users can
@@ -134,6 +136,8 @@ public:
std::function<bool(const std::string &arg, const counter_snapshot &cs)> filter) const;
private:
+ friend class utils::singleton<perf_counters>;
+
perf_counters();
~perf_counters();
@@ -165,12 +169,7 @@ private:
// timestamp in seconds when take snapshot of current counters
int64_t _timestamp;
- dsn_handle_t _perf_counters_cmd;
- dsn_handle_t _perf_counters_by_substr_cmd;
- dsn_handle_t _perf_counters_by_prefix_cmd;
- dsn_handle_t _perf_counters_by_postfix_cmd;
-
- friend class utils::singleton<perf_counters>;
+ std::vector<std::unique_ptr<command_deregister>> _cmds;
};
} // namespace dsn
diff --git a/src/replica/replica_stub.cpp b/src/replica/replica_stub.cpp
index 6d7395c0f..cd4e64726 100644
--- a/src/replica/replica_stub.cpp
+++ b/src/replica/replica_stub.cpp
@@ -82,14 +82,6 @@ bool replica_stub::s_not_exit_on_log_failure = false;
replica_stub::replica_stub(replica_state_subscriber subscriber /*= nullptr*/,
bool is_long_subscriber /* = true*/)
: serverlet("replica_stub"),
- _kill_partition_command(nullptr),
- _deny_client_command(nullptr),
- _verbose_client_log_command(nullptr),
- _verbose_commit_log_command(nullptr),
- _trigger_chkpt_command(nullptr),
- _query_compact_command(nullptr),
- _query_app_envs_command(nullptr),
- _max_concurrent_bulk_load_downloading_count_command(nullptr),
_deny_client(false),
_verbose_client_log(false),
_verbose_commit_log(false),
@@ -104,12 +96,6 @@ replica_stub::replica_stub(replica_state_subscriber subscriber /*= nullptr*/,
{
#ifdef DSN_ENABLE_GPERF
_is_releasing_memory = false;
- _release_tcmalloc_memory_command = nullptr;
- _get_tcmalloc_status_command = nullptr;
- _max_reserved_memory_percentage_command = nullptr;
- _release_all_reserved_memory_command = nullptr;
-#elif defined(DSN_USE_JEMALLOC)
- _dump_jemalloc_stats_command = nullptr;
#endif
_replica_state_subscriber = subscriber;
_is_long_subscriber = is_long_subscriber;
@@ -2275,7 +2261,7 @@ void replica_stub::open_service()
#if !defined(DSN_ENABLE_GPERF) && defined(DSN_USE_JEMALLOC)
void replica_stub::register_jemalloc_ctrl_command()
{
- _dump_jemalloc_stats_command = ::dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(::dsn::command_manager::instance().register_command(
{"replica.dump-jemalloc-stats"},
fmt::format("replica.dump-jemalloc-stats <{}> [buffer size]", kAllJeStatsTypesStr),
"dump stats of jemalloc",
@@ -2303,7 +2289,7 @@ void replica_stub::register_jemalloc_ctrl_command()
dsn::je_dump_stats(type, static_cast<size_t>(buf_sz), stats);
return stats;
- });
+ }));
}
#endif
@@ -2316,7 +2302,7 @@ void replica_stub::register_ctrl_command()
/// failure_detector::register_ctrl_commands and nfs_client_impl::register_cli_commands
static std::once_flag flag;
std::call_once(flag, [&]() {
- _kill_partition_command = ::dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(::dsn::command_manager::instance().register_command(
{"replica.kill_partition"},
"replica.kill_partition [app_id [partition_index]]",
"replica.kill_partition: kill partitions by (all, one app, one partition)",
@@ -2336,17 +2322,17 @@ void replica_stub::register_ctrl_command()
}
dsn::error_code e = this->on_kill_replica(pid);
return std::string(e.to_string());
- });
+ }));
- _deny_client_command = ::dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(::dsn::command_manager::instance().register_command(
{"replica.deny-client"},
"replica.deny-client <true|false>",
"replica.deny-client - control if deny client read & write request",
[this](const std::vector<std::string> &args) {
return remote_command_set_bool_flag(_deny_client, "deny-client", args);
- });
+ }));
- _verbose_client_log_command = ::dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(::dsn::command_manager::instance().register_command(
{"replica.verbose-client-log"},
"replica.verbose-client-log <true|false>",
"replica.verbose-client-log - control if print verbose error log when reply read & "
@@ -2354,18 +2340,18 @@ void replica_stub::register_ctrl_command()
[this](const std::vector<std::string> &args) {
return remote_command_set_bool_flag(
_verbose_client_log, "verbose-client-log", args);
- });
+ }));
- _verbose_commit_log_command = ::dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(::dsn::command_manager::instance().register_command(
{"replica.verbose-commit-log"},
"replica.verbose-commit-log <true|false>",
"replica.verbose-commit-log - control if print verbose log when commit mutation",
[this](const std::vector<std::string> &args) {
return remote_command_set_bool_flag(
_verbose_commit_log, "verbose-commit-log", args);
- });
+ }));
- _trigger_chkpt_command = ::dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(::dsn::command_manager::instance().register_command(
{"replica.trigger-checkpoint"},
"replica.trigger-checkpoint [id1,id2,...] (where id is 'app_id' or "
"'app_id.partition_id')",
@@ -2378,9 +2364,9 @@ void replica_stub::register_ctrl_command()
rep->get_gpid().thread_hash());
return std::string("triggered");
});
- });
+ }));
- _query_compact_command = ::dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(::dsn::command_manager::instance().register_command(
{"replica.query-compact"},
"replica.query-compact [id1,id2,...] (where id is 'app_id' or 'app_id.partition_id')",
"replica.query-compact - query full compact status on the underlying storage engine",
@@ -2388,9 +2374,9 @@ void replica_stub::register_ctrl_command()
return exec_command_on_replica(args, true, [](const replica_ptr &rep) {
return rep->query_manual_compact_state();
});
- });
+ }));
- _query_app_envs_command = ::dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(::dsn::command_manager::instance().register_command(
{"replica.query-app-envs"},
"replica.query-app-envs [id1,id2,...] (where id is 'app_id' or 'app_id.partition_id')",
"replica.query-app-envs - query app envs on the underlying storage engine",
@@ -2400,19 +2386,19 @@ void replica_stub::register_ctrl_command()
rep->query_app_envs(kv_map);
return dsn::utils::kv_map_to_string(kv_map, ',', '=');
});
- });
+ }));
#ifdef DSN_ENABLE_GPERF
- _release_tcmalloc_memory_command = ::dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(::dsn::command_manager::instance().register_command(
{"replica.release-tcmalloc-memory"},
"replica.release-tcmalloc-memory <true|false>",
"replica.release-tcmalloc-memory - control if try to release tcmalloc memory",
[this](const std::vector<std::string> &args) {
return remote_command_set_bool_flag(
_release_tcmalloc_memory, "release-tcmalloc-memory", args);
- });
+ }));
- _get_tcmalloc_status_command = ::dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(::dsn::command_manager::instance().register_command(
{"replica.get-tcmalloc-status"},
"replica.get-tcmalloc-status - get status of tcmalloc",
"get status of tcmalloc",
@@ -2420,9 +2406,9 @@ void replica_stub::register_ctrl_command()
char buf[4096];
MallocExtension::instance()->GetStats(buf, 4096);
return std::string(buf);
- });
+ }));
- _max_reserved_memory_percentage_command = dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(::dsn::command_manager::instance().register_command(
{"replica.mem-release-max-reserved-percentage"},
"replica.mem-release-max-reserved-percentage [num | DEFAULT]",
"control tcmalloc max reserved but not-used memory percentage",
@@ -2447,46 +2433,45 @@ void replica_stub::register_ctrl_command()
_mem_release_max_reserved_mem_percentage = percentage;
}
return result;
- });
+ }));
- _release_all_reserved_memory_command = ::dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(::dsn::command_manager::instance().register_command(
{"replica.release-all-reserved-memory"},
"replica.release-all-reserved-memory - release tcmalloc all reserved-not-used memory",
"release tcmalloc all reserverd not-used memory back to operating system",
[this](const std::vector<std::string> &args) {
auto release_bytes = gc_tcmalloc_memory(true);
return "OK, release_bytes=" + std::to_string(release_bytes);
- });
+ }));
#elif defined(DSN_USE_JEMALLOC)
register_jemalloc_ctrl_command();
#endif
- _max_concurrent_bulk_load_downloading_count_command =
- dsn::command_manager::instance().register_command(
- {"replica.max-concurrent-bulk-load-downloading-count"},
- "replica.max-concurrent-bulk-load-downloading-count [num | DEFAULT]",
- "control stub max_concurrent_bulk_load_downloading_count",
- [this](const std::vector<std::string> &args) {
- std::string result("OK");
- if (args.empty()) {
- result = "max_concurrent_bulk_load_downloading_count=" +
- std::to_string(_max_concurrent_bulk_load_downloading_count);
- return result;
- }
-
- if (args[0] == "DEFAULT") {
- _max_concurrent_bulk_load_downloading_count =
- _options.max_concurrent_bulk_load_downloading_count;
- return result;
- }
+ _cmds.emplace_back(::dsn::command_manager::instance().register_command(
+ {"replica.max-concurrent-bulk-load-downloading-count"},
+ "replica.max-concurrent-bulk-load-downloading-count [num | DEFAULT]",
+ "control stub max_concurrent_bulk_load_downloading_count",
+ [this](const std::vector<std::string> &args) {
+ std::string result("OK");
+ if (args.empty()) {
+ result = "max_concurrent_bulk_load_downloading_count=" +
+ std::to_string(_max_concurrent_bulk_load_downloading_count);
+ return result;
+ }
- int32_t count = 0;
- if (!dsn::buf2int32(args[0], count) || count <= 0) {
- result = std::string("ERR: invalid arguments");
- } else {
- _max_concurrent_bulk_load_downloading_count = count;
- }
+ if (args[0] == "DEFAULT") {
+ _max_concurrent_bulk_load_downloading_count =
+ _options.max_concurrent_bulk_load_downloading_count;
return result;
- });
+ }
+
+ int32_t count = 0;
+ if (!dsn::buf2int32(args[0], count) || count <= 0) {
+ result = std::string("ERR: invalid arguments");
+ } else {
+ _max_concurrent_bulk_load_downloading_count = count;
+ }
+ return result;
+ }));
});
}
@@ -2605,43 +2590,10 @@ void replica_stub::close()
// this replica may not be opened
// or is already closed by calling tool_app::stop_all_apps()
// in this case, just return
- if (_kill_partition_command == nullptr) {
+ if (_cmds.empty()) {
return;
}
-
- UNREGISTER_VALID_HANDLER(_kill_partition_command);
- UNREGISTER_VALID_HANDLER(_deny_client_command);
- UNREGISTER_VALID_HANDLER(_verbose_client_log_command);
- UNREGISTER_VALID_HANDLER(_verbose_commit_log_command);
- UNREGISTER_VALID_HANDLER(_trigger_chkpt_command);
- UNREGISTER_VALID_HANDLER(_query_compact_command);
- UNREGISTER_VALID_HANDLER(_query_app_envs_command);
-#ifdef DSN_ENABLE_GPERF
- UNREGISTER_VALID_HANDLER(_release_tcmalloc_memory_command);
- UNREGISTER_VALID_HANDLER(_get_tcmalloc_status_command);
- UNREGISTER_VALID_HANDLER(_max_reserved_memory_percentage_command);
- UNREGISTER_VALID_HANDLER(_release_all_reserved_memory_command);
-#elif defined(DSN_USE_JEMALLOC)
- UNREGISTER_VALID_HANDLER(_dump_jemalloc_stats_command);
-#endif
- UNREGISTER_VALID_HANDLER(_max_concurrent_bulk_load_downloading_count_command);
-
- _kill_partition_command = nullptr;
- _deny_client_command = nullptr;
- _verbose_client_log_command = nullptr;
- _verbose_commit_log_command = nullptr;
- _trigger_chkpt_command = nullptr;
- _query_compact_command = nullptr;
- _query_app_envs_command = nullptr;
-#ifdef DSN_ENABLE_GPERF
- _release_tcmalloc_memory_command = nullptr;
- _get_tcmalloc_status_command = nullptr;
- _max_reserved_memory_percentage_command = nullptr;
- _release_all_reserved_memory_command = nullptr;
-#elif defined(DSN_USE_JEMALLOC)
- _dump_jemalloc_stats_command = nullptr;
-#endif
- _max_concurrent_bulk_load_downloading_count_command = nullptr;
+ _cmds.clear();
if (_config_sync_timer_task != nullptr) {
_config_sync_timer_task->cancel(true);
diff --git a/src/replica/replica_stub.h b/src/replica/replica_stub.h
index e0c4b3823..d21310ea4 100644
--- a/src/replica/replica_stub.h
+++ b/src/replica/replica_stub.h
@@ -372,22 +372,7 @@ private:
std::unique_ptr<replica_backup_server> _backup_server;
// command_handlers
- dsn_handle_t _kill_partition_command;
- dsn_handle_t _deny_client_command;
- dsn_handle_t _verbose_client_log_command;
- dsn_handle_t _verbose_commit_log_command;
- dsn_handle_t _trigger_chkpt_command;
- dsn_handle_t _query_compact_command;
- dsn_handle_t _query_app_envs_command;
-#ifdef DSN_ENABLE_GPERF
- dsn_handle_t _release_tcmalloc_memory_command;
- dsn_handle_t _get_tcmalloc_status_command;
- dsn_handle_t _max_reserved_memory_percentage_command;
- dsn_handle_t _release_all_reserved_memory_command;
-#elif defined(DSN_USE_JEMALLOC)
- dsn_handle_t _dump_jemalloc_stats_command;
-#endif
- dsn_handle_t _max_concurrent_bulk_load_downloading_count_command;
+ std::vector<std::unique_ptr<command_deregister>> _cmds;
bool _deny_client;
bool _verbose_client_log;
diff --git a/src/runtime/service_engine.cpp b/src/runtime/service_engine.cpp
index e5ff96449..45e187e7e 100644
--- a/src/runtime/service_engine.cpp
+++ b/src/runtime/service_engine.cpp
@@ -179,26 +179,20 @@ service_engine::service_engine()
{
_env = nullptr;
- _get_runtime_info_cmd = dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(dsn::command_manager::instance().register_command(
{"engine"},
"engine - get engine internal information",
"engine [app-id]",
- &service_engine::get_runtime_info);
+ &service_engine::get_runtime_info));
- _get_queue_info_cmd = dsn::command_manager::instance().register_command(
+ _cmds.emplace_back(dsn::command_manager::instance().register_command(
{"system.queue"},
"system.queue - get queue internal information",
"system.queue",
- &service_engine::get_queue_info);
+ &service_engine::get_queue_info));
}
-service_engine::~service_engine()
-{
- _nodes_by_app_id.clear();
-
- UNREGISTER_VALID_HANDLER(_get_runtime_info_cmd);
- UNREGISTER_VALID_HANDLER(_get_queue_info_cmd);
-}
+service_engine::~service_engine() { _nodes_by_app_id.clear(); }
void service_engine::init_before_toollets(const service_spec &spec)
{
diff --git a/src/runtime/service_engine.h b/src/runtime/service_engine.h
index 404c793a5..a02ae2938 100644
--- a/src/runtime/service_engine.h
+++ b/src/runtime/service_engine.h
@@ -107,6 +107,7 @@ private:
error_code init_rpc_engine();
};
+class command_deregister;
typedef std::map<int, std::shared_ptr<service_node>> service_nodes_by_app_id;
class service_engine : public utils::singleton<service_engine>
{
@@ -132,8 +133,7 @@ private:
service_spec _spec;
env_provider *_env;
- dsn_handle_t _get_runtime_info_cmd;
- dsn_handle_t _get_queue_info_cmd;
+ std::vector<std::unique_ptr<command_deregister>> _cmds;
bool _simulator;
diff --git a/src/runtime/task/task_engine.h b/src/runtime/task/task_engine.h
index 97f5f56ed..200df3961 100644
--- a/src/runtime/task/task_engine.h
+++ b/src/runtime/task/task_engine.h
@@ -99,11 +99,7 @@ class task_engine
{
public:
task_engine(service_node *node);
- ~task_engine()
- {
- stop();
- UNREGISTER_VALID_HANDLER(_task_queue_max_length_cmd);
- }
+ ~task_engine() { stop(); }
//
// service management routines
@@ -134,7 +130,7 @@ private:
std::vector<task_worker_pool *> _pools;
volatile bool _is_running;
service_node *_node;
- dsn_handle_t _task_queue_max_length_cmd;
+ std::unique_ptr<command_deregister> _task_queue_max_length_cmd;
};
// -------------------- inline implementation ----------------------------
diff --git a/src/utils/command_manager.cpp b/src/utils/command_manager.cpp
index 8b4d742cd..5cafeff42 100644
--- a/src/utils/command_manager.cpp
+++ b/src/utils/command_manager.cpp
@@ -24,37 +24,38 @@
* THE SOFTWARE.
*/
+#include "utils/command_manager.h"
+
#include <iostream>
#include <sstream>
#include <thread>
-#include "utils/command_manager.h"
#include "utils/fmt_logging.h"
+#include "utils/smart_pointers.h"
#include "utils/utils.h"
namespace dsn {
-dsn_handle_t command_manager::register_command(const std::vector<std::string> &commands,
- const std::string &help_one_line,
- const std::string &help_long,
- command_handler handler)
+std::unique_ptr<command_deregister>
+command_manager::register_command(const std::vector<std::string> &commands,
+ const std::string &help_one_line,
+ const std::string &help_long,
+ command_handler handler)
{
utils::auto_write_lock l(_lock);
bool is_valid_cmd = false;
-
for (const std::string &cmd : commands) {
if (!cmd.empty()) {
is_valid_cmd = true;
- auto it = _handlers.find(cmd);
- CHECK(it == _handlers.end(), "command '{}' already regisered", cmd);
+ CHECK(_handlers.find(cmd) == _handlers.end(), "command '{}' already regisered", cmd);
}
}
CHECK(is_valid_cmd, "should not register empty command");
command_instance *c = new command_instance();
c->commands = commands;
- c->help_long = help_long;
c->help_short = help_one_line;
+ c->help_long = help_long;
c->handler = handler;
for (const std::string &cmd : commands) {
@@ -62,10 +63,11 @@ dsn_handle_t command_manager::register_command(const std::vector<std::string> &c
_handlers[cmd] = c;
}
}
- return c;
+
+ return dsn::make_unique<command_deregister>(reinterpret_cast<uintptr_t>(c));
}
-void command_manager::deregister_command(dsn_handle_t handle)
+void command_manager::deregister_command(uintptr_t handle)
{
auto c = reinterpret_cast<command_instance *>(handle);
CHECK_NOTNULL(c, "cannot deregister a null handle");
@@ -98,34 +100,34 @@ bool command_manager::run_command(const std::string &cmd,
command_manager::command_manager()
{
- register_command({"help", "h", "H", "Help"},
- "help|Help|h|H [command] - display help information",
- "",
- [this](const std::vector<std::string> &args) {
- std::stringstream ss;
-
- if (args.size() == 0) {
- utils::auto_read_lock l(_lock);
- for (const auto &c : this->_handlers) {
- ss << c.second->help_short << std::endl;
- }
- } else {
- utils::auto_read_lock l(_lock);
- auto it = _handlers.find(args[0]);
- if (it == _handlers.end())
- ss << "cannot find command '" << args[0] << "'";
- else {
- ss.width(6);
- ss << std::left << it->first << ": " << it->second->help_short
- << std::endl
- << it->second->help_long << std::endl;
- }
- }
-
- return ss.str();
- });
-
- register_command(
+ _cmds.emplace_back(register_command({"help", "h", "H", "Help"},
+ "help|Help|h|H [command] - display help information",
+ "",
+ [this](const std::vector<std::string> &args) {
+ std::stringstream ss;
+
+ if (args.size() == 0) {
+ utils::auto_read_lock l(_lock);
+ for (const auto &c : this->_handlers) {
+ ss << c.second->help_short << std::endl;
+ }
+ } else {
+ utils::auto_read_lock l(_lock);
+ auto it = _handlers.find(args[0]);
+ if (it == _handlers.end())
+ ss << "cannot find command '" << args[0] << "'";
+ else {
+ ss.width(6);
+ ss << std::left << it->first << ": "
+ << it->second->help_short << std::endl
+ << it->second->help_long << std::endl;
+ }
+ }
+
+ return ss.str();
+ }));
+
+ _cmds.emplace_back(register_command(
{"repeat", "r", "R", "Repeat"},
"repeat|Repeat|r|R interval_seconds max_count command - execute command periodically",
"repeat|Repeat|r|R interval_seconds max_count command - execute command every interval "
@@ -169,9 +171,16 @@ command_manager::command_manager()
}
return "repeat command completed";
- });
+ }));
}
-command_manager::~command_manager() { _handlers.clear(); }
+command_manager::~command_manager()
+{
+ _cmds.clear();
+ _handlers.clear();
+ // TODO(yingchun): enable this check when all commands deregister correctly.
+ // CHECK(_handlers.empty(), "All commands must be deregistered before command_manager been
+ // destroyed", _handlers.begin()->first);
+}
} // namespace dsn
diff --git a/src/utils/command_manager.h b/src/utils/command_manager.h
index 3bd8204d1..27a3b4560 100644
--- a/src/utils/command_manager.h
+++ b/src/utils/command_manager.h
@@ -30,32 +30,36 @@
#include "utils/api_utilities.h"
#include "utils/autoref_ptr.h"
+#include "utils/ports.h"
#include "utils/singleton.h"
#include "utils/synchronize.h"
namespace dsn {
+class command_deregister;
+
class command_manager : public ::dsn::utils::singleton<command_manager>
{
public:
typedef std::function<std::string(const std::vector<std::string> &)> command_handler;
- command_manager();
-
- ~command_manager();
-
- dsn_handle_t register_command(const std::vector<std::string> &commands,
- const std::string &help_one_line,
- const std::string &help_long,
- command_handler handler);
-
- void deregister_command(dsn_handle_t handle);
+ // TODO(yingchun): add __attribute__((warn_unused_result)) in future refactor
+ std::unique_ptr<command_deregister> register_command(const std::vector<std::string> &commands,
+ const std::string &help_one_line,
+ const std::string &help_long,
+ command_handler handler);
bool run_command(const std::string &cmd,
const std::vector<std::string> &args,
/*out*/ std::string &output);
private:
+ friend class command_deregister;
+ friend class utils::singleton<command_manager>;
+
+ command_manager();
+ ~command_manager();
+
struct command_instance : public ref_counter
{
std::vector<std::string> commands;
@@ -64,20 +68,32 @@ private:
command_handler handler;
};
+ void deregister_command(uintptr_t handle);
+
typedef ref_ptr<command_instance> command_instance_ptr;
utils::rw_lock_nr _lock;
std::map<std::string, command_instance_ptr> _handlers;
+
+ std::vector<std::unique_ptr<command_deregister>> _cmds;
};
-} // namespace dsn
+class command_deregister
+{
+public:
+ command_deregister(uintptr_t id) : cmd_id_(id) {}
+ ~command_deregister()
+ {
+ if (cmd_id_ != 0) {
+ dsn::command_manager::instance().deregister_command(cmd_id_);
+ cmd_id_ = 0;
+ }
+ }
-#define UNREGISTER_VALID_HANDLER(ptr) \
- do { \
- if (ptr != nullptr) { \
- dsn::command_manager::instance().deregister_command(ptr); \
- ptr = nullptr; \
- } \
- } while (0)
+private:
+ uintptr_t cmd_id_ = 0;
+};
+
+} // namespace dsn
// if args are empty, then return the old flag;
// otherwise set the proper "flag" according to args
diff --git a/src/utils/test/command_manager.cpp b/src/utils/test/command_manager.cpp
deleted file mode 100644
index 3fe076786..000000000
--- a/src/utils/test/command_manager.cpp
+++ /dev/null
@@ -1,80 +0,0 @@
-/*
- * The MIT License (MIT)
- *
- * Copyright (c) 2015 Microsoft Corporation
- *
- * -=- Robust Distributed System Nucleus (rDSN) -=-
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-/*
- * Description:
- * Unit-test for command_manager.
- *
- * Revision history:
- * Nov., 2015, @qinzuoyan (Zuoyan Qin), first version
- * xxxx-xx-xx, author, fix bug about xxx
- */
-
-#include "utils/command_manager.h"
-#include <gtest/gtest.h>
-
-using namespace ::dsn;
-
-void command_manager_module_init()
-{
- dsn::command_manager::instance().register_command(
- {"test-cmd"},
- "test-cmd - just for command_manager unit-test",
- "test-cmd arg1 arg2 ...",
- [](const std::vector<std::string> &args) {
- std::stringstream ss;
- ss << "test-cmd response: [";
- for (size_t i = 0; i < args.size(); ++i) {
- if (i != 0)
- ss << " ";
- ss << args[i];
- }
- ss << "]";
- return ss.str();
- });
-}
-
-TEST(command_manager, exist_command)
-{
- const std::string cmd = "test-cmd";
- const std::vector<std::string> cmd_args{"this", "is", "test", "argument"};
- std::string output;
- dsn::command_manager::instance().run_command(cmd, cmd_args, output);
-
- std::string expect_output = "test-cmd response: [this is test argument]";
- ASSERT_EQ(output, expect_output);
-}
-
-TEST(command_manager, not_exist_command)
-{
- const std::string cmd = "not-exist-cmd";
- const std::vector<std::string> cmd_args{"arg1", "arg2"};
- std::string output;
- dsn::command_manager::instance().run_command(cmd, cmd_args, output);
-
- std::string expect_output = std::string("unknown command '") + cmd + "'";
- ASSERT_EQ(output, expect_output);
-}
diff --git a/src/utils/test/command_manager_test.cpp b/src/utils/test/command_manager_test.cpp
new file mode 100644
index 000000000..b6a732ce6
--- /dev/null
+++ b/src/utils/test/command_manager_test.cpp
@@ -0,0 +1,69 @@
+// 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 "utils/command_manager.h"
+
+#include <boost/algorithm/string/join.hpp>
+#include <fmt/ostream.h>
+#include <gtest/gtest.h>
+
+using std::string;
+using std::vector;
+
+namespace dsn {
+
+class command_manager_test : public ::testing::Test
+{
+public:
+ command_manager_test()
+ {
+ _cmd = command_manager::instance().register_command(
+ {"test-cmd"},
+ "test-cmd - just for command_manager unit-test",
+ "test-cmd arg1 arg2 ...",
+ [](const vector<string> &args) {
+ return fmt::format("test-cmd response: [{}]", boost::join(args, " "));
+ });
+ }
+
+private:
+ std::unique_ptr<command_deregister> _cmd;
+};
+
+TEST_F(command_manager_test, exist_command)
+{
+ const string cmd = "test-cmd";
+ const vector<string> cmd_args{"this", "is", "test", "argument"};
+ string output;
+ command_manager::instance().run_command(cmd, cmd_args, output);
+
+ const string expect_output = "test-cmd response: [this is test argument]";
+ ASSERT_EQ(expect_output, output);
+}
+
+TEST_F(command_manager_test, not_exist_command)
+{
+ const string cmd = "not-exist-cmd";
+ const vector<string> cmd_args{"arg1", "arg2"};
+ string output;
+ command_manager::instance().run_command(cmd, cmd_args, output);
+
+ const string expect_output = string("unknown command '") + cmd + "'";
+ ASSERT_EQ(expect_output, output);
+}
+
+} // namespace dsn
diff --git a/src/utils/test/main.cpp b/src/utils/test/main.cpp
index 163f8d70d..9081f30a1 100644
--- a/src/utils/test/main.cpp
+++ b/src/utils/test/main.cpp
@@ -16,18 +16,15 @@
// under the License.
#include <gtest/gtest.h>
+
#include "utils/api_utilities.h"
-#include "utils/logging_provider.h"
#include "utils/flags.h"
-
-extern void command_manager_module_init();
+#include "utils/logging_provider.h"
GTEST_API_ int main(int argc, char **argv)
{
testing::InitGoogleTest(&argc, argv);
- command_manager_module_init();
- // init logging
dsn_log_init("dsn::tools::simple_logger", "./", nullptr);
dsn::flags_initialize();
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pegasus.apache.org
For additional commands, e-mail: commits-help@pegasus.apache.org