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

fix(server/transporter.go) fix the short timeout problem of sending snapshot #616

Merged
merged 1 commit into from
Mar 12, 2014

Conversation

xiang90
Copy link
Contributor

@xiang90 xiang90 commented Mar 8, 2014

Snapshot can be tens of MB. We need to have a minute level timeout for sending snapshot.
We add a separate snapshot http client for snapshot request, which has 120s timeout.
fix #591

There is issue in raft too. I have fixed that in a coming pull request for raft.

…napshot

Snapshot can be tens of MB. We need to have a minute level timeout for sending snapshot.
We add a separate snapshot http client for snapshot request, which has 120s timeout.
@xiang90
Copy link
Contributor Author

xiang90 commented Mar 8, 2014

@philips Shall we also add test in this pull request? But we will be testing the httpclient package from https://github.com/mreiferson/go-httpclient.

@xiang90
Copy link
Contributor Author

xiang90 commented Mar 8, 2014

/cc @unihorn

@@ -58,6 +78,9 @@ func NewTransporter(followersStats *raftFollowersStats, serverStats *raftServerS
func (t *transporter) SetTLSConfig(tlsConf tls.Config) {
t.transport.TLSClientConfig = &tlsConf
t.transport.DisableCompression = true

t.snapshotTransport.TLSClientConfig = &tlsConf
t.snapshotTransport.DisableCompression = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to disable compression on large things like the snapshot?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@philips I just copied the previous setting. I do not think we are using gzip on the server side for now. So disabling or not makes no difference.

Copy link
Contributor

Choose a reason for hiding this comment

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

ack

@philips
Copy link
Contributor

philips commented Mar 10, 2014

lgtm

@philips
Copy link
Contributor

philips commented Mar 10, 2014

@xiangli-cmu Does this fix #591 or do we need to make snapshotting itself more efficient too?

@xiang90
Copy link
Contributor Author

xiang90 commented Mar 10, 2014

@philips This fixes #591.
We definitely want to make snapshot itself more efficient:

  1. JSON is slow
  2. I hope we can do incremental snapshot. So we do not need to encoding the whole structure each time.

@philips
Copy link
Contributor

philips commented Mar 11, 2014

@xiangli-cmu Do you want to merge this one?

xiang90 added a commit that referenced this pull request Mar 12, 2014
fix(server/transporter.go) fix the short timeout problem of sending snapshot
@xiang90 xiang90 merged commit c8e65c1 into etcd-io:master Mar 12, 2014
@xiang90 xiang90 deleted the add_snapshot_post branch March 12, 2014 01:51
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.

Serious error when adding node to a cluster with > 20,000 things in a directory
2 participants