-
Notifications
You must be signed in to change notification settings - Fork 58
feat: support table-level slow query on meta server #314
Conversation
@@ -56,6 +56,10 @@ namespace replication { | |||
|
|||
static const char *lock_state = "lock"; | |||
static const char *unlock_state = "unlock"; | |||
// env name of table level latency | |||
static const std::string ENV_TABLE_LEVEL_GET_LATENCY("table_level_get_latency"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
环境变量的常量字符串放在meta-server的模块里?感觉和一起的环境变量的风格不太一致。解释一下原因?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
环境变量的常量字符串放在meta-server的模块里?感觉和一起的环境变量的风格不太一致。解释一下原因?
环境变量放在这里主要是用于检测用于设置的非法值。
rdsn中正常的设置环境变量的流程是:client向meta server发送请求设置环境变量,而replica通过定时向meta server查询的方式获取环境变量。
以前的环境变量设置过程中,在meta server中没有对非法值做检测,设置完之后直接返回给client。然而,只有当同步到replica时才进行判断,并将非法值抛弃。这样client无法获知该环境变量是否设置成功。所以在meta server中对该环境变量做有效值判断(>= 20ms)。
对于在rdsn中加入pegasus相关的环境变量,导致rdsn中侵入了pegasus的逻辑处理,之前咨询过@wutao, 说是rdsn以后会和pegasus代码合并到一起,所以暂时没考虑这个问题。
另外,环境变量的风格我会统一一下
src/dist/replication/test/meta_test/unit_test/meta_table_level_slow_query_test.cpp
Outdated
Show resolved
Hide resolved
src/dist/replication/test/meta_test/unit_test/meta_table_level_slow_query_test.cpp
Outdated
Show resolved
Hide resolved
return response.err; | ||
if (resp_task->error() != dsn::ERR_OK) { | ||
response.err = resp_task->error(); | ||
return response; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个条件判断是rpc出错,这时response其实是没有意义的,返回response也是没有意义的。我看comment里说将set_app_envs的返回值从error_code改成response是为了方便print hint_message,我觉得可以改成下面这样:
error_code set_app_envs(app_name, keys, values, &hint_message)
,把response的hint_message返回给shell就好
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
没太看懂什么意思,为啥要把hint_message单独拆出来值返回?我是觉得error_code和hint_message是相关联的,分开返回不是特别好
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
error_code和hint_message不总是相关联的,在rpc失败的时候就只有rpc的error_code,没有hint_message。在rpc失败的时候,并没有response,现在是怎么都会返回一个response,我是觉得这个有点奇怪。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我理解你的意思了。但是如果用参数返回的方式,如果某个调用不想要hint_message,也必须去创建一个变量传入接口,我感觉不是特别友好
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set_app_envs这个函数的hint_message应该都是有用的,若set成功,hint_message里是这个app的所有envs,若set失败,hint_message是对失败原因的解释。如果是从接口层面觉得不友好,还是可以延续原来的逻辑,在replica_ddl_client.cpp里面把hint_message print出来,只返回error_code
if (response.err != ERR_OK) {
// add hint_message to explain rpc failed
return response.err;
} else {
std::cout << "set app envs succeed" << std::endl;
if (!response.hint_message.empty()) {
std::cout << "=============================" << std::endl;
std::cout << response.hint_message << std::endl;
std::cout << "=============================" << std::endl;
}
}
What problem does this PR solve?
add the check for parameter of table level slow query, in the method of server_state::set_app_envs
What is changed and how it works?
1.Add the check for table level slow query parameter, which must be >= 20 * 1000 * 1000ns
2.The return of replication_ddl_client::set_app_envs is modified from dsn::error_code to configuration_update_app_env_response, so the shell can get the hit message and print it to terminal.
3.change the way of call to meta from request_meta to call_rpc_sync