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

server: change MAX_GRPC_SEND_MSG_LEN #2040

Merged
merged 4 commits into from Jul 17, 2017

Conversation

Projects
None yet
4 participants
@choleraehyq

Set region size to 256MB may cause grpc msg length bigger than 128M.

@zhangjinpeng1987 @disksing PTAL

Signed-off-by: Cholerae Hu huyingqian@pingcap.com

Show outdated Hide outdated src/server/server.rs
@@ -33,7 +33,7 @@ use super::raft_client::RaftClient;
const DEFAULT_COPROCESSOR_BATCH: usize = 50;
const MAX_GRPC_RECV_MSG_LEN: usize = 10 * 1024 * 1024;
const MAX_GRPC_SEND_MSG_LEN: usize = 128 * 1024 * 1024;
const MAX_GRPC_SEND_MSG_LEN: usize = 8 * 1024 * 1024 * 1024;

This comment has been minimized.

@BusyJay

BusyJay Jul 14, 2017

Contributor

Why not update it according to the size of region?

@BusyJay

BusyJay Jul 14, 2017

Contributor

Why not update it according to the size of region?

Show outdated Hide outdated src/server/server.rs
@@ -33,7 +34,7 @@ use super::raft_client::RaftClient;
const DEFAULT_COPROCESSOR_BATCH: usize = 50;
const MAX_GRPC_RECV_MSG_LEN: usize = 10 * 1024 * 1024;
const MAX_GRPC_SEND_MSG_LEN: usize = 128 * 1024 * 1024;
const MAX_GRPC_SEND_MSG_LEN: usize = REGION_SPLIT_SIZE as usize * 1024 * 1024;

This comment has been minimized.

@zhangjinpeng1987

zhangjinpeng1987 Jul 14, 2017

Member

How about 128 or 64 * REGION_SPLIT_SIZE?

@zhangjinpeng1987

zhangjinpeng1987 Jul 14, 2017

Member

How about 128 or 64 * REGION_SPLIT_SIZE?

Cholerae Hu added some commits Jul 14, 2017

Cholerae Hu
server: change MAX_GRPC_SEND_MSG_LEN to 8g
Signed-off-by: Cholerae Hu <huyingqian@pingcap.com>
Cholerae Hu
server: MAX_GRPC_SEND_MSG_LEN
Signed-off-by: Cholerae Hu <huyingqian@pingcap.com>
Show outdated Hide outdated src/server/server.rs
@@ -33,7 +34,7 @@ use super::raft_client::RaftClient;
const DEFAULT_COPROCESSOR_BATCH: usize = 50;
const MAX_GRPC_RECV_MSG_LEN: usize = 10 * 1024 * 1024;
const MAX_GRPC_SEND_MSG_LEN: usize = 128 * 1024 * 1024;
const MAX_GRPC_SEND_MSG_LEN: usize = 128 * REGION_SPLIT_SIZE as usize * 1024 * 1024;

This comment has been minimized.

@siddontang

siddontang Jul 16, 2017

Contributor

REGION_SPLIT_SIZE is already 256 * 1024 * 1024. I think you use a huge length here.

@siddontang

siddontang Jul 16, 2017

Contributor

REGION_SPLIT_SIZE is already 256 * 1024 * 1024. I think you use a huge length here.

This comment has been minimized.

@siddontang

siddontang Jul 16, 2017

Contributor

If you want to use REGION_SPLIT_SIZE, use a small multiplier, not 128 * 1024 * 1024, the bad value here.

Or use a fixed value directly, like 1 GB, 4 GB or 8 GB.

@siddontang

siddontang Jul 16, 2017

Contributor

If you want to use REGION_SPLIT_SIZE, use a small multiplier, not 128 * 1024 * 1024, the bad value here.

Or use a fixed value directly, like 1 GB, 4 GB or 8 GB.

This comment has been minimized.

@choleraehyq

choleraehyq Jul 16, 2017

Yeah, my fault.

@choleraehyq

choleraehyq Jul 16, 2017

Yeah, my fault.

@choleraehyq choleraehyq changed the title from server: change MAX_GRPC_SEND_MSG_LEN to 8g to server: change MAX_GRPC_SEND_MSG_LEN Jul 16, 2017

Cholerae Hu
server: adjust MAX_GRPC_SEND_MSG_LEN
Signed-off-by: Cholerae Hu <huyingqian@pingcap.com>
Show outdated Hide outdated src/server/server.rs
@@ -33,7 +34,7 @@ use super::raft_client::RaftClient;
const DEFAULT_COPROCESSOR_BATCH: usize = 50;
const MAX_GRPC_RECV_MSG_LEN: usize = 10 * 1024 * 1024;
const MAX_GRPC_SEND_MSG_LEN: usize = 128 * 1024 * 1024;
const MAX_GRPC_SEND_MSG_LEN: usize = 128 * REGION_SPLIT_SIZE as usize;

This comment has been minimized.

@BusyJay

BusyJay Jul 17, 2017

Contributor

I think 1.5~2 should be enough.

@BusyJay

BusyJay Jul 17, 2017

Contributor

I think 1.5~2 should be enough.

Cholerae Hu
server: adjust MAX_GRPC_SEND_MSG_LEN
Signed-off-by: Cholerae Hu <huyingqian@pingcap.com>
@choleraehyq

This comment has been minimized.

Show comment
Hide comment

@BusyJay Updated to 4x.

@BusyJay

This comment has been minimized.

Show comment
Hide comment
@BusyJay

BusyJay Jul 17, 2017

Contributor

LGTM

Contributor

BusyJay commented Jul 17, 2017

LGTM

@choleraehyq choleraehyq merged commit 8ddc288 into master Jul 17, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@choleraehyq choleraehyq deleted the hyq/grpcsize branch Jul 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment