From 63016204282abdddbd6ea00e86423977c4a5f605 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 11 Jun 2017 10:51:17 +0200 Subject: [PATCH 01/11] s3: Add more debug logs --- src/restic/backend/s3/s3.go | 2 +- src/restic/repository/packer_manager.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/restic/backend/s3/s3.go b/src/restic/backend/s3/s3.go index 8b41bfe6b..9418a2d76 100644 --- a/src/restic/backend/s3/s3.go +++ b/src/restic/backend/s3/s3.go @@ -213,7 +213,7 @@ func (be *Backend) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err return err } - debug.Log("Save %v at %v", h, objName) + debug.Log("Save %v at %v (%d bytes)", h, objName, size) // Check key does not already exist _, err = be.client.StatObject(be.bucketname, objName) diff --git a/src/restic/repository/packer_manager.go b/src/restic/repository/packer_manager.go index 697cbfdcc..dabb9743c 100644 --- a/src/restic/repository/packer_manager.go +++ b/src/restic/repository/packer_manager.go @@ -105,7 +105,7 @@ func (r *packerManager) insertPacker(p *Packer) { // savePacker stores p in the backend. func (r *Repository) savePacker(p *Packer) error { - debug.Log("save packer with %d blobs\n", p.Packer.Count()) + debug.Log("save packer with %d blobs (%d bytes)\n", p.Packer.Count(), p.Packer.Size()) _, err := p.Packer.Finalize() if err != nil { return err From d3c06c39f93d7493d94763ad220000ba230b5950 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Fri, 9 Jun 2017 22:32:42 +0200 Subject: [PATCH 02/11] debug: Fix EOF detection in HTTP transport --- src/restic/debug/round_tripper_debug.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/restic/debug/round_tripper_debug.go b/src/restic/debug/round_tripper_debug.go index 0ff9343ad..523d8438d 100644 --- a/src/restic/debug/round_tripper_debug.go +++ b/src/restic/debug/round_tripper_debug.go @@ -52,7 +52,9 @@ func (rd *eofDetectReader) Close() error { func (tr eofDetectRoundTripper) RoundTrip(req *http.Request) (res *http.Response, err error) { res, err = tr.RoundTripper.RoundTrip(req) - res.Body = &eofDetectReader{rd: res.Body} + if res != nil && res.Body != nil { + res.Body = &eofDetectReader{rd: res.Body} + } return res, err } From 19da56a6eacf91eb6f44c5d9ae2fbf6bc8a58ab2 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Fri, 9 Jun 2017 22:41:08 +0200 Subject: [PATCH 03/11] debug: Add log before panic() --- src/restic/archiver/archiver.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/restic/archiver/archiver.go b/src/restic/archiver/archiver.go index fd471a273..c1d4324a6 100644 --- a/src/restic/archiver/archiver.go +++ b/src/restic/archiver/archiver.go @@ -161,6 +161,7 @@ func (arch *Archiver) saveChunk(ctx context.Context, chunk chunker.Chunk, p *res err := arch.Save(ctx, restic.DataBlob, chunk.Data, id) // TODO handle error if err != nil { + debug.Log("Save(%v) failed: %v", id.Str(), err) panic(err) } From a4e8dc3371433c11619e031d84d2941141c6c3d4 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 11 Jun 2017 11:04:40 +0200 Subject: [PATCH 04/11] s3: Improve error message in debug log --- src/restic/backend/s3/s3.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/restic/backend/s3/s3.go b/src/restic/backend/s3/s3.go index 9418a2d76..beac864b5 100644 --- a/src/restic/backend/s3/s3.go +++ b/src/restic/backend/s3/s3.go @@ -238,7 +238,7 @@ func (be *Backend) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err info, err := coreClient.PutObject(be.bucketname, objName, size, rd, nil, nil, nil) be.sem.ReleaseToken() - debug.Log("%v -> %v bytes, err %#v", objName, info.Size, err) + debug.Log("%v -> %v bytes, err %#v: %v", objName, info.Size, err, err) return errors.Wrap(err, "client.PutObject") } From 08e1d9ffadc04df1fa459420361afd38a0701078 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 11 Jun 2017 11:15:15 +0200 Subject: [PATCH 05/11] s3: Switch back to high-level API, limit connections --- src/restic/backend/s3/config.go | 2 +- src/restic/backend/s3/config_test.go | 28 ++++----- src/restic/backend/s3/s3.go | 71 ++--------------------- src/restic/backend/s3/s3_internal_test.go | 71 ----------------------- 4 files changed, 21 insertions(+), 151 deletions(-) delete mode 100644 src/restic/backend/s3/s3_internal_test.go diff --git a/src/restic/backend/s3/config.go b/src/restic/backend/s3/config.go index 3ae6038ab..6e8e9f2f6 100644 --- a/src/restic/backend/s3/config.go +++ b/src/restic/backend/s3/config.go @@ -25,7 +25,7 @@ type Config struct { // NewConfig returns a new Config with the default values filled in. func NewConfig() Config { return Config{ - Connections: 20, + Connections: 5, } } diff --git a/src/restic/backend/s3/config_test.go b/src/restic/backend/s3/config_test.go index 67611f3cc..8e5f3a420 100644 --- a/src/restic/backend/s3/config_test.go +++ b/src/restic/backend/s3/config_test.go @@ -10,89 +10,89 @@ var configTests = []struct { Endpoint: "eu-central-1", Bucket: "bucketname", Prefix: "restic", - Connections: 20, + Connections: 5, }}, {"s3://eu-central-1/bucketname/", Config{ Endpoint: "eu-central-1", Bucket: "bucketname", Prefix: "restic", - Connections: 20, + Connections: 5, }}, {"s3://eu-central-1/bucketname/prefix/directory", Config{ Endpoint: "eu-central-1", Bucket: "bucketname", Prefix: "prefix/directory", - Connections: 20, + Connections: 5, }}, {"s3://eu-central-1/bucketname/prefix/directory/", Config{ Endpoint: "eu-central-1", Bucket: "bucketname", Prefix: "prefix/directory", - Connections: 20, + Connections: 5, }}, {"s3:eu-central-1/foobar", Config{ Endpoint: "eu-central-1", Bucket: "foobar", Prefix: "restic", - Connections: 20, + Connections: 5, }}, {"s3:eu-central-1/foobar/", Config{ Endpoint: "eu-central-1", Bucket: "foobar", Prefix: "restic", - Connections: 20, + Connections: 5, }}, {"s3:eu-central-1/foobar/prefix/directory", Config{ Endpoint: "eu-central-1", Bucket: "foobar", Prefix: "prefix/directory", - Connections: 20, + Connections: 5, }}, {"s3:eu-central-1/foobar/prefix/directory/", Config{ Endpoint: "eu-central-1", Bucket: "foobar", Prefix: "prefix/directory", - Connections: 20, + Connections: 5, }}, {"s3:https://hostname:9999/foobar", Config{ Endpoint: "hostname:9999", Bucket: "foobar", Prefix: "restic", - Connections: 20, + Connections: 5, }}, {"s3:https://hostname:9999/foobar/", Config{ Endpoint: "hostname:9999", Bucket: "foobar", Prefix: "restic", - Connections: 20, + Connections: 5, }}, {"s3:http://hostname:9999/foobar", Config{ Endpoint: "hostname:9999", Bucket: "foobar", Prefix: "restic", UseHTTP: true, - Connections: 20, + Connections: 5, }}, {"s3:http://hostname:9999/foobar/", Config{ Endpoint: "hostname:9999", Bucket: "foobar", Prefix: "restic", UseHTTP: true, - Connections: 20, + Connections: 5, }}, {"s3:http://hostname:9999/bucket/prefix/directory", Config{ Endpoint: "hostname:9999", Bucket: "bucket", Prefix: "prefix/directory", UseHTTP: true, - Connections: 20, + Connections: 5, }}, {"s3:http://hostname:9999/bucket/prefix/directory/", Config{ Endpoint: "hostname:9999", Bucket: "bucket", Prefix: "prefix/directory", UseHTTP: true, - Connections: 20, + Connections: 5, }}, } diff --git a/src/restic/backend/s3/s3.go b/src/restic/backend/s3/s3.go index beac864b5..6db2e7da9 100644 --- a/src/restic/backend/s3/s3.go +++ b/src/restic/backend/s3/s3.go @@ -157,63 +157,15 @@ func (be *Backend) Path() string { return be.prefix } -// getRemainingSize returns number of bytes remaining. If it is not possible to -// determine the size, panic() is called. -func getRemainingSize(rd io.Reader) (size int64, err error) { - type Sizer interface { - Size() int64 - } - - type Lenner interface { - Len() int - } - - if r, ok := rd.(Lenner); ok { - size = int64(r.Len()) - } else if r, ok := rd.(Sizer); ok { - size = r.Size() - } else if f, ok := rd.(*os.File); ok { - fi, err := f.Stat() - if err != nil { - return 0, err - } - - pos, err := f.Seek(0, io.SeekCurrent) - if err != nil { - return 0, err - } - - size = fi.Size() - pos - } else { - panic(fmt.Sprintf("Save() got passed a reader without a method to determine the data size, type is %T", rd)) - } - return size, nil -} - -// preventCloser wraps an io.Reader to run a function instead of the original Close() function. -type preventCloser struct { - io.Reader - f func() -} - -func (wr preventCloser) Close() error { - wr.f() - return nil -} - // Save stores data in the backend at the handle. func (be *Backend) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err error) { + debug.Log("Save %v", h) + if err := h.Valid(); err != nil { return err } objName := be.Filename(h) - size, err := getRemainingSize(rd) - if err != nil { - return err - } - - debug.Log("Save %v at %v (%d bytes)", h, objName, size) // Check key does not already exist _, err = be.client.StatObject(be.bucketname, objName) @@ -222,23 +174,12 @@ func (be *Backend) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err return errors.New("key already exists") } - be.sem.GetToken() - - // wrap the reader so that net/http client cannot close the reader, return - // the token instead. - rd = preventCloser{ - Reader: rd, - f: func() { - debug.Log("Close()") - }, - } - debug.Log("PutObject(%v, %v)", be.bucketname, objName) - coreClient := minio.Core{Client: be.client} - info, err := coreClient.PutObject(be.bucketname, objName, size, rd, nil, nil, nil) - + be.sem.GetToken() + n, err := be.client.PutObject(be.bucketname, objName, rd, "application/octet-stream") be.sem.ReleaseToken() - debug.Log("%v -> %v bytes, err %#v: %v", objName, info.Size, err, err) + + debug.Log("%v -> %v bytes, err %#v: %v", objName, n, err, err) return errors.Wrap(err, "client.PutObject") } diff --git a/src/restic/backend/s3/s3_internal_test.go b/src/restic/backend/s3/s3_internal_test.go deleted file mode 100644 index ce6c96036..000000000 --- a/src/restic/backend/s3/s3_internal_test.go +++ /dev/null @@ -1,71 +0,0 @@ -package s3 - -import ( - "bytes" - "io" - "io/ioutil" - "os" - "restic/test" - "testing" -) - -func writeFile(t testing.TB, data []byte, offset int64) *os.File { - tempfile, err := ioutil.TempFile("", "restic-test-") - if err != nil { - t.Fatal(err) - } - - if _, err = tempfile.Write(data); err != nil { - t.Fatal(err) - } - - if _, err = tempfile.Seek(offset, io.SeekStart); err != nil { - t.Fatal(err) - } - - return tempfile -} - -func TestGetRemainingSize(t *testing.T) { - length := 18 * 1123 - partialRead := 1005 - - data := test.Random(23, length) - - partReader := bytes.NewReader(data) - buf := make([]byte, partialRead) - _, _ = io.ReadFull(partReader, buf) - - partFileReader := writeFile(t, data, int64(partialRead)) - defer func() { - if err := partFileReader.Close(); err != nil { - t.Fatal(err) - } - - if err := os.Remove(partFileReader.Name()); err != nil { - t.Fatal(err) - } - }() - - var tests = []struct { - io.Reader - size int64 - }{ - {bytes.NewReader([]byte("foobar test")), 11}, - {partReader, int64(length - partialRead)}, - {partFileReader, int64(length - partialRead)}, - } - - for _, test := range tests { - t.Run("", func(t *testing.T) { - size, err := getRemainingSize(test.Reader) - if err != nil { - t.Fatal(err) - } - - if size != test.size { - t.Fatalf("invalid size returned, want %v, got %v", test.size, size) - } - }) - } -} From c4220105977b4236ad345340fa8d3f8678d882a2 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 11 Jun 2017 13:45:06 +0200 Subject: [PATCH 06/11] s3: Fix test --- src/restic/backend/location/location_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/restic/backend/location/location_test.go b/src/restic/backend/location/location_test.go index 09e45cecd..0f07bf412 100644 --- a/src/restic/backend/location/location_test.go +++ b/src/restic/backend/location/location_test.go @@ -123,7 +123,7 @@ var parseTests = []struct { Endpoint: "eu-central-1", Bucket: "bucketname", Prefix: "restic", - Connections: 20, + Connections: 5, }, }, }, @@ -134,7 +134,7 @@ var parseTests = []struct { Endpoint: "hostname.foo", Bucket: "bucketname", Prefix: "restic", - Connections: 20, + Connections: 5, }, }, }, @@ -145,7 +145,7 @@ var parseTests = []struct { Endpoint: "hostname.foo", Bucket: "bucketname", Prefix: "prefix/directory", - Connections: 20, + Connections: 5, }, }, }, @@ -156,7 +156,7 @@ var parseTests = []struct { Endpoint: "eu-central-1", Bucket: "repo", Prefix: "restic", - Connections: 20, + Connections: 5, }, }, }, @@ -167,7 +167,7 @@ var parseTests = []struct { Endpoint: "eu-central-1", Bucket: "repo", Prefix: "prefix/directory", - Connections: 20, + Connections: 5, }, }, }, @@ -178,7 +178,7 @@ var parseTests = []struct { Endpoint: "hostname.foo", Bucket: "repo", Prefix: "restic", - Connections: 20, + Connections: 5, }, }, }, @@ -189,7 +189,7 @@ var parseTests = []struct { Endpoint: "hostname.foo", Bucket: "repo", Prefix: "prefix/directory", - Connections: 20, + Connections: 5, }, }, }, @@ -201,7 +201,7 @@ var parseTests = []struct { Bucket: "repo", Prefix: "restic", UseHTTP: true, - Connections: 20, + Connections: 5, }, }, }, From a89a7a783a1570239dab17e4054063a273b3f3c3 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 11 Jun 2017 13:46:24 +0200 Subject: [PATCH 07/11] s3: Correct comment on the `connections` option --- src/restic/backend/s3/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/restic/backend/s3/config.go b/src/restic/backend/s3/config.go index 6e8e9f2f6..45a040d86 100644 --- a/src/restic/backend/s3/config.go +++ b/src/restic/backend/s3/config.go @@ -19,7 +19,7 @@ type Config struct { Prefix string Layout string `option:"layout" help:"use this backend layout (default: auto-detect)"` - Connections uint `option:"connections" help:"set a limit for the number of concurrent connections (default: 20)"` + Connections uint `option:"connections" help:"set a limit for the number of concurrent connections (default: 5)"` } // NewConfig returns a new Config with the default values filled in. From 58de8bf3927d07649cf04e9ca2381760b907a6a6 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 11 Jun 2017 13:46:54 +0200 Subject: [PATCH 08/11] swift/rest: Reduce number of connections --- src/restic/backend/location/location_test.go | 6 +++--- src/restic/backend/rest/config.go | 4 ++-- src/restic/backend/rest/config_test.go | 2 +- src/restic/backend/swift/config.go | 4 ++-- src/restic/backend/swift/config_test.go | 6 +++--- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/restic/backend/location/location_test.go b/src/restic/backend/location/location_test.go index 0f07bf412..4aaa440cd 100644 --- a/src/restic/backend/location/location_test.go +++ b/src/restic/backend/location/location_test.go @@ -211,7 +211,7 @@ var parseTests = []struct { Config: swift.Config{ Container: "container17", Prefix: "", - Connections: 20, + Connections: 5, }, }, }, @@ -221,7 +221,7 @@ var parseTests = []struct { Config: swift.Config{ Container: "container17", Prefix: "prefix97", - Connections: 20, + Connections: 5, }, }, }, @@ -230,7 +230,7 @@ var parseTests = []struct { Location{Scheme: "rest", Config: rest.Config{ URL: parseURL("http://hostname.foo:1234/"), - Connections: 20, + Connections: 5, }, }, }, diff --git a/src/restic/backend/rest/config.go b/src/restic/backend/rest/config.go index e1ad4e726..f696d17de 100644 --- a/src/restic/backend/rest/config.go +++ b/src/restic/backend/rest/config.go @@ -11,7 +11,7 @@ import ( // Config contains all configuration necessary to connect to a REST server. type Config struct { URL *url.URL - Connections uint `option:"connections" help:"set a limit for the number of concurrent connections (default: 20)"` + Connections uint `option:"connections" help:"set a limit for the number of concurrent connections (default: 5)"` } func init() { @@ -21,7 +21,7 @@ func init() { // NewConfig returns a new Config with the default values filled in. func NewConfig() Config { return Config{ - Connections: 20, + Connections: 5, } } diff --git a/src/restic/backend/rest/config_test.go b/src/restic/backend/rest/config_test.go index 0f27d1c09..ed5413323 100644 --- a/src/restic/backend/rest/config_test.go +++ b/src/restic/backend/rest/config_test.go @@ -21,7 +21,7 @@ var configTests = []struct { }{ {"rest:http://localhost:1234", Config{ URL: parseURL("http://localhost:1234"), - Connections: 20, + Connections: 5, }}, } diff --git a/src/restic/backend/swift/config.go b/src/restic/backend/swift/config.go index 6ef2d4d6f..255c28fd8 100644 --- a/src/restic/backend/swift/config.go +++ b/src/restic/backend/swift/config.go @@ -26,7 +26,7 @@ type Config struct { Prefix string DefaultContainerPolicy string - Connections uint `option:"connections" help:"set a limit for the number of concurrent connections (default: 20)"` + Connections uint `option:"connections" help:"set a limit for the number of concurrent connections (default: 5)"` } func init() { @@ -36,7 +36,7 @@ func init() { // NewConfig returns a new config with the default values filled in. func NewConfig() Config { return Config{ - Connections: 20, + Connections: 5, } } diff --git a/src/restic/backend/swift/config_test.go b/src/restic/backend/swift/config_test.go index 2c1f4c4aa..35f091a9b 100644 --- a/src/restic/backend/swift/config_test.go +++ b/src/restic/backend/swift/config_test.go @@ -11,21 +11,21 @@ var configTests = []struct { Config{ Container: "cnt1", Prefix: "", - Connections: 20, + Connections: 5, }, }, { "swift:cnt2:/prefix", Config{Container: "cnt2", Prefix: "prefix", - Connections: 20, + Connections: 5, }, }, { "swift:cnt3:/prefix/longer", Config{Container: "cnt3", Prefix: "prefix/longer", - Connections: 20, + Connections: 5, }, }, } From 907c201693f661a2e5fa915ab9d68ed54a6d93c5 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 11 Jun 2017 14:17:44 +0200 Subject: [PATCH 09/11] debug: Add version number to debug log --- src/cmds/restic/main.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/cmds/restic/main.go b/src/cmds/restic/main.go index b3877c5b1..f8439e30d 100644 --- a/src/cmds/restic/main.go +++ b/src/cmds/restic/main.go @@ -9,6 +9,7 @@ import ( "restic" "restic/debug" "restic/options" + "runtime" "github.com/spf13/cobra" @@ -57,6 +58,8 @@ func init() { func main() { debug.Log("main %#v", os.Args) + debug.Log("restic %s, compiled with %v on %v/%v", + version, runtime.Version(), runtime.GOOS, runtime.GOARCH) err := cmdRoot.Execute() switch { From 73b296918b97d1f8d94879e85d6cec8a2d478c58 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 11 Jun 2017 14:30:56 +0200 Subject: [PATCH 10/11] s3: Reorder debug messages This way the semaphore token acquisition can be observed in the debug log. --- src/restic/backend/s3/s3.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/restic/backend/s3/s3.go b/src/restic/backend/s3/s3.go index 6db2e7da9..aa557e7e4 100644 --- a/src/restic/backend/s3/s3.go +++ b/src/restic/backend/s3/s3.go @@ -174,8 +174,8 @@ func (be *Backend) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err return errors.New("key already exists") } - debug.Log("PutObject(%v, %v)", be.bucketname, objName) be.sem.GetToken() + debug.Log("PutObject(%v, %v)", be.bucketname, objName) n, err := be.client.PutObject(be.bucketname, objName, rd, "application/octet-stream") be.sem.ReleaseToken() @@ -215,14 +215,14 @@ func (be *Backend) Load(ctx context.Context, h restic.Handle, length int, offset objName := be.Filename(h) - be.sem.GetToken() - byteRange := fmt.Sprintf("bytes=%d-", offset) if length > 0 { byteRange = fmt.Sprintf("bytes=%d-%d", offset, offset+int64(length)-1) } headers := minio.NewGetReqHeaders() headers.Add("Range", byteRange) + + be.sem.GetToken() debug.Log("Load(%v) send range %v", h, byteRange) coreClient := minio.Core{Client: be.client} From 422c0dfb5eb72e4bf03ec46f58b229c2471a58cd Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 11 Jun 2017 14:38:16 +0200 Subject: [PATCH 11/11] s3: Exit test loop for minio server on success --- src/restic/backend/s3/s3_test.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/restic/backend/s3/s3_test.go b/src/restic/backend/s3/s3_test.go index 470a7ab5f..12ca73a10 100644 --- a/src/restic/backend/s3/s3_test.go +++ b/src/restic/backend/s3/s3_test.go @@ -53,13 +53,12 @@ func runMinio(ctx context.Context, t testing.TB, dir, key, secret string) func() time.Sleep(200 * time.Millisecond) c, err := net.Dial("tcp", "localhost:9000") - if err != nil { - continue - } - - success = true - if err := c.Close(); err != nil { - t.Fatal(err) + if err == nil { + success = true + if err := c.Close(); err != nil { + t.Fatal(err) + } + break } }