backend: Improve Save()

As mentioned in issue [#1560](https://github.com/restic/restic/pull/1560#issuecomment-364689346)
this changes the signature for `backend.Save()`. It now takes a
parameter of interface type `RewindReader`, so that the backend
implementations or our `RetryBackend` middleware can reset the reader to
the beginning and then retry an upload operation.

The `RewindReader` interface also provides a `Length()` method, which is
used in the backend to get the size of the data to be saved. This
removes several ugly hacks we had to do to pull the size back out of the
`io.Reader` passed to `Save()` before. In the `s3` and `rest` backend
this is actively used.
This commit is contained in:
Alexander Neumann
2018-03-03 14:20:54 +01:00
parent 58306bfabb
commit 99f7fd74e3
29 changed files with 387 additions and 204 deletions

View File

@@ -3,6 +3,7 @@ package azure
import (
"context"
"io"
"io/ioutil"
"net/http"
"os"
"path"
@@ -114,19 +115,8 @@ func (be *Backend) Path() string {
return be.prefix
}
// 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) {
func (be *Backend) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
if err := h.Valid(); err != nil {
return err
}
@@ -137,18 +127,10 @@ func (be *Backend) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err
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("InsertObject(%v, %v)", be.container.Name, objName)
err = be.container.GetBlobReference(objName).CreateBlockBlobFromReader(rd, nil)
// wrap the reader so that net/http client cannot close the reader
err := be.container.GetBlobReference(objName).CreateBlockBlobFromReader(ioutil.NopCloser(rd), nil)
be.sem.ReleaseToken()
debug.Log("%v, err %#v", objName, err)

View File

@@ -185,7 +185,7 @@ func (be *b2Backend) openReader(ctx context.Context, h restic.Handle, length int
}
// Save stores data in the backend at the handle.
func (be *b2Backend) Save(ctx context.Context, h restic.Handle, rd io.Reader) error {
func (be *b2Backend) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
ctx, cancel := context.WithCancel(ctx)
defer cancel()

View File

@@ -45,7 +45,7 @@ func (be *ErrorBackend) fail(p float32) bool {
}
// Save stores the data in the backend under the given handle.
func (be *ErrorBackend) Save(ctx context.Context, h restic.Handle, rd io.Reader) error {
func (be *ErrorBackend) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
if be.fail(be.FailSave) {
return errors.Errorf("Save(%v) random error induced", h)
}

View File

@@ -8,7 +8,6 @@ import (
"github.com/cenkalti/backoff"
"github.com/restic/restic/internal/debug"
"github.com/restic/restic/internal/errors"
"github.com/restic/restic/internal/restic"
)
@@ -47,23 +46,9 @@ func (be *RetryBackend) retry(ctx context.Context, msg string, f func() error) e
}
// Save stores the data in the backend under the given handle.
func (be *RetryBackend) Save(ctx context.Context, h restic.Handle, rd io.Reader) error {
seeker, ok := rd.(io.Seeker)
if !ok {
return errors.Errorf("reader %T is not a seeker", rd)
}
pos, err := seeker.Seek(0, io.SeekCurrent)
if err != nil {
return errors.Wrap(err, "Seek")
}
if pos != 0 {
return errors.Errorf("reader is not at the beginning (pos %v)", pos)
}
func (be *RetryBackend) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
return be.retry(ctx, fmt.Sprintf("Save(%v)", h), func() error {
_, err := seeker.Seek(0, io.SeekStart)
err := rd.Rewind()
if err != nil {
return err
}

View File

@@ -13,48 +13,11 @@ import (
"github.com/restic/restic/internal/test"
)
func TestBackendRetrySeeker(t *testing.T) {
be := &mock.Backend{
SaveFn: func(ctx context.Context, h restic.Handle, rd io.Reader) error {
return nil
},
}
retryBackend := RetryBackend{
Backend: be,
}
data := test.Random(24, 23*14123)
type wrapReader struct {
io.Reader
}
var rd io.Reader
rd = wrapReader{bytes.NewReader(data)}
err := retryBackend.Save(context.TODO(), restic.Handle{}, rd)
if err == nil {
t.Fatal("did not get expected error for retry backend with non-seeker reader")
}
rd = bytes.NewReader(data)
_, err = io.CopyN(ioutil.Discard, rd, 5)
if err != nil {
t.Fatal(err)
}
err = retryBackend.Save(context.TODO(), restic.Handle{}, rd)
if err == nil {
t.Fatal("did not get expected error for partial reader")
}
}
func TestBackendSaveRetry(t *testing.T) {
buf := bytes.NewBuffer(nil)
errcount := 0
be := &mock.Backend{
SaveFn: func(ctx context.Context, h restic.Handle, rd io.Reader) error {
SaveFn: func(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
if errcount == 0 {
errcount++
_, err := io.CopyN(ioutil.Discard, rd, 120)
@@ -75,7 +38,7 @@ func TestBackendSaveRetry(t *testing.T) {
}
data := test.Random(23, 5*1024*1024+11241)
err := retryBackend.Save(context.TODO(), restic.Handle{}, bytes.NewReader(data))
err := retryBackend.Save(context.TODO(), restic.Handle{}, restic.NewByteReader(data))
if err != nil {
t.Fatal(err)
}

View File

@@ -207,7 +207,7 @@ func (be *Backend) Path() string {
}
// Save stores data in the backend at the handle.
func (be *Backend) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err error) {
func (be *Backend) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
if err := h.Valid(); err != nil {
return err
}
@@ -250,6 +250,7 @@ func (be *Backend) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err
info, err := be.service.Objects.Insert(be.bucketName,
&storage.Object{
Name: objName,
Size: uint64(rd.Length()),
}).Media(rd, cs).Do()
be.sem.ReleaseToken()

View File

@@ -98,7 +98,7 @@ func (b *Local) IsNotExist(err error) bool {
}
// Save stores data in the backend at the handle.
func (b *Local) Save(ctx context.Context, h restic.Handle, rd io.Reader) error {
func (b *Local) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
debug.Log("Save %v", h)
if err := h.Valid(); err != nil {
return err

View File

@@ -59,7 +59,7 @@ func (be *MemoryBackend) IsNotExist(err error) bool {
}
// Save adds new Data to the backend.
func (be *MemoryBackend) Save(ctx context.Context, h restic.Handle, rd io.Reader) error {
func (be *MemoryBackend) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
if err := h.Valid(); err != nil {
return err
}

View File

@@ -9,6 +9,7 @@ import (
"net/http"
"net/url"
"path"
"strconv"
"strings"
"golang.org/x/net/context/ctxhttp"
@@ -105,7 +106,7 @@ func (b *restBackend) Location() string {
}
// Save stores data in the backend at the handle.
func (b *restBackend) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err error) {
func (b *restBackend) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
if err := h.Valid(); err != nil {
return err
}
@@ -114,12 +115,11 @@ func (b *restBackend) Save(ctx context.Context, h restic.Handle, rd io.Reader) (
defer cancel()
// make sure that client.Post() cannot close the reader by wrapping it
rd = ioutil.NopCloser(rd)
req, err := http.NewRequest(http.MethodPost, b.Filename(h), rd)
req, err := http.NewRequest(http.MethodPost, b.Filename(h), ioutil.NopCloser(rd))
if err != nil {
return errors.Wrap(err, "NewRequest")
}
req.Header.Set("Content-Length", strconv.Itoa(rd.Length()))
req.Header.Set("Content-Type", "application/octet-stream")
req.Header.Set("Accept", contentTypeV2)

View File

@@ -240,7 +240,7 @@ func lenForFile(f *os.File) (int64, error) {
}
// Save stores data in the backend at the handle.
func (be *Backend) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err error) {
func (be *Backend) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
debug.Log("Save %v", h)
if err := h.Valid(); err != nil {
@@ -252,27 +252,11 @@ func (be *Backend) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err
be.sem.GetToken()
defer be.sem.ReleaseToken()
var size int64 = -1
type lenner interface {
Len() int
}
// find size for reader
if f, ok := rd.(*os.File); ok {
size, err = lenForFile(f)
if err != nil {
return err
}
} else if l, ok := rd.(lenner); ok {
size = int64(l.Len())
}
opts := minio.PutObjectOptions{}
opts.ContentType = "application/octet-stream"
debug.Log("PutObject(%v, %v, %v)", be.cfg.Bucket, objName, size)
n, err := be.client.PutObjectWithContext(ctx, be.cfg.Bucket, objName, ioutil.NopCloser(rd), size, opts)
debug.Log("PutObject(%v, %v, %v)", be.cfg.Bucket, objName, rd.Length())
n, err := be.client.PutObjectWithContext(ctx, be.cfg.Bucket, objName, ioutil.NopCloser(rd), int64(rd.Length()), opts)
debug.Log("%v -> %v bytes, err %#v: %v", objName, n, err, err)

View File

@@ -282,7 +282,7 @@ func Join(parts ...string) string {
}
// Save stores data in the backend at the handle.
func (r *SFTP) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err error) {
func (r *SFTP) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
debug.Log("Save %v", h)
if err := r.clientError(); err != nil {
return err

View File

@@ -156,8 +156,8 @@ func (be *beSwift) openReader(ctx context.Context, h restic.Handle, length int,
}
// Save stores data in the backend at the handle.
func (be *beSwift) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err error) {
if err = h.Valid(); err != nil {
func (be *beSwift) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
if err := h.Valid(); err != nil {
return err
}
@@ -171,7 +171,7 @@ func (be *beSwift) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err
encoding := "binary/octet-stream"
debug.Log("PutObject(%v, %v, %v)", be.container, objName, encoding)
_, err = be.conn.ObjectPut(be.container, objName, rd, true, "", encoding, nil)
_, err := be.conn.ObjectPut(be.container, objName, rd, true, "", encoding, nil)
debug.Log("%v, err %#v", objName, err)
return errors.Wrap(err, "client.PutObject")

View File

@@ -14,7 +14,8 @@ func saveRandomFile(t testing.TB, be restic.Backend, length int) ([]byte, restic
data := test.Random(23, length)
id := restic.Hash(data)
handle := restic.Handle{Type: restic.DataFile, Name: id.String()}
if err := be.Save(context.TODO(), handle, bytes.NewReader(data)); err != nil {
err := be.Save(context.TODO(), handle, restic.NewByteReader(data))
if err != nil {
t.Fatalf("Save() error: %+v", err)
}
return data, handle
@@ -148,16 +149,11 @@ func (s *Suite) BenchmarkSave(t *testing.B) {
id := restic.Hash(data)
handle := restic.Handle{Type: restic.DataFile, Name: id.String()}
rd := bytes.NewReader(data)
rd := restic.NewByteReader(data)
t.SetBytes(int64(length))
t.ResetTimer()
for i := 0; i < t.N; i++ {
if _, err := rd.Seek(0, 0); err != nil {
t.Fatal(err)
}
if err := be.Save(context.TODO(), handle, rd); err != nil {
t.Fatal(err)
}

View File

@@ -10,7 +10,6 @@ import (
"os"
"reflect"
"sort"
"strings"
"testing"
"time"
@@ -85,7 +84,7 @@ func (s *Suite) TestConfig(t *testing.T) {
t.Fatalf("did not get expected error for non-existing config")
}
err = b.Save(context.TODO(), restic.Handle{Type: restic.ConfigFile}, strings.NewReader(testString))
err = b.Save(context.TODO(), restic.Handle{Type: restic.ConfigFile}, restic.NewByteReader([]byte(testString)))
if err != nil {
t.Fatalf("Save() error: %+v", err)
}
@@ -135,7 +134,7 @@ func (s *Suite) TestLoad(t *testing.T) {
id := restic.Hash(data)
handle := restic.Handle{Type: restic.DataFile, Name: id.String()}
err = b.Save(context.TODO(), handle, bytes.NewReader(data))
err = b.Save(context.TODO(), handle, restic.NewByteReader(data))
if err != nil {
t.Fatalf("Save() error: %+v", err)
}
@@ -250,7 +249,7 @@ func (s *Suite) TestList(t *testing.T) {
data := test.Random(rand.Int(), rand.Intn(100)+55)
id := restic.Hash(data)
h := restic.Handle{Type: restic.DataFile, Name: id.String()}
err := b.Save(context.TODO(), h, bytes.NewReader(data))
err := b.Save(context.TODO(), h, restic.NewByteReader(data))
if err != nil {
t.Fatal(err)
}
@@ -340,7 +339,7 @@ func (s *Suite) TestListCancel(t *testing.T) {
data := []byte(fmt.Sprintf("random test blob %v", i))
id := restic.Hash(data)
h := restic.Handle{Type: restic.DataFile, Name: id.String()}
err := b.Save(context.TODO(), h, bytes.NewReader(data))
err := b.Save(context.TODO(), h, restic.NewByteReader(data))
if err != nil {
t.Fatal(err)
}
@@ -443,7 +442,7 @@ func (s *Suite) TestListCancel(t *testing.T) {
}
type errorCloser struct {
io.Reader
io.ReadSeeker
l int
t testing.TB
}
@@ -453,10 +452,15 @@ func (ec errorCloser) Close() error {
return errors.New("forbidden method close was called")
}
func (ec errorCloser) Len() int {
func (ec errorCloser) Length() int {
return ec.l
}
func (ec errorCloser) Rewind() error {
_, err := ec.ReadSeeker.Seek(0, io.SeekStart)
return err
}
// TestSave tests saving data in the backend.
func (s *Suite) TestSave(t *testing.T) {
seedRand(t)
@@ -480,7 +484,7 @@ func (s *Suite) TestSave(t *testing.T) {
Type: restic.DataFile,
Name: fmt.Sprintf("%s-%d", id, i),
}
err := b.Save(context.TODO(), h, bytes.NewReader(data))
err := b.Save(context.TODO(), h, restic.NewByteReader(data))
test.OK(t, err)
buf, err := backend.LoadAll(context.TODO(), b, h)
@@ -532,7 +536,7 @@ func (s *Suite) TestSave(t *testing.T) {
// wrap the tempfile in an errorCloser, so we can detect if the backend
// closes the reader
err = b.Save(context.TODO(), h, errorCloser{t: t, l: length, Reader: tmpfile})
err = b.Save(context.TODO(), h, errorCloser{t: t, l: length, ReadSeeker: tmpfile})
if err != nil {
t.Fatal(err)
}
@@ -542,25 +546,10 @@ func (s *Suite) TestSave(t *testing.T) {
t.Fatalf("error removing item: %+v", err)
}
// try again directly with the temp file
if _, err = tmpfile.Seek(588, io.SeekStart); err != nil {
t.Fatal(err)
}
err = b.Save(context.TODO(), h, tmpfile)
if err != nil {
t.Fatal(err)
}
if err = tmpfile.Close(); err != nil {
t.Fatal(err)
}
err = b.Remove(context.TODO(), h)
if err != nil {
t.Fatalf("error removing item: %+v", err)
}
if err = os.Remove(tmpfile.Name()); err != nil {
t.Fatal(err)
}
@@ -585,7 +574,7 @@ func (s *Suite) TestSaveFilenames(t *testing.T) {
for i, test := range filenameTests {
h := restic.Handle{Name: test.name, Type: restic.DataFile}
err := b.Save(context.TODO(), h, strings.NewReader(test.data))
err := b.Save(context.TODO(), h, restic.NewByteReader([]byte(test.data)))
if err != nil {
t.Errorf("test %d failed: Save() returned %+v", i, err)
continue
@@ -622,7 +611,7 @@ var testStrings = []struct {
func store(t testing.TB, b restic.Backend, tpe restic.FileType, data []byte) restic.Handle {
id := restic.Hash(data)
h := restic.Handle{Name: id.String(), Type: tpe}
err := b.Save(context.TODO(), h, bytes.NewReader(data))
err := b.Save(context.TODO(), h, restic.NewByteReader([]byte(data)))
test.OK(t, err)
return h
}
@@ -776,7 +765,7 @@ func (s *Suite) TestBackend(t *testing.T) {
test.Assert(t, !ok, "removed blob still present")
// create blob
err = b.Save(context.TODO(), h, strings.NewReader(ts.data))
err = b.Save(context.TODO(), h, restic.NewByteReader([]byte(ts.data)))
test.OK(t, err)
// list items

View File

@@ -24,7 +24,8 @@ func TestLoadAll(t *testing.T) {
data := rtest.Random(23+i, rand.Intn(MiB)+500*KiB)
id := restic.Hash(data)
err := b.Save(context.TODO(), restic.Handle{Name: id.String(), Type: restic.DataFile}, bytes.NewReader(data))
h := restic.Handle{Name: id.String(), Type: restic.DataFile}
err := b.Save(context.TODO(), h, restic.NewByteReader(data))
rtest.OK(t, err)
buf, err := backend.LoadAll(context.TODO(), b, restic.Handle{Type: restic.DataFile, Name: id.String()})
@@ -49,7 +50,8 @@ func TestLoadSmallBuffer(t *testing.T) {
data := rtest.Random(23+i, rand.Intn(MiB)+500*KiB)
id := restic.Hash(data)
err := b.Save(context.TODO(), restic.Handle{Name: id.String(), Type: restic.DataFile}, bytes.NewReader(data))
h := restic.Handle{Name: id.String(), Type: restic.DataFile}
err := b.Save(context.TODO(), h, restic.NewByteReader(data))
rtest.OK(t, err)
buf, err := backend.LoadAll(context.TODO(), b, restic.Handle{Type: restic.DataFile, Name: id.String()})
@@ -74,7 +76,8 @@ func TestLoadLargeBuffer(t *testing.T) {
data := rtest.Random(23+i, rand.Intn(MiB)+500*KiB)
id := restic.Hash(data)
err := b.Save(context.TODO(), restic.Handle{Name: id.String(), Type: restic.DataFile}, bytes.NewReader(data))
h := restic.Handle{Name: id.String(), Type: restic.DataFile}
err := b.Save(context.TODO(), h, restic.NewByteReader(data))
rtest.OK(t, err)
buf, err := backend.LoadAll(context.TODO(), b, restic.Handle{Type: restic.DataFile, Name: id.String()})