From 2fc3320e59d4015027182577587c5f42002fc43e Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Thu, 10 Dec 2015 22:42:17 -0800 Subject: [PATCH 1/3] rafthttp: kill the receiving body timeout TODO in snapshot sender --- rafthttp/snapshot_sender.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/rafthttp/snapshot_sender.go b/rafthttp/snapshot_sender.go index 8a9a99e93..53e348278 100644 --- a/rafthttp/snapshot_sender.go +++ b/rafthttp/snapshot_sender.go @@ -27,6 +27,11 @@ import ( "github.com/coreos/etcd/snap" ) +var ( + // timeout for reading snapshot response body + snapResponseReadTimeout = 5 * time.Second +) + type snapshotSender struct { from, to types.ID cid types.ID @@ -108,9 +113,6 @@ func (s *snapshotSender) post(req *http.Request) (err error) { result := make(chan responseAndError, 1) go func() { - // TODO: cancel the request if it has waited for a long time(~5s) after - // it has write out the full request body, which helps to avoid receiver - // dies when sender is waiting for response // TODO: the snapshot could be large and eat up all resources when writing // it out. Send it block by block and rest some time between to give the // time for main loop to run. @@ -119,8 +121,12 @@ func (s *snapshotSender) post(req *http.Request) (err error) { result <- responseAndError{resp, nil, err} return } + + // close the response body when timeouts. + // prevents from reading the body forever when the other side dies right after + // successfully receives the request body. + time.AfterFunc(snapResponseReadTimeout, func() { resp.Body.Close() }) body, err := ioutil.ReadAll(resp.Body) - resp.Body.Close() result <- responseAndError{resp, body, err} }() From 95c29838e33fc5f9a2a12f958a79a1b187582b9d Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Thu, 10 Dec 2015 22:45:10 -0800 Subject: [PATCH 2/3] rafthttp: move ReadCloser to ioutil --- pkg/ioutil/readcloser.go | 24 ++++++++++++++++++++++++ rafthttp/snapshot_sender.go | 9 ++------- 2 files changed, 26 insertions(+), 7 deletions(-) create mode 100644 pkg/ioutil/readcloser.go diff --git a/pkg/ioutil/readcloser.go b/pkg/ioutil/readcloser.go new file mode 100644 index 000000000..7c1fda1c2 --- /dev/null +++ b/pkg/ioutil/readcloser.go @@ -0,0 +1,24 @@ +// Copyright 2015 CoreOS, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ioutil + +import "io" + +// ReaderAndCloser implements io.ReadCloser interface by combining +// reader and closer together. +type ReaderAndCloser struct { + io.Reader + io.Closer +} diff --git a/rafthttp/snapshot_sender.go b/rafthttp/snapshot_sender.go index 53e348278..c2b46a9ab 100644 --- a/rafthttp/snapshot_sender.go +++ b/rafthttp/snapshot_sender.go @@ -22,6 +22,7 @@ import ( "time" "github.com/coreos/etcd/pkg/httputil" + pioutil "github.com/coreos/etcd/pkg/ioutil" "github.com/coreos/etcd/pkg/types" "github.com/coreos/etcd/raft" "github.com/coreos/etcd/snap" @@ -142,12 +143,6 @@ func (s *snapshotSender) post(req *http.Request) (err error) { } } -// readCloser implements io.ReadCloser interface. -type readCloser struct { - io.Reader - io.Closer -} - func createSnapBody(merged snap.Message) io.ReadCloser { buf := new(bytes.Buffer) enc := &messageEncoder{w: buf} @@ -156,7 +151,7 @@ func createSnapBody(merged snap.Message) io.ReadCloser { plog.Panicf("encode message error (%v)", err) } - return &readCloser{ + return &pioutil.ReaderAndCloser{ Reader: io.MultiReader(buf, merged.ReadCloser), Closer: merged.ReadCloser, } From 7d78e0c85e5fec072adbf95a3d853d0d45479c42 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Thu, 10 Dec 2015 22:46:23 -0800 Subject: [PATCH 3/3] rafthttp: remove the unncessary TODO The issue is not caused by this code, but by reading snapshot from disk. etcd assumes the snapshot of v3 kv should live in memory. If not, etcd does not work well anyway. --- rafthttp/snapshot_sender.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/rafthttp/snapshot_sender.go b/rafthttp/snapshot_sender.go index c2b46a9ab..9059a4c43 100644 --- a/rafthttp/snapshot_sender.go +++ b/rafthttp/snapshot_sender.go @@ -114,9 +114,6 @@ func (s *snapshotSender) post(req *http.Request) (err error) { result := make(chan responseAndError, 1) go func() { - // TODO: the snapshot could be large and eat up all resources when writing - // it out. Send it block by block and rest some time between to give the - // time for main loop to run. resp, err := s.tr.RoundTrip(req) if err != nil { result <- responseAndError{resp, nil, err}