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

Unbounded number of connections to the HTTP proxy #678

Open
gabrielrussoc opened this issue Jul 19, 2023 · 3 comments
Open

Unbounded number of connections to the HTTP proxy #678

gabrielrussoc opened this issue Jul 19, 2023 · 3 comments

Comments

@gabrielrussoc
Copy link

gabrielrussoc commented Jul 19, 2023

Hi all,

I started exploring a two-layer deployment of a bazel remote where the first layer points to a second deployment of the bazel remote via the --http_proxy.url flag. I noticed a very high CPU usage on the first layer coming from dialling TCP connections:

Screenshot 2023-07-19 at 15 54 58

It turns out the proxy uses the default http client, which under the hood uses the default transport that does not limit the number of connections:

httpClient := &http.Client{}

From https://pkg.go.dev/net/http:

// MaxConnsPerHost optionally limits the total number of
// connections per host, including connections in the dialing,
// active, and idle states. On limit violation, dials will block.
//
// Zero means no limit.
MaxConnsPerHost int

Should we expose a knob to control this value and limit the amount of connections we open? In practice this ended up hogging all the CPU on the machine which causes all sorts of weirdness. It sounds reasonable to optionally block new connections instead, and perhaps even to introduce a metric to show the amount of active requests to the proxy

Happy to send a PR if this all makes sense

@mostynb
Copy link
Collaborator

mostynb commented Jul 19, 2023

Sounds like a nice feature to have. Is there a reasonable heuristic that could set this value automatically, or would this only be useful with a new configuration setting?

@gabrielrussoc
Copy link
Author

gabrielrussoc commented Jul 20, 2023

We could try for some multiple of the number of available CPUs, but this could cause unexpected regressions to folks already relying on the unbounded behaviour. I'd rather keep this obvious behind a flag to avoid any surprises when upgrading

@mostynb
Copy link
Collaborator

mostynb commented Jul 20, 2023

OK, sounds reasonable.

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

No branches or pull requests

2 participants