Skip to content
This repository has been archived by the owner on Feb 21, 2024. It is now read-only.

bump MaxIdleConns(PerHost) like v0.7 #934

Merged
merged 1 commit into from
Nov 9, 2017

Conversation

jaffee
Copy link
Member

@jaffee jaffee commented Nov 6, 2017

Overview

This helps "connection reset" issues when the cluster is under high query load
from many clients.

This also brings in the default http transport options which were (mistakenly?)
removed by the TLS work. I'm not sure what the ramifications would be of using a
blank http.Transport{} instead of http.DefaultTransport, but it seems best to
avoid a sweeping change.

Fixes #906

Pull request checklist

Code review checklist

This is the checklist that the reviewer will follow while reviewing your pull request. You do not need to do anything with this checklist, but be aware of what the reviewer will be looking for.

  • Ensure that any changes to external docs have been included in this pull request.
  • If the changes require that minor/major versions need to be updated, tag the PR appropriately.
  • Ensure the new code is properly commented and follows Idiomatic Go.
  • Check that tests have been written and that they cover the new functionality.
  • Run tests and ensure they pass.
  • Build and run the code, performing any applicable integration testing.

This helps "connection reset" issues when the cluster is under high query load
from many clients.

This also brings in the default http transport options which were (mistakenly?)
removed by the TLS work. I'm not sure what the ramifications would be of using a
blank http.Transport{} instead of http.DefaultTransport, but it seems best to
avoid a sweeping change.
@jaffee jaffee requested review from yuce and tgruben November 6, 2017 16:29
Copy link
Contributor

@yuce yuce left a comment

Choose a reason for hiding this comment

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

That makes sense. I assumed http.Transport{} would mean the default options, but probably that's not the case.

@jaffee
Copy link
Member Author

jaffee commented Nov 6, 2017

yea @yuce - I would have thought so too, but I discovered this in my recent deep dive into net/http - the blank http.Client{} actually uses http.DefaultTransport which has a bunch of options already set.

@jaffee jaffee merged commit d83f2c6 into FeatureBaseDB:master Nov 9, 2017
@jaffee jaffee deleted the http-client-config branch June 20, 2018 00:28
seebs pushed a commit to seebs/pilosa that referenced this pull request Nov 19, 2020
Collect size-on-disk usage data from all nodes
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can not assign requested address error
2 participants