Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support customized server bvar prefix #1854

Merged
merged 1 commit into from
Aug 19, 2022

Conversation

jenrryyou
Copy link
Contributor

通过ServerOption自定义Server的Bvar prefix。
目前bvar默认的前缀是rpc_server_{server监听的port},但是存在如下问题:

  1. 由于port改变(如使用range port或者其他port可能会改变的场景)导致bvar name变化,使得配置的dashboard和监控失效
  2. 为了支持IPV6和Unix Domain socket,引入了ExtendedEndPoint,不过生成prefix存在BUG,会固定用EXTENDED_ENDPOINT_PORT(123456789)生成前缀(rpc_server_123456789),导致不同的Server bvar前缀相同。虽然修复不难,但是Unix Domain socket不太好处理,因为没有port信息,如果用file_path的话又太长。

基于以上考虑,最好还是用户通过ServerOption传入Server name生成prefix,如果name为空,则保持原来的逻辑。

@jenrryyou jenrryyou marked this pull request as ready for review July 24, 2022 14:45
@wwbmmm
Copy link
Contributor

wwbmmm commented Jul 25, 2022

百度内部是通过gflag来自定义server prefix的,代码大致如下:

std::string Server::ServerPrefix() const {
    if (FLAGS_customized_server_prefix.empty()) {
         return base::string_printf("%s_%d", g_server_info_prefix, listen_address().port);
    } else {
        return FLAGS_customized_server_prefix;
    }
}

可以看下哪种方式更方便一些。

@jenrryyou
Copy link
Contributor Author

jenrryyou commented Jul 25, 2022

百度内部是通过gflag来自定义server prefix的,代码大致如下:

std::string Server::ServerPrefix() const {
    if (FLAGS_customized_server_prefix.empty()) {
         return base::string_printf("%s_%d", g_server_info_prefix, listen_address().port);
    } else {
        return FLAGS_customized_server_prefix;
    }
}

可以看下哪种方式更方便一些。

用gflag如果有多个server的情况不能区分各自server的指标,个人觉得这类server spefic的属性放server option更合理一些。

@wwbmmm
Copy link
Contributor

wwbmmm commented Aug 1, 2022

用gflag如果有多个server的情况不能区分各自server的指标,个人觉得这类server spefic的属性放server option更合理一些。

LGTM

src/brpc/server.cpp Outdated Show resolved Hide resolved
src/brpc/server.h Outdated Show resolved Hide resolved
@jenrryyou jenrryyou force-pushed the customized_brpc_bvar_prefix branch from 1df96d7 to 0467824 Compare August 19, 2022 02:36
@serverglen serverglen merged commit cf0aa33 into apache:master Aug 19, 2022
@Huixxi Huixxi added enhancement improvements on existing features discussion open problems and feature requests labels Oct 10, 2022
@jenrryyou jenrryyou deleted the customized_brpc_bvar_prefix branch October 15, 2022 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion open problems and feature requests enhancement improvements on existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants