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

High memory footprint per connection #5751

Closed
atollena opened this issue Oct 31, 2022 · 1 comment
Closed

High memory footprint per connection #5751

atollena opened this issue Oct 31, 2022 · 1 comment

Comments

@atollena
Copy link
Collaborator

atollena commented Oct 31, 2022

gRPC-go allocates two buffers per transport: one for reads, and one for writes. They are allocated on transport creation in http_util.go. With their respective default sizes of 32KB and 64KB, those buffers can account for a significant part of grpc-go memory footprint. This is especially noticeable when there are a high number of mostly idle connections.

As an example, with 1000 connections open, those buffers consume 100MB of memory. We first noticed this problem server-side in our deployments when using the round robin balancer in services that have a lot of clients (in this use case, setting up some form of subsetting would help). Another scenario where this issue is prominent is for control planes with streaming, such as xDS management servers.

It is possible to disable or to tune the size of those buffers by setting ReadBufferSize/WriteBufferSize on the server side and WithReadBufferSize/WithWriteBufferSize on the client side, but there are performance implications. From reading the comments and code, those buffers are meant to batch calls to the transport, thereby reducing the number of system calls: the code dealing with frames reads and writes with size 9 bytes (HTTP2 frame header to get the frame size) immediately followed by a read or write of the size of the frame. This means that in most cases, when the socket becomes readable or there is data to be written, at least two syscalls can be batched.

From what I could read, the benchmarks present in this repository focus on latency and throughput, and it is not surprising that those buffers improve the results. Note also that the benchmarks override the buffer size to 128KB, both for servers and clients. This is a problem because some benchmark results may not reflect the reality of what most users are experiencing. The benchmarks do not account for fixed memory usage per connection.

Things I'm trying to get out of this issue are:

  1. Gains to be had by changing the implementation without significant performance loss. In particular, it is unclear to me why the write buffer does not use bufio.Writer and instead has a custom implementation that allocates a buffer of twice the passed in size (2*32KB by default). This was introduced in this change: transport: refactor to reduce lock contention and improve performance #1962.
  2. Make sure that benchmarks use the default values for buffer sizes -- I'm happy to propose the fix for this.
  3. Opportunities for documenting how to tune those values based on use cases.
  4. Explore whether there is value in adding read and write buffer sizes as a benchmark dimension, as well as measuring the cost of idle connections.
  5. More creative ideas to improve memory usage when there are many mostly idle connections (e.g. dynamically tune buffer size based on activity). We are somewhat limited by the way IOs are done in go: we can't just allocate memory when a call to read is ready, since socket readiness and read are not separate (see net: add mechanism to wait for readability on a TCPConn golang/go#15735).
@easwars
Copy link
Contributor

easwars commented Nov 1, 2022

@atollena : We discussed this issue today and we've decided to create issues for the list of things you've mentioned in here. That way, we can keep the discussions scoped in the respective issues/PRs.

Thanks for filing this issue.

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

No branches or pull requests

3 participants