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

refactor(listener) refactor listener related code #626

Merged
merged 1 commit into from
Mar 14, 2014

Conversation

xiang90
Copy link
Contributor

@xiang90 xiang90 commented Mar 13, 2014

Remove duplicate code around creating http listener.
Start to listen incoming http requests just before serving them.

@xiang90
Copy link
Contributor Author

xiang90 commented Mar 13, 2014

@unihorn @philips

@xiang90
Copy link
Contributor Author

xiang90 commented Mar 13, 2014

This is the first step towards fixing the TLS issue.

@philips
Copy link
Contributor

philips commented Mar 13, 2014

/cc @bcwaldon

@bcwaldon
Copy link
Contributor

lgtm if it works

Remove duplicate code around creating http listener.
Start to listen incoming http requests just before serving them.
@xiang90
Copy link
Contributor Author

xiang90 commented Mar 13, 2014

@unihorn @bcwaldon All the tests passed except the TLS one I am fixing. More prs are needed to fix that one.

@freeformz
Copy link

Inquiring minds want to know which TLS issue?

@xiang90
Copy link
Contributor Author

xiang90 commented Mar 13, 2014

@freeformz We have a TLS functional test that keep on failing https://drone.io/github.com/coreos/etcd/91
The root cause are:

  1. bad timeout assumption for first time TLS handshake
  2. bad order of starting etcd internal service

It is not a huge issue for normal use case, but in the test case we start all the TLS nodes together on the same machine.

xiang90 added a commit that referenced this pull request Mar 14, 2014
refactor(listener) refactor listener related code
@xiang90 xiang90 merged commit 79e4c83 into etcd-io:master Mar 14, 2014
@xiang90 xiang90 deleted the refactor_listener branch March 14, 2014 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants