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.

Sign up for GitHub

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

Jump to bottom

Bump default of HTTPMaxConnsPerClient to 200 #7289

Merged
merged 1 commit into from
Feb 13, 2020
Merged

Bump default of HTTPMaxConnsPerClient to 200 #7289

merged 1 commit into from
Feb 13, 2020

Conversation

hanshasselberg
Copy link
Member

@hanshasselberg hanshasselberg commented Feb 13, 2020 •
edited
Loading

Fixes #7284.

Todo:

  • where do we call out this problem for vault with consul 1.7.0 and 1.6.3?
    • discuss.hashicorp.com?

@hanshasselberg hanshasselberg added waiting-reply Waiting on response from Original Poster or another individual in the thread backport/1.6 labels Feb 13, 2020
@hanshasselberg hanshasselberg self-assigned this Feb 13, 2020
@hanshasselberg hanshasselberg requested a review from a team February 13, 2020 13:40
mkeeler
mkeeler approved these changes Feb 13, 2020
View reviewed changes
Copy link
Member

@mkeeler mkeeler left a comment

Choose a reason for hiding this comment

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

LGTM

mkeeler
mkeeler requested changes Feb 13, 2020
View reviewed changes
Copy link
Member

@mkeeler mkeeler left a comment

Choose a reason for hiding this comment

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

Actually, I think the limit that needs to be raised is probably http_max_conns_per_client

@hanshasselberg hanshasselberg changed the title Bump default of RPCMaxConnsPerClient to 200 Bump default of HTTPMaxConnsPerClient to 200 Feb 13, 2020
mkeeler
mkeeler approved these changes Feb 13, 2020
View reviewed changes
Copy link
Member

@mkeeler mkeeler left a comment

Choose a reason for hiding this comment

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

:shipit:

@hanshasselberg hanshasselberg added this to the 1.7.1 milestone Feb 13, 2020
@hanshasselberg hanshasselberg merged commit cb0f944 into master Feb 13, 2020
@hanshasselberg hanshasselberg deleted the bump_rpc_defaults branch February 13, 2020 15:27
@jameshartig jameshartig mentioned this pull request Feb 29, 2020
Closed
@jameshartig
Copy link
Contributor

jameshartig commented Feb 29, 2020 •
edited
Loading

I'm not sure if it's something worth mentioning with the vault case as well but I ran into an issue with prometheus and this limit as well: prometheus/prometheus#6640. I ended up raising the limit to 500 for now to resolve the issue.

Can we add metrics to track the http connections and tcp connections? It would've helped with debugging and understanding what the limit should be raised to, if necessary.

@ghost ghost removed the waiting-reply Waiting on response from Original Poster or another individual in the thread label Feb 29, 2020
@BijanJohn
Copy link

Ran into this same issue in staging today after upgrading consul to 1.7.1. Finally realized that the max conns needed to be bumped. Exposing the metric would be helpful.

@obourdon
Copy link

Might also be related to issue #7259

@jsosulska jsosulska mentioned this pull request May 4, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkeeler mkeeler mkeeler approved these changes

Assignees

@hanshasselberg hanshasselberg

Labels
None yet
Projects
None yet
Milestone
1.7.1
Development

Successfully merging this pull request may close these issues.

Vault default parallelism with Consul backend exceeds new Consul 1.6.3 default limits
5 participants
@hanshasselberg @jameshartig @BijanJohn @obourdon @mkeeler