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 window size config and change the default value #1239

Merged
merged 4 commits into from
Mar 19, 2024

Conversation

gengliqi
Copy link
Member

@gengliqi gengliqi commented Mar 19, 2024

Add GrpcInitialWindowSize and GrpcInitialConnWindowSize and change their default value from 1GiB to 128MiB.

The default value 1GiB may cause a TiDB OOM issue, see pingcap/tidb#49353.

The initial default value came from this PR pingcap/tidb#3377 where the author referred to pingcap/tidb#3377 (comment) as the evidence to justify the 1GiB default value. In this referenced document, the http2-window size is set to 1MiB and http2-connection-window is set to 1GiB. However, this document did not clearly describe how the test was done. I guess the key factor in getting faster results is changing http2-window from 64KiB to 1MiB, not changing http2-connection-window to 1GiB because at the top of the doc said the speed was just 4MiB/s if not using http2.

There is a lot of evidence that 1GiB is too large.

  1. If these two values are not set, grpc-go will use the dynamic window size and the maximum of this dynamic window size is 16MiB. https://github.com/grpc/grpc-go/blob/dadbbfa2863a67ed640aac924b7b7fd18b50a429/internal/transport/bdp_estimator.go#L30
  2. In cockroachdb master branch, the default values of GrpcInitialWindowSize and GrpcInitialConnWindowSize are 2MiB and 32MiB, respectively. The maximum of these two values is 64MiB. https://github.com/cockroachdb/cockroach/blob/master/pkg/rpc/settings.go#L69-L70. (rpc: permit gRPC stream window sizes up to 64 MB cockroachdb/cockroach#111255)

As a result, changing both default values to 128MiB will not likely cause performance issues but will likely reduce many TiDB OOM issues.

@gengliqi gengliqi requested a review from disksing March 19, 2024 10:45
Signed-off-by: gengliqi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants