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

Add gRPC local subchannel support for tikv client. #338

Closed
wants to merge 2 commits into from

Conversation

blacktear23
Copy link

As default configuration gRPC will reuse TCP connections to each TiKV server what ever how many TiKV client instance created. This PR will enable grpc. use_local_subchannel_pool settings to ensure every TiKV client will create it self owned TCP connections to TiKV (or PD).

This will help to use more CPU when heavy load on gRPC calls. And as default settings it will make user confused when create many TiKV client but OS shows only one TCP connection to each TiKV server.

Signed-off-by: yulai.li <blacktear23@gmail.com>
Copy link
Collaborator

@ekexium ekexium left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test for this change?

let cb = ChannelBuilder::new(env)
.keepalive_time(Duration::from_secs(10))
.keepalive_timeout(Duration::from_secs(3));
.keepalive_timeout(Duration::from_secs(3))
.raw_cfg_int(CString::new("grpc.use_local_subchannel_pool").unwrap(), 1);
Copy link
Collaborator

@ekexium ekexium Mar 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

raw_cfg_int is not intended to be used directly according to its doc. I think it would be better to add a method grpc-rs to set it.

Copy link
Author

@blacktear23 blacktear23 Mar 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I will create a PR for grpc-rs first. But this is the easy way to set the configuration at rust-client side.
And why not grpc-rs not provide a method like set_config_int(&str, int).

@blacktear23
Copy link
Author

@ekexium tikv/grpc-rs#565 is created.

@yongman
Copy link
Contributor

yongman commented Jul 26, 2022

@blacktear23 any update to this pr?
Or I have made an alternative pr #361 for this.

@blacktear23
Copy link
Author

@yongman nothing changed, we can go ahead with your pr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants