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

[ffresty] maxIdleConnsPerHost Setting #154

Merged

Conversation

onelapahead
Copy link
Contributor

@onelapahead onelapahead commented Oct 22, 2024

Follow's up #153. Reading common performance problems in Go net/http, the maxConnsPerHost is helpful being unlimited by default but it sounds like the maxIdleConnsPerHost will always default to 2 based on the source:

// DefaultMaxIdleConnsPerHost is the default value of [Transport]'s
// MaxIdleConnsPerHost.
const DefaultMaxIdleConnsPerHost = 2
// ...
type Transport struct {
  // ...
  // MaxIdleConnsPerHost, if non-zero, controls the maximum idle
  // (keep-alive) connections to keep per-host. If zero,
  // DefaultMaxIdleConnsPerHost is used.
  MaxIdleConnsPerHost int
  // ...
}

Based on https://stackoverflow.com/a/31810116 and https://www.reddit.com/r/golang/comments/1730po3/comment/k51x8jn/?utm_source=share&utm_medium=web3x&utm_name=web3xcss&utm_term=1&utm_content=share_button we should see our clients default to creating many more connections per host under higher load.

Copy link
Contributor

@EnriqueL8 EnriqueL8 left a comment

Choose a reason for hiding this comment

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

Looks good - thanks for the clear explanation and links

Copy link
Contributor

@peterbroadhurst peterbroadhurst left a comment

Choose a reason for hiding this comment

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

Understand and agree with this change.

Including the fact it changes default behavior for all consumers of this library.

@peterbroadhurst peterbroadhurst merged commit 0bd135d into hyperledger:main Oct 23, 2024
3 checks passed
@EnriqueL8 EnriqueL8 deleted the ffresty-maxidleconnsperhost branch October 24, 2024 16:28
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.

3 participants