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
grpc: multi conns to send raft messages. #1921
Conversation
src/server/raft_client.rs
Outdated
.or_insert_with(|| Conn::new(env, addr)) | ||
} | ||
|
||
pub fn send(&mut self, addr: SocketAddr, msg: RaftMessage) -> Result<()> { | ||
let index = msg.get_region_id() as usize % self.conn_size; |
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.
Is this the same as random selection?
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.
no, to ensure the same region use the same connection
src/server/raft_client.rs
Outdated
} | ||
|
||
impl RaftClient { | ||
pub fn new(env: Arc<Environment>) -> RaftClient { | ||
pub fn new(env: Arc<Environment>, conn_size: usize) -> RaftClient { |
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.
use config will better?
# Conflicts: # etc/config-template.toml # src/bin/tikv-server.rs # src/server/config.rs # src/server/raft_client.rs # src/server/server.rs # tests/raftstore/server.rs
etc/config-template.toml
Outdated
@@ -25,6 +25,8 @@ messages-per-tick = 4096 | |||
# grpc-concurrency = 2 | |||
# The number of max concurrent streams/requests on a client connection. | |||
# grpc-concurrent-stream = 1024 | |||
# The number of connections to send raft messages. | |||
# grpc-raft-conn-size = 10 |
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.
size may be not proper here. num may be better.
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.
Rest LGTM
PTAL @javaforfun |
LGTM |
No description provided.