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

etcdserver,embed: clean up/reorganize client/peer/corrupt handlers #8898

Merged
merged 4 commits into from
Nov 21, 2017

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Nov 20, 2017

Split from #8554.
Preliminary work for #8313.

This doesn't change any server behavior.

Preliminary commit for initial hash checking.
Dial timeout when other nodes have not been booted.

Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
@gyuho gyuho requested a review from xiang90 November 20, 2017 23:07
embed/etcd.go Outdated
@@ -66,13 +66,13 @@ const (
type Etcd struct {
Peers []*peerListener
Clients []net.Listener
clientServers map[string]*serveCtx
Copy link
Contributor

Choose a reason for hiding this comment

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

why this is renamed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wanted to make it clear that it handles client requests.

Copy link
Contributor

Choose a reason for hiding this comment

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

this is a map that contains the context of the server that handles the client requires.

the important part is that it is a map of context. so we cannot really get rid of the ctx part.

i would rather keep the previous name, and add comments to the field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. Just reverted that name change.
Will merge after test passes. Thanks.

@xiang90
Copy link
Contributor

xiang90 commented Nov 20, 2017

lgtm

embed/etcd.go Outdated
@@ -66,13 +66,14 @@ const (
type Etcd struct {
Peers []*peerListener
Clients []net.Listener
sctxs map[string]*serveCtx // client servers
Copy link
Contributor

Choose a reason for hiding this comment

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

// a map of contexts for the servers that serves client requests.

Priliminary commit to start client server later.

Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
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.

None yet

2 participants