You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by GitBox <gi...@apache.org> on 2021/07/14 12:32:39 UTC
[GitHub] [incubator-brpc] Huixxi opened a new pull request #1478: bvar supports non-uniform user customized name
Huixxi opened a new pull request #1478:
URL: https://github.com/apache/incubator-brpc/pull/1478
Some user customized bvar names may have specific means, like "3AE5D89" will be normalized to "3_AE_5_D_89" by default that may cause trouble to the configuration such as monitoring. To solve this problem, bvar should support non-uniform customized name when users want to.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] wasphin commented on a change in pull request #1478: bvar supports non-uniform user customized name
Posted by GitBox <gi...@apache.org>.
wasphin commented on a change in pull request #1478:
URL: https://github.com/apache/incubator-brpc/pull/1478#discussion_r674508933
##########
File path: src/bvar/variable.h
##########
@@ -228,6 +228,11 @@ class Variable {
// RPCTest -> rpctest
// HELLO -> hello
void to_underscored_name(std::string* out, const butil::StringPiece& name);
+// no normalization
+void no_name_normalization(std::string* out, const base::StringPiece& name);
Review comment:
这个函数名可能不是很恰当.
##########
File path: src/bvar/variable.cpp
##########
@@ -147,12 +150,12 @@ int Variable::expose_impl(const butil::StringPiece& prefix,
_name.clear();
_name.reserve((prefix.size() + name.size()) * 5 / 4);
if (!prefix.empty()) {
- to_underscored_name(&_name, prefix);
+ g_name_normalization(&_name, prefix);
if (!_name.empty() && butil::back_char(_name) != '_') {
_name.push_back('_');
Review comment:
包不包括 `prefix` 和 `name` 间的 `_` 呢? 是不是补个说明比较好?
##########
File path: src/bvar/variable.cpp
##########
@@ -832,6 +835,14 @@ const bool ALLOW_UNUSED dummy_bvar_dump_prefix = ::GFLAGS_NS::RegisterFlagValida
&FLAGS_bvar_dump_prefix, wakeup_dumping_thread);
const bool ALLOW_UNUSED dummy_bvar_dump_tabs = ::GFLAGS_NS::RegisterFlagValidator(
&FLAGS_bvar_dump_tabs, wakeup_dumping_thread);
+
+void set_name_normalization_func(name_normalization_func func_name) {
+ g_name_normalization = func_name;
Review comment:
是否需要检查有效性呢? 从实现上看, 函数必须是有效的, 因为调用部分未做检查.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] Huixxi commented on a change in pull request #1478: bvar supports non-uniform user customized name
Posted by GitBox <gi...@apache.org>.
Huixxi commented on a change in pull request #1478:
URL: https://github.com/apache/incubator-brpc/pull/1478#discussion_r674529725
##########
File path: src/bvar/variable.h
##########
@@ -228,6 +228,11 @@ class Variable {
// RPCTest -> rpctest
// HELLO -> hello
void to_underscored_name(std::string* out, const butil::StringPiece& name);
+// no normalization
+void no_name_normalization(std::string* out, const base::StringPiece& name);
Review comment:
这个函数的目的是提供一个不做归一化的函数，同时不影响原有的默认归一化逻辑，如果用户想保留原始bvar名字，则直接使用该函数替换默认的to_underscored_name函数，请问对于该函数的命名有什么好的建议么？
--
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.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] Huixxi commented on a change in pull request #1478: bvar supports non-uniform user customized name
Posted by GitBox <gi...@apache.org>.
Huixxi commented on a change in pull request #1478:
URL: https://github.com/apache/incubator-brpc/pull/1478#discussion_r674529938
##########
File path: src/bvar/variable.cpp
##########
@@ -832,6 +835,14 @@ const bool ALLOW_UNUSED dummy_bvar_dump_prefix = ::GFLAGS_NS::RegisterFlagValida
&FLAGS_bvar_dump_prefix, wakeup_dumping_thread);
const bool ALLOW_UNUSED dummy_bvar_dump_tabs = ::GFLAGS_NS::RegisterFlagValidator(
&FLAGS_bvar_dump_tabs, wakeup_dumping_thread);
+
+void set_name_normalization_func(name_normalization_func func_name) {
+ g_name_normalization = func_name;
Review comment:
是检查传入函数func_name的有效性么？
--
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.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] Huixxi commented on a change in pull request #1478: bvar supports non-uniform user customized name
Posted by GitBox <gi...@apache.org>.
Huixxi commented on a change in pull request #1478:
URL: https://github.com/apache/incubator-brpc/pull/1478#discussion_r674528929
##########
File path: src/bvar/variable.cpp
##########
@@ -147,12 +150,12 @@ int Variable::expose_impl(const butil::StringPiece& prefix,
_name.clear();
_name.reserve((prefix.size() + name.size()) * 5 / 4);
if (!prefix.empty()) {
- to_underscored_name(&_name, prefix);
+ g_name_normalization(&_name, prefix);
if (!_name.empty() && butil::back_char(_name) != '_') {
_name.push_back('_');
Review comment:
prefix 和 name 间的 _是总会有的，不包括在名字归一化处理里面的，请问是需要在提交的时候补充说明一下么？
--
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.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] Huixxi commented on a change in pull request #1478: bvar supports non-uniform user customized name
Posted by GitBox <gi...@apache.org>.
Huixxi commented on a change in pull request #1478:
URL: https://github.com/apache/incubator-brpc/pull/1478#discussion_r674529938
##########
File path: src/bvar/variable.cpp
##########
@@ -832,6 +835,14 @@ const bool ALLOW_UNUSED dummy_bvar_dump_prefix = ::GFLAGS_NS::RegisterFlagValida
&FLAGS_bvar_dump_prefix, wakeup_dumping_thread);
const bool ALLOW_UNUSED dummy_bvar_dump_tabs = ::GFLAGS_NS::RegisterFlagValidator(
&FLAGS_bvar_dump_tabs, wakeup_dumping_thread);
+
+void set_name_normalization_func(name_normalization_func func_name) {
+ g_name_normalization = func_name;
Review comment:
是检查传入函数func_name的有效性么？是否为NULL？
--
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.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org