-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[ws-proxy] tune idle connection pool #4461
Conversation
2450f1d
to
a3d2511
Compare
Setting 0 and 32 did help, but it is slow. I would play a bit more with Caddy and ws-proxy settings to see whether it can be faster. |
It comes from 1 mis 20s to just 20s if 100 connections per host are allowed, let's see if 1000 makes bigger difference. Updated: 1000 actually makes everything slower again, will stick with 100. |
b9a2461
to
4054d62
Compare
Codecov Report
@@ Coverage Diff @@
## main #4461 +/- ##
=========================================
+ Coverage 0 67.32% +67.32%
=========================================
Files 0 7 +7
Lines 0 808 +808
=========================================
+ Hits 0 544 +544
- Misses 0 214 +214
- Partials 0 50 +50
Continue to review full report at Codecov.
|
By default the connection pool is capped only by 2 connections per host. If a client tries to open many connections then latency increased significantly, see https://stackoverflow.com/a/60114945/961588. This commit configures the connection pool be unlimited but capped by 100 connections per host. It was inspired by measurement in Caddy: caddyserver/caddy#2805
FYI: I'm using this branch to work on VS Code please don't merge. I don't have other env which allow to open VS Code from sources reliably. |
Also, this is hard to judge in preview envs because there we have 3 proxies in a row: staging proxy -> proxy -> ws-proxy |
@geropl How should we proceed then? It can be load tested only on staging? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works as expected. Takes more than 20s with my connection, but absolutely no timeouts whatsoever. 👍
@akosyakov IMO we should merge as we're lagging the prober tools (per-branch staging) to test this, and the changes are reasonable. We can still test and fine-tune on staging if necessary.
What it does
By default the connection pool is capped only by 2 connections per host.
If a client tries to open many connections then latency increased significantly, see https://stackoverflow.com/a/60114945/961588. This commit configures the connection pool be unlimited but capped by 100 connections per host. It was inspired by measurement in Caddy: caddyserver/caddy#2805
I tried different values:
How to test
We should also have some load testing.