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

Improve http client set up #76

Open
andrefurlan-db opened this issue Dec 12, 2022 · 2 comments
Open

Improve http client set up #76

andrefurlan-db opened this issue Dec 12, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@andrefurlan-db
Copy link
Contributor

andrefurlan-db commented Dec 12, 2022

Reuse http client across connections

Go http client is meant to be reused as it has its own connection pool. This would reduce the need to many https handshakes on connection creation.
Be careful to not reuse the thrift client as that one is not thread safe.

Increase http client MaxIdleConnsPerHost

Our use case it pretty much all about connecting to the same host. So we should set MaxIdleConnsPerHost to a number way higher than default 2.

@andrefurlan-db andrefurlan-db added the enhancement New feature or request label Dec 12, 2022
@zcking
Copy link

zcking commented Dec 14, 2022

@andrefurlan-db the most common tuned value I've seen for MaxIdleConnsPerHost is 100. I definitely think it's a great idea to increase this from the default value of 2, but should this value also be configurable by users, such as from query params in the DSN?

@andrefurlan-db
Copy link
Contributor Author

Hi @zcking , until we have some real requests and use cases on why setting this is important to our users, I will prefer to not make it configurable. So, I am not disagreeing with you, I just want to wait and see if there is demand for such low consequence knob.

@andrefurlan-db andrefurlan-db linked a pull request Dec 16, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants