meta_service & server_state: support set/del/clear envs for app #13

LoveHeat · 2018-04-16T02:49:13Z

No description provided.

qinzuoyan · 2018-04-16T03:26:59Z

src/dist/replication/replication.thrift

+    3:optional app_env_set_request      set_req;
+    4:optional app_env_del_request      del_req;
+    5:optional app_env_clear_request    clear_req;
+}


+struct configuration_update_app_env_request
+{

1:string app_name;

2:app_env_operation op = app_env_operation.APP_ENV_OP_INVALID;

3:optional list keys; // used for set & del

4:optional list values; // used for set

5:optional string clear_prefix; // used for clear, empty means clear all
+}

qinzuoyan · 2018-04-16T03:28:57Z

include/dsn/cpp/serverlet.h

@@ -129,6 +129,12 @@ class serverlet : public virtual clientlet
                                              void (T::*handler)(TRpcHolder),
                                              dsn::gpid gpid = {});

+    template <typename TRpcHolder>


去掉这个

master 分支的 rdsn 已经为你提供了这个函数。https://github.com/XiaoMi/rdsn/blob/master/include/dsn/cpp/serverlet.h

qinzuoyan · 2018-04-16T03:29:40Z

include/dsn/dist/replication/replication_ddl_client.h

@@ -156,6 +156,12 @@ class replication_ddl_client : public clientlet
                                         int32_t backup_history_count_to_keep = 0,
                                         const std::string &start_time = std::string());

+    dsn::error_code
+    set_app_env(const std::string &app_name, const std::string &key, const std::string &vlaue);
+    dsn::error_code del_app_envs(const std::string &app_name, const std::vector<std::string> &keys);


传std::set更合适吧？

neverchanje · 2018-04-16T07:00:38Z

include/dsn/dist/replication/replication_ddl_client.h

@@ -156,6 +156,12 @@ class replication_ddl_client : public clientlet
                                         int32_t backup_history_count_to_keep = 0,
                                         const std::string &start_time = std::string());

+    dsn::error_code
+    set_app_env(const std::string &app_name, const std::string &key, const std::string &vlaue);


typo: value 拼错

neverchanje · 2018-04-16T07:05:06Z

src/dist/replication/meta_server/meta_service.cpp

+void meta_service::update_app_env(const app_env_rpc &env_rpc)
+{
+    configuration_update_app_env_response response;
+    RPC_CHECK_STATUS(env_rpc.dsn_request(), response);


用 env_rpc.response()，不用构造 response。

neverchanje · 2018-04-16T07:09:09Z

我建议 app_env 相关的代码统一写到一个单独的文件：meta_app_env_service.h/.cpp，方便代码阅读。

neverchanje · 2018-04-16T07:52:07Z

另外建议参考 https://github.com/XiaoMi/rdsn/blob/wutao_dup_test/src/dist/replication/test/meta_test/unit_test/meta_duplication_service_test.cpp 写单测吧。

qinzuoyan

还要改

qinzuoyan · 2018-04-16T08:51:12Z

src/dist/replication/meta_server/meta_service.cpp

+                         nullptr,
+                         std::bind(&server_state::clear_app_envs, _state.get(), env_rpc));
+        break;
+    default: // app_env_operation::type::APP_ENV_OP_INVALID


如果invalid，返回的错误码是什么？

qinzuoyan · 2018-04-16T09:00:13Z

src/dist/replication/meta_server/server_state.cpp

+        zauto_read_lock l(_lock);
+        std::shared_ptr<app_state> app = get_app(app_name);
+        if (app == nullptr) {
+            dwarn("del_app_env failed with invalid app_name(%s)", app_name.c_str());


response.err是什么？

qinzuoyan · 2018-04-16T09:00:45Z

src/dist/replication/meta_server/server_state.cpp

+        zauto_read_lock l(_lock);
+        std::shared_ptr<app_state> app = get_app(app_name);
+        if (app == nullptr) {
+            dwarn("del_app_env failed with invalid app_name(%s)", app_name.c_str());


response.err是什么？

qinzuoyan · 2018-04-16T09:01:15Z

src/dist/replication/meta_server/server_state.cpp

+        zauto_read_lock l(_lock);
+        std::shared_ptr<app_state> app = get_app(app_name);
+        if (app == nullptr) {
+            dwarn("set_app_env failed with invalid app_name(%s)", app_name.c_str());


response.err是什么？

qinzuoyan · 2018-04-16T09:13:48Z

src/dist/replication/ddl_lib/replication_ddl_client.cpp

+        std::cout << "set app env succeed" << std::endl;
+        if (!response.hint_message.empty()) {
+            std::cout << "=============================" << std::endl;
+            std::cout << response.hint_message << std::endl;


hint_message有没有可能为空？

操作成功时候为空

qinzuoyan · 2018-04-16T09:15:21Z

src/dist/replication/replication.thrift

+    2:app_env_operation op = app_env_operation.APP_ENV_OP_INVALID;
+    3:optional list<string> keys;           // used for set and del
+    4:optional list<string> values;         // only used for set
+    5:optional bool clear_all;              // only used for clear;


最好去掉clear_all

qinzuoyan

LGTM

meta_service & server_state: support set/del/clear envs for app

2bca852

LoveHeat requested review from qinzuoyan, shengofsun, neverchanje and acelyc111 April 16, 2018 02:49

qinzuoyan reviewed Apr 16, 2018

View reviewed changes

neverchanje reviewed Apr 16, 2018

View reviewed changes

meta_service & server_state: fix according to comment

13c39ff

qinzuoyan reviewed Apr 16, 2018

View reviewed changes

LoveHeat added 2 commits April 16, 2018 18:42

meta_service & server_state: fix according to comment

54f0f3f

server_state: add test test for set/del/clear app envs

e0af9c4

qinzuoyan reviewed Apr 16, 2018

View reviewed changes

qinzuoyan merged commit 576de17 into XiaoMi:master Apr 17, 2018

foreverneverer mentioned this pull request Jan 10, 2020

ASAN: heap-buffer-overflow in meta_test_base.h #376

Closed

ZhongChaoqiang mentioned this pull request Apr 27, 2022

fix: crash when huge write #1099

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

meta_service & server_state: support set/del/clear envs for app #13

meta_service & server_state: support set/del/clear envs for app #13

LoveHeat commented Apr 16, 2018

qinzuoyan Apr 16, 2018

qinzuoyan Apr 16, 2018

neverchanje Apr 16, 2018 •

edited

Loading

qinzuoyan Apr 16, 2018

neverchanje Apr 16, 2018

neverchanje Apr 16, 2018 •

edited

Loading

neverchanje commented Apr 16, 2018

neverchanje commented Apr 16, 2018

qinzuoyan left a comment

qinzuoyan Apr 16, 2018

LoveHeat Apr 16, 2018

qinzuoyan Apr 16, 2018

LoveHeat Apr 16, 2018

qinzuoyan Apr 16, 2018

LoveHeat Apr 16, 2018

qinzuoyan Apr 16, 2018

LoveHeat Apr 16, 2018

qinzuoyan Apr 16, 2018

LoveHeat Apr 16, 2018

qinzuoyan Apr 16, 2018

qinzuoyan left a comment

meta_service & server_state: support set/del/clear envs for app #13

meta_service & server_state: support set/del/clear envs for app #13

Conversation

LoveHeat commented Apr 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neverchanje Apr 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neverchanje Apr 16, 2018 • edited Loading

Choose a reason for hiding this comment

neverchanje commented Apr 16, 2018

neverchanje commented Apr 16, 2018

qinzuoyan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qinzuoyan left a comment

Choose a reason for hiding this comment

neverchanje Apr 16, 2018 •

edited

Loading

neverchanje Apr 16, 2018 •

edited

Loading