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

Increase maxIdleConnection limit when creating etcd client in apiserver. #7353

Merged
merged 1 commit into from
Apr 28, 2015

Conversation

wojtek-t
Copy link
Member

@fgrzadkowski fgrzadkowski self-assigned this Apr 27, 2015
@fgrzadkowski
Copy link
Contributor

lgtm.

@fgrzadkowski fgrzadkowski added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 27, 2015
@fgrzadkowski
Copy link
Contributor

Will merge on green

@xiang90
Copy link
Contributor

xiang90 commented Apr 27, 2015

lgtm

// dial attempts to open a TCP connection to the provided address, explicitly
// enabling keep-alives with a one-second interval.
//
// TODO: This is copy-pasted from etcd.Client.dial(), since that method isn't public.
Copy link
Member

Choose a reason for hiding this comment

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

:( Unfortunately, that means it needs to be located in a file in third_party

Copy link
Member Author

Choose a reason for hiding this comment

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

Where exactly I should move it?

Copy link
Member

Choose a reason for hiding this comment

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

make a new directory/file in ``third_party/forked//.go` and make it a public function there. Also include the third party LICENSE and any header at the top of the file.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry for my ignorance here - but what should LICENCE file contain?
Any of files under Godeps/_workspace/src/github.com/coreos/go-etcd/etcd doesn't contain any licence-related header.

Copy link
Member

Choose a reason for hiding this comment

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

In that case, don't put the license or the header in. :)

Copy link
Member

Choose a reason for hiding this comment

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

@sillsm to verify that I'm recalling the instructions correctly...

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@lavalamp lavalamp removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 27, 2015
@lavalamp lavalamp assigned lavalamp and unassigned fgrzadkowski Apr 27, 2015
@lavalamp
Copy link
Member

I really wanted to merge this but we should probably keep the lawyercats happy.

@wojtek-t
Copy link
Member Author

PTAL

@wojtek-t
Copy link
Member Author

@lavalamp I hope that the current directory structure is fine - if not please let me know.

@lavalamp
Copy link
Member

LGTM

lavalamp added a commit that referenced this pull request Apr 28, 2015
Increase maxIdleConnection limit when creating etcd client in apiserver.
@lavalamp lavalamp merged commit 19ae113 into kubernetes:master Apr 28, 2015
@wojtek-t wojtek-t deleted the too_many_dials branch April 30, 2015 12:14
@smarterclayton
Copy link
Contributor

Rather than forking this, it might be easier to reuse our existing TLS transport options:

    transport, err := client.TransportFor(&client.Config{
        TLSClientConfig: client.TLSClientConfig{
            CertFile: etcdClientInfo.ClientCert.CertFile,
            KeyFile:  etcdClientInfo.ClientCert.KeyFile,
            CAFile:   etcdClientInfo.CA,
        },
        WrapTransport: func(rt http.RoundTripper) http.RoundTripper {
            transport := rt.(*http.Transport)
            dialer := &net.Dialer{
                Timeout: 30 * time.Second,
                // Lower the keep alive for connections.
                KeepAlive: 1 * time.Second,
            }
            // Because watches are very bursty, defends against long delays
            // in watch reconnections.
            transport.MaxIdleConnsPerHost = 500
            transport.Dial = dialer.Dial
            return transport
        },
    })
    etcdClient := etcdclient.NewClient(etcdClientInfo.URLs)
    etcdClient.SetTransport(transport.(*http.Transport))

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.

Too much time spent in net.(*netFD).dial
6 participants