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.

Sign up for GitHub

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

Jump to bottom

go.mod: upgrade gRPC #134971

Closed
tbg opened this issue Nov 12, 2024 · 1 comment · May be fixed by #136278
Closed

go.mod: upgrade gRPC #134971

tbg opened this issue Nov 12, 2024 · 1 comment · May be fixed by #136278
Assignees
@tbg
Labels
A-server-networking Pertains to network addressing,routing,initialization C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team

Comments

@tbg
Copy link
Member

tbg commented Nov 12, 2024 •
edited
Loading

We should upgrade gRPC. There are a number of perf improvements, for example:

codec: Implement a new Codec which uses buffer recycling for encoded message (#7356)
introduce a mem package to facilitate buffer reuse (#7432)
client: Improve RPC performance by reducing work while holding a lock (#7132)
*: Allow building without x/net/trace by using grpcnotrace to enable dead code elimination (#6954)
rand: improve performance and simplify implementation of grpcrand by adopting math/rand's top-level functions for go version 1.21.0 and newer. (#6925)
grpc: skip compression of empty messages as an optimization (#6842)
orca: use atomic pointer to improve performance in server metrics recorder (#6799)
client & server: Add experimental [With]SharedWriteBuffer to improve performance by reducing allocations when sending RPC messages. (Disabled by default.) (#6309)
server: improve stream handler goroutine worker allocation when NumStreamWorkers is used (#6004)

Jira issue: CRDB-44320
Epic: CRDB-43584
@tbg tbg added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team A-server-networking Pertains to network addressing,routing,initialization o-perf-efficiency Related to performance efficiency labels Nov 12, 2024
@tbg tbg self-assigned this Nov 12, 2024
@tbg tbg mentioned this issue Nov 12, 2024
Draft
@tbg tbg added the T-dev-inf label Nov 13, 2024
@tbg tbg assigned rickystewart and unassigned tbg Nov 13, 2024
@tbg tbg removed the T-kv KV Team label Nov 13, 2024
@tbg tbg mentioned this issue Nov 27, 2024
Draft
16 tasks
@tbg tbg assigned tbg and unassigned rickystewart Nov 27, 2024
@tbg tbg added T-kv KV Team and removed T-dev-inf labels Nov 27, 2024
This was referenced Dec 3, 2024
Merged
Closed
craig bot pushed a commit that referenced this issue Dec 10, 2024
@tbg
Merge #136558
bb4d357 
136558: rpc: add BenchmarkGRPCPing r=tbg a=tbg

This benchmarks simple unary requests across a gRPC server with
CockroachDB-specific settings (snappy compression, etc).

The benchmarks show a problematic amount of allocations especially
at small payload sizes, which is likely a common case in CRDB.

We also see that it's highly beneficial to reuse RPC streams, i.e.
to use bidirectional streaming RPCs to implement unary request-response RPCs.
This is because gRPC implements unary RPCs using one-off streams, which
incurs high overhead. I would liken it to CockroachDB using DistSQL for point
lookups.

<details><summary>Results</summary>
<p>

```
./dev bench --timeout 1h --ignore-cache --stream-output --bench-mem ./pkg/rpc --filter BenchmarkGRPCPing --test-args '-test.benchtime=5s -test.cpu=8 -test.count 7 -test.timeout=1h' 2>&1 | tee -a bench.txt
benchstat -col /rpc bench.txt
```

```
                         │  UnaryUnary  │            StreamStream            │
                         │    sec/op    │   sec/op     vs base               │
GRPCPing/bytes=______1-8   117.23µ ± 6%   63.78µ ± 6%  -45.60% (p=0.001 n=7)
GRPCPing/bytes=____256-8   119.66µ ± 2%   69.45µ ± 4%  -41.96% (p=0.001 n=7)
GRPCPing/bytes=___1024-8   118.16µ ± 5%   65.98µ ± 2%  -44.16% (p=0.001 n=7)
GRPCPing/bytes=___2048-8   127.25µ ± 3%   79.78µ ± 6%  -37.31% (p=0.001 n=7)
GRPCPing/bytes=___4096-8   141.04µ ± 1%   86.34µ ± 4%  -38.78% (p=0.001 n=7)
GRPCPing/bytes=___8192-8    174.0µ ± 4%   117.8µ ± 3%  -32.31% (p=0.001 n=7)
GRPCPing/bytes=__16384-8    241.6µ ± 2%   180.7µ ± 5%  -25.21% (p=0.001 n=7)
GRPCPing/bytes=__32768-8    349.8µ ± 2%   296.5µ ± 2%  -15.25% (p=0.001 n=7)
GRPCPing/bytes=__65536-8    495.1µ ± 2%   428.3µ ± 5%  -13.49% (p=0.001 n=7)
GRPCPing/bytes=_262144-8    1.418m ± 4%   1.386m ± 3%        ~ (p=0.053 n=7)
GRPCPing/bytes=1048576-8    6.002m ± 6%   5.885m ± 2%        ~ (p=0.259 n=7)
geomean                     301.1µ        214.6µ       -28.74%

                         │  UnaryUnary  │             StreamStream             │
                         │     B/s      │      B/s       vs base               │
GRPCPing/bytes=______1-8   400.4Ki ± 5%    732.4Ki ± 5%  +82.93% (p=0.001 n=7)
GRPCPing/bytes=____256-8   4.463Mi ± 2%    7.687Mi ± 4%  +72.22% (p=0.001 n=7)
GRPCPing/bytes=___1024-8   16.92Mi ± 5%    30.30Mi ± 2%  +79.09% (p=0.001 n=7)
GRPCPing/bytes=___2048-8   31.06Mi ± 3%    49.54Mi ± 6%  +59.50% (p=0.001 n=7)
GRPCPing/bytes=___4096-8   55.71Mi ± 1%    91.02Mi ± 5%  +63.37% (p=0.001 n=7)
GRPCPing/bytes=___8192-8   90.06Mi ± 4%   133.04Mi ± 3%  +47.73% (p=0.001 n=7)
GRPCPing/bytes=__16384-8   129.5Mi ± 2%    173.2Mi ± 5%  +33.70% (p=0.001 n=7)
GRPCPing/bytes=__32768-8   178.8Mi ± 2%    211.0Mi ± 2%  +18.00% (p=0.001 n=7)
GRPCPing/bytes=__65536-8   252.6Mi ± 2%    292.0Mi ± 5%  +15.60% (p=0.001 n=7)
GRPCPing/bytes=_262144-8   352.6Mi ± 4%    360.9Mi ± 4%        ~ (p=0.053 n=7)
GRPCPing/bytes=1048576-8   333.2Mi ± 6%    339.9Mi ± 2%        ~ (p=0.259 n=7)
geomean                    48.06Mi         67.42Mi       +40.27%

                         │  UnaryUnary   │            StreamStream             │
                         │     B/op      │     B/op      vs base               │
GRPCPing/bytes=______1-8   14.992Ki ± 2%   3.633Ki ± 2%  -75.77% (p=0.001 n=7)
GRPCPing/bytes=____256-8   17.512Ki ± 3%   6.460Ki ± 1%  -63.11% (p=0.001 n=7)
GRPCPing/bytes=___1024-8    26.63Ki ± 1%   15.67Ki ± 1%  -41.18% (p=0.001 n=7)
GRPCPing/bytes=___2048-8    38.52Ki ± 1%   27.67Ki ± 1%  -28.17% (p=0.001 n=7)
GRPCPing/bytes=___4096-8    65.89Ki ± 1%   54.97Ki ± 0%  -16.57% (p=0.001 n=7)
GRPCPing/bytes=___8192-8    116.9Ki ± 1%   105.4Ki ± 1%   -9.88% (p=0.001 n=7)
GRPCPing/bytes=__16384-8    215.6Ki ± 0%   203.5Ki ± 1%   -5.61% (p=0.001 n=7)
GRPCPing/bytes=__32768-8    459.8Ki ± 1%   444.5Ki ± 1%   -3.33% (p=0.001 n=7)
GRPCPing/bytes=__65536-8    808.4Ki ± 1%   791.8Ki ± 1%   -2.05% (p=0.001 n=7)
GRPCPing/bytes=_262144-8    3.289Mi ± 0%   3.276Mi ± 0%   -0.40% (p=0.017 n=7)
GRPCPing/bytes=1048576-8    12.94Mi ± 0%   12.94Mi ± 0%        ~ (p=0.053 n=7)
geomean                     182.4Ki        130.5Ki       -28.42%

                         │ UnaryUnary  │           StreamStream            │
                         │  allocs/op  │ allocs/op   vs base               │
GRPCPing/bytes=______1-8   182.00 ± 0%   46.00 ± 0%  -74.73% (p=0.001 n=7)
GRPCPing/bytes=____256-8   182.00 ± 0%   46.00 ± 0%  -74.73% (p=0.001 n=7)
GRPCPing/bytes=___1024-8   182.00 ± 0%   47.00 ± 0%  -74.18% (p=0.001 n=7)
GRPCPing/bytes=___2048-8   183.00 ± 0%   48.00 ± 2%  -73.77% (p=0.001 n=7)
GRPCPing/bytes=___4096-8   183.00 ± 0%   48.00 ± 0%  -73.77% (p=0.001 n=7)
GRPCPing/bytes=___8192-8   183.00 ± 0%   50.00 ± 2%  -72.68% (p=0.001 n=7)
GRPCPing/bytes=__16384-8   194.00 ± 0%   55.00 ± 2%  -71.65% (p=0.001 n=7)
GRPCPing/bytes=__32768-8   209.00 ± 0%   72.00 ± 1%  -65.55% (p=0.001 n=7)
GRPCPing/bytes=__65536-8   216.00 ± 0%   80.00 ± 1%  -62.96% (p=0.001 n=7)
GRPCPing/bytes=_262144-8    275.0 ± 0%   141.0 ± 1%  -48.73% (p=0.001 n=7)
GRPCPing/bytes=1048576-8    487.0 ± 1%   350.0 ± 3%  -28.13% (p=0.001 n=7)
geomean                     214.1        69.37       -67.60%
```

</p>
</details> 

Informs #134971.


Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>
@ajstorm ajstorm removed the o-perf-efficiency Related to performance efficiency label Dec 16, 2024
@ajstorm
Copy link
Collaborator

ajstorm commented Dec 16, 2024

Closing because we have no immediate plans to upgrade gRPC due to performance issues. Follow on work is continuing in #137252.

@ajstorm ajstorm closed this as completed Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tbg tbg

Labels
A-server-networking Pertains to network addressing,routing,initialization C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

DEPS: upgrade grpc to v1.68.0 tbg/cockroach
3 participants
@rickystewart @tbg @ajstorm