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

Allow configuring the dispatcher and connection pool of the KubernetesClient shared underlying http client. #8892

Merged
merged 2 commits into from Feb 26, 2018

Conversation

davidfestal
Copy link
Contributor

What does this PR do?

Allow configuring the dispatcher and connection pool of
the KubernetesClient shared underlying http client.

For example, default values are 64 concurrent asynchronous
requests, and only 5 concurrent asynchronous requests per-host,
which obviously doesn't seem correct for multi-user scenarios,
knowing that Che keeps a number of connections opened
(e.g. for commands or ws-agent logs)

Leaving these wrong default values can lead to workspace
starts being stuck in the middle of the process while waiting
for an available request, as soon as several users start
a workspace on the same che-server.

... of the `KubernetesClient` shared underlying http client.

For example, default values are 64 concurrent asynchronous
requests, and **only 5 concurrent asynchronous requests per-host**,
which obviously doesn't seem correct for multi-user scenarios,
knowing that Che keeps a number of connections opened
(e.g. for commands or ws-agent logs)

Leaving these wrong default values can lead to workspace
starts being stuck in the middle of the process while waiting 
for an available request, as soon as several users start
a workspace on the same che-server.

Signed-off-by: David Festal <dfestal@redhat.com>
@codenvy-ci
Copy link

Can one of the admins verify this patch?

@benoitf benoitf added the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Feb 23, 2018
@davidfestal
Copy link
Contributor Author

ci-test-ocp

@codenvy-ci
Copy link

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:8892
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@skabashnyuk
Copy link
Contributor

Maybe we should move httpClient creation in dedicated class HttpClientProvider it will reduce the size of KubernetesClientFactory constructor?

@davidfestal
Copy link
Contributor Author

Maybe we should move httpClient creation in dedicated class HttpClientProvider it will reduce the size of KubernetesClientFactory constructor?

Why not in a next PR ;-)

# Keep-alive timeout of the connection pool
# of the Kubernetes-client shared http client
# in minutes
che.infra.kubernetes.client.http.connection_pool.keep_alive.mins=5

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We practice separation of measurement unit by underscore instead of a dot

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit e88a264

... as requested by @garagatyi in [this
comment](#8892 (comment))

Signed-off-by: David Festal <dfestal@redhat.com>
@davidfestal davidfestal merged commit 9232fe1 into master Feb 26, 2018
@benoitf benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Feb 26, 2018
@benoitf benoitf added this to the 6.2.0 milestone Feb 26, 2018
riuvshin pushed a commit that referenced this pull request Feb 26, 2018
…esClient` shared underlying http client. (#8892)

Allow configuring the dispatcher and connection pool of
the `KubernetesClient` shared underlying http client.

For example, default values are 64 concurrent asynchronous
requests, and **only 5 concurrent asynchronous requests per-host**,
which obviously doesn't seem correct for multi-user scenarios,
knowing that Che keeps a number of connections opened
(e.g. for commands or ws-agent logs)

Leaving these wrong default values can lead to workspace
starts being stuck in the middle of the process while waiting
for an available request, as soon as several users start
a workspace on the same che-server.

Signed-off-by: David Festal <dfestal@redhat.com>
@skabashnyuk skabashnyuk deleted the configure_http_client_connections branch March 13, 2018 19:38
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.

None yet

9 participants