Skip to content

Open fewer connections to rex servers - #3645

Open
peterebden wants to merge 1 commit into
thought-machine:masterfrom
peterebden:peter/cas-connections
Open

peterebden wants to merge 1 commit into
thought-machine:masterfrom
peterebden:peter/cas-connections

Conversation

@peterebden

Copy link
Copy Markdown
Collaborator

Also makes the limit of CAS connections configurable.

remote-apis-sdks changed a while back to eagerly open 25 connections to each server. I think that's fairly antisocial (also generates a lot of noise in our logs) and mostly unnecessary; it seems to be working around an older gRPC bug about what happens if you have a bunch of long-running RPCs on a single connection.

It's a bit fiddly because we need to support different possible infrastructure setups:

  • Layer 4 load balancing (notably within Kubernetes via services, which is relevant to us internally) which doesn't have any limits on what happens within a connection, but also doesn't itself load balance within a connection
  • Layer 7 load balancing (e.g. through a GCP or AWS ALB) which will load-balance on its backends but may impose limits of streams per connection.

We then have two different use cases, which currently have the same "always 25" policy, but are actually completely opposite to one another:

  • The CAS connections run relatively short, high-bandwidth RPCs. We default to 4 here which should do a reasonable job of maximising network resources from the client machine; more seems not very useful.
  • The Execute connections are Execute or WaitExecution calls which are very low-bandwidth and much longer-lived. We base this off 1 per 64 workers to try to avoid any of the aforementioned issues with stream limits per connection; this might be a bit defensive possibly but it should be fairly safe.

In any case we should end up with significantly fewer connections opened, reducing the load on the client (connections & TLS handshakes) and more notably on the servers.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant