From f893ec57cb858dd2730ec5dbd09cf3951d986d84 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sat, 5 Mar 2016 15:58:39 +0100 Subject: [PATCH 1/8] Add test and benchmark for PackerManager --- src/restic/repository/packer_manager.go | 7 +- src/restic/repository/packer_manager_test.go | 122 +++++++++++++++++++ 2 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 src/restic/repository/packer_manager_test.go diff --git a/src/restic/repository/packer_manager.go b/src/restic/repository/packer_manager.go index a7716418e..2791af51c 100644 --- a/src/restic/repository/packer_manager.go +++ b/src/restic/repository/packer_manager.go @@ -9,9 +9,14 @@ import ( "restic/pack" ) +// Saver implements saving data in a backend. +type Saver interface { + Save(h backend.Handle, p []byte) error +} + // packerManager keeps a list of open packs and creates new on demand. type packerManager struct { - be backend.Backend + be Saver key *crypto.Key pm sync.Mutex packs []*pack.Packer diff --git a/src/restic/repository/packer_manager_test.go b/src/restic/repository/packer_manager_test.go new file mode 100644 index 000000000..4af073e4d --- /dev/null +++ b/src/restic/repository/packer_manager_test.go @@ -0,0 +1,122 @@ +package repository + +import ( + "io" + "math/rand" + "restic/backend" + "restic/backend/mem" + "restic/crypto" + "restic/pack" + "testing" +) + +func randomID(rd io.Reader) backend.ID { + id := backend.ID{} + _, err := io.ReadFull(rd, id[:]) + if err != nil { + panic(err) + } + return id +} + +func fillPacks(t testing.TB, rnd *rand.Rand, be Saver, pm *packerManager) (bytes int) { + for i := 0; i < 100; i++ { + l := rnd.Intn(1 << 20) + seed := rnd.Int63() + + packer, err := pm.findPacker(uint(l)) + if err != nil { + t.Fatal(err) + } + + rd := rand.New(rand.NewSource(seed)) + id := randomID(rd) + n, err := packer.Add(pack.Data, id, io.LimitReader(rd, int64(l))) + + if n != int64(l) { + t.Errorf("Add() returned invalid number of bytes: want %v, got %v", n, l) + } + bytes += l + + if packer.Size() < minPackSize && pm.countPacker() < maxPackers { + pm.insertPacker(packer) + continue + } + + data, err := packer.Finalize() + if err != nil { + t.Fatal(err) + } + + h := backend.Handle{Type: backend.Data, Name: randomID(rd).String()} + + err = be.Save(h, data) + if err != nil { + t.Fatal(err) + } + } + + return bytes +} + +func flushRemainingPacks(t testing.TB, rnd *rand.Rand, be Saver, pm *packerManager) (bytes int) { + if pm.countPacker() > 0 { + for _, packer := range pm.packs { + data, err := packer.Finalize() + if err != nil { + t.Fatal(err) + } + bytes += len(data) + + h := backend.Handle{Type: backend.Data, Name: randomID(rnd).String()} + + err = be.Save(h, data) + if err != nil { + t.Fatal(err) + } + } + } + + return bytes +} + +type fakeBackend struct{} + +func (f *fakeBackend) Save(h backend.Handle, data []byte) error { + return nil +} + +func TestPackerManager(t *testing.T) { + rnd := rand.New(rand.NewSource(23)) + + be := mem.New() + pm := &packerManager{ + be: be, + key: crypto.NewRandomKey(), + } + + bytes := fillPacks(t, rnd, be, pm) + bytes += flushRemainingPacks(t, rnd, be, pm) + + t.Logf("saved %d bytes", bytes) +} + +func BenchmarkPackerManager(t *testing.B) { + rnd := rand.New(rand.NewSource(23)) + + be := &fakeBackend{} + pm := &packerManager{ + be: be, + key: crypto.NewRandomKey(), + } + + t.ResetTimer() + + bytes := 0 + for i := 0; i < t.N; i++ { + bytes += fillPacks(t, rnd, be, pm) + } + + bytes += flushRemainingPacks(t, rnd, be, pm) + t.Logf("saved %d bytes", bytes) +} From f956f60f9f337485be247206f9699bda98c50f7f Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 6 Mar 2016 12:26:25 +0100 Subject: [PATCH 2/8] PackerManager: use tempfiles instead of memory buffers --- src/restic/pack/pack.go | 42 +++++---- src/restic/repository/packer_manager.go | 58 +++++++++++- src/restic/repository/packer_manager_test.go | 94 ++++++++++++++------ src/restic/repository/repository.go | 21 +++-- src/restic/test/backend.go | 5 +- src/restic/test/helpers.go | 5 +- 6 files changed, 170 insertions(+), 55 deletions(-) diff --git a/src/restic/pack/pack.go b/src/restic/pack/pack.go index 7aa8360be..e983eae3f 100644 --- a/src/restic/pack/pack.go +++ b/src/restic/pack/pack.go @@ -83,26 +83,26 @@ type Packer struct { bytes uint k *crypto.Key - buf *bytes.Buffer + wr io.Writer m sync.Mutex } // NewPacker returns a new Packer that can be used to pack blobs -// together. -func NewPacker(k *crypto.Key, buf []byte) *Packer { - return &Packer{k: k, buf: bytes.NewBuffer(buf)} +// together. If wr is nil, a bytes.Buffer is used. +func NewPacker(k *crypto.Key, wr io.Writer) *Packer { + return &Packer{k: k, wr: wr} } // Add saves the data read from rd as a new blob to the packer. Returned is the // number of bytes written to the pack. -func (p *Packer) Add(t BlobType, id backend.ID, rd io.Reader) (int64, error) { +func (p *Packer) Add(t BlobType, id backend.ID, data []byte) (int, error) { p.m.Lock() defer p.m.Unlock() c := Blob{Type: t, ID: id} - n, err := io.Copy(p.buf, rd) + n, err := p.wr.Write(data) c.Length = uint(n) c.Offset = p.bytes p.bytes += uint(n) @@ -121,8 +121,9 @@ type headerEntry struct { } // Finalize writes the header for all added blobs and finalizes the pack. -// Returned are all bytes written, including the header. -func (p *Packer) Finalize() ([]byte, error) { +// Returned are the number of bytes written, including the header. If the +// underlying writer implements io.Closer, it is closed. +func (p *Packer) Finalize() (uint, error) { p.m.Lock() defer p.m.Unlock() @@ -131,37 +132,41 @@ func (p *Packer) Finalize() ([]byte, error) { hdrBuf := bytes.NewBuffer(nil) bytesHeader, err := p.writeHeader(hdrBuf) if err != nil { - return nil, err + return 0, err } encryptedHeader, err := crypto.Encrypt(p.k, nil, hdrBuf.Bytes()) if err != nil { - return nil, err + return 0, err } // append the header - n, err := p.buf.Write(encryptedHeader) + n, err := p.wr.Write(encryptedHeader) if err != nil { - return nil, err + return 0, err } hdrBytes := bytesHeader + crypto.Extension if uint(n) != hdrBytes { - return nil, errors.New("wrong number of bytes written") + return 0, errors.New("wrong number of bytes written") } bytesWritten += hdrBytes // write length - err = binary.Write(p.buf, binary.LittleEndian, uint32(uint(len(p.blobs))*entrySize+crypto.Extension)) + err = binary.Write(p.wr, binary.LittleEndian, uint32(uint(len(p.blobs))*entrySize+crypto.Extension)) if err != nil { - return nil, err + return 0, err } bytesWritten += uint(binary.Size(uint32(0))) p.bytes = uint(bytesWritten) - return p.buf.Bytes(), nil + if w, ok := p.wr.(io.Closer); ok { + return bytesWritten, w.Close() + } + + return bytesWritten, nil } // writeHeader constructs and writes the header to wr. @@ -208,6 +213,11 @@ func (p *Packer) Blobs() []Blob { return p.blobs } +// Writer return the underlying writer. +func (p *Packer) Writer() io.Writer { + return p.wr +} + func (p *Packer) String() string { return fmt.Sprintf("", len(p.blobs), p.bytes) } diff --git a/src/restic/repository/packer_manager.go b/src/restic/repository/packer_manager.go index 2791af51c..88a3e1ac7 100644 --- a/src/restic/repository/packer_manager.go +++ b/src/restic/repository/packer_manager.go @@ -1,6 +1,10 @@ package repository import ( + "fmt" + "io" + "io/ioutil" + "os" "sync" "restic/backend" @@ -11,7 +15,7 @@ import ( // Saver implements saving data in a backend. type Saver interface { - Save(h backend.Handle, p []byte) error + Save(h backend.Handle, jp []byte) error } // packerManager keeps a list of open packs and creates new on demand. @@ -20,12 +24,30 @@ type packerManager struct { key *crypto.Key pm sync.Mutex packs []*pack.Packer + + tempdir string } const minPackSize = 4 * 1024 * 1024 const maxPackSize = 16 * 1024 * 1024 const maxPackers = 200 +// NewPackerManager returns an new packer manager which writes temporary files +// to a temporary directory +func NewPackerManager(be Saver, key *crypto.Key) (pm *packerManager, err error) { + pm = &packerManager{ + be: be, + key: key, + } + + pm.tempdir, err = ioutil.TempDir("", fmt.Sprintf("restic-packs-%d-", os.Getpid())) + if err != nil { + return nil, err + } + + return pm, nil +} + // findPacker returns a packer for a new blob of size bytes. Either a new one is // created or one is returned that already has some blobs. func (r *packerManager) findPacker(size uint) (*pack.Packer, error) { @@ -47,7 +69,13 @@ func (r *packerManager) findPacker(size uint) (*pack.Packer, error) { // no suitable packer found, return new debug.Log("Repo.findPacker", "create new pack for %d bytes", size) - return pack.NewPacker(r.key, nil), nil + tmpfile, err := ioutil.TempFile(r.tempdir, "restic-pack-") + if err != nil { + return nil, err + } + + fmt.Printf("tmpfile: %v, tempdir %v\n", tmpfile.Name(), r.tempdir) + return pack.NewPacker(r.key, tmpfile), nil } // insertPacker appends p to s.packs. @@ -62,11 +90,28 @@ func (r *packerManager) insertPacker(p *pack.Packer) { // savePacker stores p in the backend. func (r *Repository) savePacker(p *pack.Packer) error { debug.Log("Repo.savePacker", "save packer with %d blobs\n", p.Count()) - data, err := p.Finalize() + n, err := p.Finalize() if err != nil { return err } + tmpfile := p.Writer().(*os.File) + f, err := os.Open(tmpfile.Name()) + if err != nil { + return err + } + + data := make([]byte, n) + m, err := io.ReadFull(f, data) + + if uint(m) != n { + return fmt.Errorf("read wrong number of bytes from %v: want %v, got %v", tmpfile.Name(), n, m) + } + + if err = f.Close(); err != nil { + return err + } + id := backend.Hash(data) h := backend.Handle{Type: backend.Data, Name: id.String()} @@ -100,3 +145,10 @@ func (r *packerManager) countPacker() int { return len(r.packs) } + +// removeTempdir deletes the temporary directory. +func (r *packerManager) removeTempdir() error { + err := os.RemoveAll(r.tempdir) + r.tempdir = "" + return err +} diff --git a/src/restic/repository/packer_manager_test.go b/src/restic/repository/packer_manager_test.go index 4af073e4d..79a7d9904 100644 --- a/src/restic/repository/packer_manager_test.go +++ b/src/restic/repository/packer_manager_test.go @@ -3,6 +3,7 @@ package repository import ( "io" "math/rand" + "os" "restic/backend" "restic/backend/mem" "restic/crypto" @@ -19,7 +20,39 @@ func randomID(rd io.Reader) backend.ID { return id } -func fillPacks(t testing.TB, rnd *rand.Rand, be Saver, pm *packerManager) (bytes int) { +const maxBlobSize = 1 << 20 + +func saveFile(t testing.TB, be Saver, filename string, n int) { + f, err := os.Open(filename) + if err != nil { + t.Fatal(err) + } + + data := make([]byte, n) + m, err := io.ReadFull(f, data) + + if m != n { + t.Fatalf("read wrong number of bytes from %v: want %v, got %v", filename, m, n) + } + + if err = f.Close(); err != nil { + t.Fatal(err) + } + + h := backend.Handle{Type: backend.Data, Name: backend.Hash(data).String()} + + err = be.Save(h, data) + if err != nil { + t.Fatal(err) + } + + err = os.Remove(filename) + if err != nil { + t.Fatal(err) + } +} + +func fillPacks(t testing.TB, rnd *rand.Rand, be Saver, pm *packerManager, buf []byte) (bytes int) { for i := 0; i < 100; i++ { l := rnd.Intn(1 << 20) seed := rnd.Int63() @@ -31,9 +64,14 @@ func fillPacks(t testing.TB, rnd *rand.Rand, be Saver, pm *packerManager) (bytes rd := rand.New(rand.NewSource(seed)) id := randomID(rd) - n, err := packer.Add(pack.Data, id, io.LimitReader(rd, int64(l))) + buf = buf[:l] + _, err = io.ReadFull(rd, buf) + if err != nil { + t.Fatal(err) + } - if n != int64(l) { + n, err := packer.Add(pack.Data, id, buf) + if n != l { t.Errorf("Add() returned invalid number of bytes: want %v, got %v", n, l) } bytes += l @@ -43,17 +81,13 @@ func fillPacks(t testing.TB, rnd *rand.Rand, be Saver, pm *packerManager) (bytes continue } - data, err := packer.Finalize() + bytesWritten, err := packer.Finalize() if err != nil { t.Fatal(err) } - h := backend.Handle{Type: backend.Data, Name: randomID(rd).String()} - - err = be.Save(h, data) - if err != nil { - t.Fatal(err) - } + tmpfile := packer.Writer().(*os.File) + saveFile(t, be, tmpfile.Name(), int(bytesWritten)) } return bytes @@ -62,18 +96,14 @@ func fillPacks(t testing.TB, rnd *rand.Rand, be Saver, pm *packerManager) (bytes func flushRemainingPacks(t testing.TB, rnd *rand.Rand, be Saver, pm *packerManager) (bytes int) { if pm.countPacker() > 0 { for _, packer := range pm.packs { - data, err := packer.Finalize() + n, err := packer.Finalize() if err != nil { t.Fatal(err) } - bytes += len(data) + bytes += int(n) - h := backend.Handle{Type: backend.Data, Name: randomID(rnd).String()} - - err = be.Save(h, data) - if err != nil { - t.Fatal(err) - } + tmpfile := packer.Writer().(*os.File) + saveFile(t, be, tmpfile.Name(), bytes) } } @@ -90,33 +120,45 @@ func TestPackerManager(t *testing.T) { rnd := rand.New(rand.NewSource(23)) be := mem.New() - pm := &packerManager{ - be: be, - key: crypto.NewRandomKey(), + pm, err := NewPackerManager(be, crypto.NewRandomKey()) + if err != nil { + t.Fatal(err) } - bytes := fillPacks(t, rnd, be, pm) + blobBuf := make([]byte, maxBlobSize) + + bytes := fillPacks(t, rnd, be, pm, blobBuf) bytes += flushRemainingPacks(t, rnd, be, pm) t.Logf("saved %d bytes", bytes) + err = pm.removeTempdir() + if err != nil { + t.Fatal(err) + } } func BenchmarkPackerManager(t *testing.B) { rnd := rand.New(rand.NewSource(23)) be := &fakeBackend{} - pm := &packerManager{ - be: be, - key: crypto.NewRandomKey(), + pm, err := NewPackerManager(be, crypto.NewRandomKey()) + if err != nil { + t.Fatal(err) } + blobBuf := make([]byte, maxBlobSize) t.ResetTimer() bytes := 0 for i := 0; i < t.N; i++ { - bytes += fillPacks(t, rnd, be, pm) + bytes += fillPacks(t, rnd, be, pm, blobBuf) } bytes += flushRemainingPacks(t, rnd, be, pm) t.Logf("saved %d bytes", bytes) + + err = pm.removeTempdir() + if err != nil { + t.Fatal(err) + } } diff --git a/src/restic/repository/repository.go b/src/restic/repository/repository.go index f981f5b76..eb86902b6 100644 --- a/src/restic/repository/repository.go +++ b/src/restic/repository/repository.go @@ -27,14 +27,19 @@ type Repository struct { } // New returns a new repository with backend be. -func New(be backend.Backend) *Repository { - return &Repository{ - be: be, - idx: NewMasterIndex(), - packerManager: &packerManager{ - be: be, - }, +func New(be backend.Backend) (*Repository, error) { + pm, err := NewPackerManager(be, nil) + if err != nil { + return nil, err } + + repo := &Repository{ + be: be, + idx: NewMasterIndex(), + packerManager: pm, + } + + return repo, nil } // Find loads the list of all blobs of type t and searches for names which start @@ -195,7 +200,7 @@ func (r *Repository) SaveAndEncrypt(t pack.BlobType, data []byte, id *backend.ID } // save ciphertext - _, err = packer.Add(t, *id, bytes.NewReader(ciphertext)) + _, err = packer.Add(t, *id, ciphertext) if err != nil { return backend.ID{}, err } diff --git a/src/restic/test/backend.go b/src/restic/test/backend.go index 5516cecdf..4d9ca6a44 100644 --- a/src/restic/test/backend.go +++ b/src/restic/test/backend.go @@ -61,7 +61,10 @@ func SetupRepo() *repository.Repository { panic(err) } - repo := repository.New(b) + repo, err := repository.New(b) + if err != nil { + panic(err) + } err = repo.Init(TestPassword) if err != nil { panic(err) diff --git a/src/restic/test/helpers.go b/src/restic/test/helpers.go index 4c3280fba..c43e15b70 100644 --- a/src/restic/test/helpers.go +++ b/src/restic/test/helpers.go @@ -213,7 +213,10 @@ func OpenLocalRepo(t testing.TB, dir string) *repository.Repository { be, err := local.Open(dir) OK(t, err) - repo := repository.New(be) + repo, err := repository.New(be) + if err != nil { + t.Fatal(err) + } err = repo.SearchKey(TestPassword) OK(t, err) From c0b5f5a8af1a53da8d082462a743821b4cc45c9c Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 6 Mar 2016 12:34:23 +0100 Subject: [PATCH 3/8] Fix all code which uses repository.New() --- src/cmds/restic/cmd_init.go | 6 +++++- src/cmds/restic/global.go | 5 ++++- src/restic/archiver_duplication_test.go | 6 +++++- src/restic/checker/checker_test.go | 6 ++++-- src/restic/pack/pack.go | 3 +++ src/restic/pack/pack_test.go | 5 +++-- 6 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/cmds/restic/cmd_init.go b/src/cmds/restic/cmd_init.go index ec790fe90..069977cb6 100644 --- a/src/cmds/restic/cmd_init.go +++ b/src/cmds/restic/cmd_init.go @@ -26,7 +26,11 @@ func (cmd CmdInit) Execute(args []string) error { "enter password again: ") } - s := repository.New(be) + s, err := repository.New(be) + if err != nil { + return err + } + err = s.Init(cmd.global.password) if err != nil { cmd.global.Exitf(1, "creating key in backend at %s failed: %v\n", cmd.global.Repo, err) diff --git a/src/cmds/restic/global.go b/src/cmds/restic/global.go index 49a2ace30..1de0bfe76 100644 --- a/src/cmds/restic/global.go +++ b/src/cmds/restic/global.go @@ -208,7 +208,10 @@ func (o GlobalOptions) OpenRepository() (*repository.Repository, error) { return nil, err } - s := repository.New(be) + s, err := repository.New(be) + if err != nil { + return nil, err + } if o.password == "" { o.password = o.ReadPassword("enter password for repository: ") diff --git a/src/restic/archiver_duplication_test.go b/src/restic/archiver_duplication_test.go index 3edc70cb3..564cdca00 100644 --- a/src/restic/archiver_duplication_test.go +++ b/src/restic/archiver_duplication_test.go @@ -78,7 +78,11 @@ func testArchiverDuplication(t *testing.T) { t.Fatal(err) } - repo := repository.New(forgetfulBackend()) + repo, err := repository.New(forgetfulBackend()) + if err != nil { + t.Fatal(err) + } + err = repo.Init("foo") if err != nil { t.Fatal(err) diff --git a/src/restic/checker/checker_test.go b/src/restic/checker/checker_test.go index 85135a98c..5db7d945e 100644 --- a/src/restic/checker/checker_test.go +++ b/src/restic/checker/checker_test.go @@ -239,7 +239,8 @@ func induceError(data []byte) { func TestCheckerModifiedData(t *testing.T) { be := mem.New() - repo := repository.New(be) + repo, err := repository.New(be) + OK(t, err) OK(t, repo.Init(TestPassword)) arch := restic.NewArchiver(repo) @@ -248,7 +249,8 @@ func TestCheckerModifiedData(t *testing.T) { t.Logf("archived as %v", id.Str()) beError := &errorBackend{Backend: be} - checkRepo := repository.New(beError) + checkRepo, err := repository.New(beError) + OK(t, err) OK(t, checkRepo.SearchKey(TestPassword)) chkr := checker.New(checkRepo) diff --git a/src/restic/pack/pack.go b/src/restic/pack/pack.go index e983eae3f..931bda869 100644 --- a/src/restic/pack/pack.go +++ b/src/restic/pack/pack.go @@ -91,6 +91,9 @@ type Packer struct { // NewPacker returns a new Packer that can be used to pack blobs // together. If wr is nil, a bytes.Buffer is used. func NewPacker(k *crypto.Key, wr io.Writer) *Packer { + if wr == nil { + wr = bytes.NewBuffer(nil) + } return &Packer{k: k, wr: wr} } diff --git a/src/restic/pack/pack_test.go b/src/restic/pack/pack_test.go index 18a7a86f4..e987ced7c 100644 --- a/src/restic/pack/pack_test.go +++ b/src/restic/pack/pack_test.go @@ -38,12 +38,13 @@ func newPack(t testing.TB, k *crypto.Key) ([]Buf, []byte, uint) { // pack blobs p := pack.NewPacker(k, nil) for _, b := range bufs { - p.Add(pack.Tree, b.id, bytes.NewReader(b.data)) + p.Add(pack.Tree, b.id, b.data) } - packData, err := p.Finalize() + _, err := p.Finalize() OK(t, err) + packData := p.Writer().(*bytes.Buffer).Bytes() return bufs, packData, p.Size() } From 015cea0c50f53fba256d64c05f4671e3d1be1ede Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 6 Mar 2016 12:35:21 +0100 Subject: [PATCH 4/8] PackerManager: Remove debug comment --- src/restic/repository/packer_manager.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/restic/repository/packer_manager.go b/src/restic/repository/packer_manager.go index 88a3e1ac7..bb1c62d93 100644 --- a/src/restic/repository/packer_manager.go +++ b/src/restic/repository/packer_manager.go @@ -74,7 +74,6 @@ func (r *packerManager) findPacker(size uint) (*pack.Packer, error) { return nil, err } - fmt.Printf("tmpfile: %v, tempdir %v\n", tmpfile.Name(), r.tempdir) return pack.NewPacker(r.key, tmpfile), nil } From cda7616c826ce8607a530384fe70e95832e4be0d Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 6 Mar 2016 13:14:06 +0100 Subject: [PATCH 5/8] Remove tempdir for packerManager --- src/cmds/restic/cmd_init.go | 5 +-- src/cmds/restic/global.go | 5 +-- src/restic/archiver_duplication_test.go | 5 +-- src/restic/checker/checker_test.go | 6 ++-- src/restic/repository/packer_manager.go | 34 +++++++++----------- src/restic/repository/packer_manager_test.go | 19 ++--------- src/restic/repository/repository.go | 12 ++----- src/restic/test/backend.go | 5 +-- src/restic/test/helpers.go | 5 +-- 9 files changed, 27 insertions(+), 69 deletions(-) diff --git a/src/cmds/restic/cmd_init.go b/src/cmds/restic/cmd_init.go index 069977cb6..887d34166 100644 --- a/src/cmds/restic/cmd_init.go +++ b/src/cmds/restic/cmd_init.go @@ -26,10 +26,7 @@ func (cmd CmdInit) Execute(args []string) error { "enter password again: ") } - s, err := repository.New(be) - if err != nil { - return err - } + s := repository.New(be) err = s.Init(cmd.global.password) if err != nil { diff --git a/src/cmds/restic/global.go b/src/cmds/restic/global.go index 1de0bfe76..49a2ace30 100644 --- a/src/cmds/restic/global.go +++ b/src/cmds/restic/global.go @@ -208,10 +208,7 @@ func (o GlobalOptions) OpenRepository() (*repository.Repository, error) { return nil, err } - s, err := repository.New(be) - if err != nil { - return nil, err - } + s := repository.New(be) if o.password == "" { o.password = o.ReadPassword("enter password for repository: ") diff --git a/src/restic/archiver_duplication_test.go b/src/restic/archiver_duplication_test.go index 564cdca00..ffcbacee4 100644 --- a/src/restic/archiver_duplication_test.go +++ b/src/restic/archiver_duplication_test.go @@ -78,10 +78,7 @@ func testArchiverDuplication(t *testing.T) { t.Fatal(err) } - repo, err := repository.New(forgetfulBackend()) - if err != nil { - t.Fatal(err) - } + repo := repository.New(forgetfulBackend()) err = repo.Init("foo") if err != nil { diff --git a/src/restic/checker/checker_test.go b/src/restic/checker/checker_test.go index 5db7d945e..85135a98c 100644 --- a/src/restic/checker/checker_test.go +++ b/src/restic/checker/checker_test.go @@ -239,8 +239,7 @@ func induceError(data []byte) { func TestCheckerModifiedData(t *testing.T) { be := mem.New() - repo, err := repository.New(be) - OK(t, err) + repo := repository.New(be) OK(t, repo.Init(TestPassword)) arch := restic.NewArchiver(repo) @@ -249,8 +248,7 @@ func TestCheckerModifiedData(t *testing.T) { t.Logf("archived as %v", id.Str()) beError := &errorBackend{Backend: be} - checkRepo, err := repository.New(beError) - OK(t, err) + checkRepo := repository.New(beError) OK(t, checkRepo.SearchKey(TestPassword)) chkr := checker.New(checkRepo) diff --git a/src/restic/repository/packer_manager.go b/src/restic/repository/packer_manager.go index bb1c62d93..9622ebb9a 100644 --- a/src/restic/repository/packer_manager.go +++ b/src/restic/repository/packer_manager.go @@ -25,7 +25,7 @@ type packerManager struct { pm sync.Mutex packs []*pack.Packer - tempdir string + pool sync.Pool } const minPackSize = 4 * 1024 * 1024 @@ -34,23 +34,21 @@ const maxPackers = 200 // NewPackerManager returns an new packer manager which writes temporary files // to a temporary directory -func NewPackerManager(be Saver, key *crypto.Key) (pm *packerManager, err error) { - pm = &packerManager{ +func NewPackerManager(be Saver, key *crypto.Key) *packerManager { + return &packerManager{ be: be, key: key, + pool: sync.Pool{ + New: func() interface{} { + return make([]byte, (minPackSize+maxPackSize)/2) + }, + }, } - - pm.tempdir, err = ioutil.TempDir("", fmt.Sprintf("restic-packs-%d-", os.Getpid())) - if err != nil { - return nil, err - } - - return pm, nil } // findPacker returns a packer for a new blob of size bytes. Either a new one is // created or one is returned that already has some blobs. -func (r *packerManager) findPacker(size uint) (*pack.Packer, error) { +func (r *packerManager) findPacker(size uint) (packer *pack.Packer, err error) { r.pm.Lock() defer r.pm.Unlock() @@ -69,7 +67,7 @@ func (r *packerManager) findPacker(size uint) (*pack.Packer, error) { // no suitable packer found, return new debug.Log("Repo.findPacker", "create new pack for %d bytes", size) - tmpfile, err := ioutil.TempFile(r.tempdir, "restic-pack-") + tmpfile, err := ioutil.TempFile("", "restic-temp-pack-") if err != nil { return nil, err } @@ -122,6 +120,11 @@ func (r *Repository) savePacker(p *pack.Packer) error { debug.Log("Repo.savePacker", "saved as %v", h) + err = os.Remove(tmpfile.Name()) + if err != nil { + return err + } + // update blobs in the index for _, b := range p.Blobs() { debug.Log("Repo.savePacker", " updating blob %v to pack %v", b.ID.Str(), id.Str()) @@ -144,10 +147,3 @@ func (r *packerManager) countPacker() int { return len(r.packs) } - -// removeTempdir deletes the temporary directory. -func (r *packerManager) removeTempdir() error { - err := os.RemoveAll(r.tempdir) - r.tempdir = "" - return err -} diff --git a/src/restic/repository/packer_manager_test.go b/src/restic/repository/packer_manager_test.go index 79a7d9904..53129a7fe 100644 --- a/src/restic/repository/packer_manager_test.go +++ b/src/restic/repository/packer_manager_test.go @@ -120,10 +120,7 @@ func TestPackerManager(t *testing.T) { rnd := rand.New(rand.NewSource(23)) be := mem.New() - pm, err := NewPackerManager(be, crypto.NewRandomKey()) - if err != nil { - t.Fatal(err) - } + pm := NewPackerManager(be, crypto.NewRandomKey()) blobBuf := make([]byte, maxBlobSize) @@ -131,20 +128,13 @@ func TestPackerManager(t *testing.T) { bytes += flushRemainingPacks(t, rnd, be, pm) t.Logf("saved %d bytes", bytes) - err = pm.removeTempdir() - if err != nil { - t.Fatal(err) - } } func BenchmarkPackerManager(t *testing.B) { rnd := rand.New(rand.NewSource(23)) be := &fakeBackend{} - pm, err := NewPackerManager(be, crypto.NewRandomKey()) - if err != nil { - t.Fatal(err) - } + pm := NewPackerManager(be, crypto.NewRandomKey()) blobBuf := make([]byte, maxBlobSize) t.ResetTimer() @@ -156,9 +146,4 @@ func BenchmarkPackerManager(t *testing.B) { bytes += flushRemainingPacks(t, rnd, be, pm) t.Logf("saved %d bytes", bytes) - - err = pm.removeTempdir() - if err != nil { - t.Fatal(err) - } } diff --git a/src/restic/repository/repository.go b/src/restic/repository/repository.go index eb86902b6..5d28ee4a7 100644 --- a/src/restic/repository/repository.go +++ b/src/restic/repository/repository.go @@ -27,19 +27,14 @@ type Repository struct { } // New returns a new repository with backend be. -func New(be backend.Backend) (*Repository, error) { - pm, err := NewPackerManager(be, nil) - if err != nil { - return nil, err - } - +func New(be backend.Backend) *Repository { repo := &Repository{ be: be, idx: NewMasterIndex(), - packerManager: pm, + packerManager: NewPackerManager(be, nil), } - return repo, nil + return repo } // Find loads the list of all blobs of type t and searches for names which start @@ -304,7 +299,6 @@ func (r *Repository) Flush() error { } } r.packs = r.packs[:0] - return nil } diff --git a/src/restic/test/backend.go b/src/restic/test/backend.go index 4d9ca6a44..5516cecdf 100644 --- a/src/restic/test/backend.go +++ b/src/restic/test/backend.go @@ -61,10 +61,7 @@ func SetupRepo() *repository.Repository { panic(err) } - repo, err := repository.New(b) - if err != nil { - panic(err) - } + repo := repository.New(b) err = repo.Init(TestPassword) if err != nil { panic(err) diff --git a/src/restic/test/helpers.go b/src/restic/test/helpers.go index c43e15b70..4c3280fba 100644 --- a/src/restic/test/helpers.go +++ b/src/restic/test/helpers.go @@ -213,10 +213,7 @@ func OpenLocalRepo(t testing.TB, dir string) *repository.Repository { be, err := local.Open(dir) OK(t, err) - repo, err := repository.New(be) - if err != nil { - t.Fatal(err) - } + repo := repository.New(be) err = repo.SearchKey(TestPassword) OK(t, err) From 1e1368eea3ef919fb243c520b7fe2536262c3eea Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 6 Mar 2016 13:59:06 +0100 Subject: [PATCH 6/8] Add randReader for tests This can be removed once we require Go 1.6. --- src/restic/repository/packer_manager_test.go | 37 ++++++++++++++++---- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/src/restic/repository/packer_manager_test.go b/src/restic/repository/packer_manager_test.go index 53129a7fe..dedca4c60 100644 --- a/src/restic/repository/packer_manager_test.go +++ b/src/restic/repository/packer_manager_test.go @@ -11,6 +11,31 @@ import ( "testing" ) +type randReader struct { + src rand.Source + rand *rand.Rand +} + +func newRandReader(src rand.Source) *randReader { + return &randReader{ + src: src, + rand: rand.New(src), + } +} + +// Read generates len(p) random bytes and writes them into p. It +// always returns len(p) and a nil error. +func (r *randReader) Read(p []byte) (n int, err error) { + for i := 0; i < len(p); i += 7 { + val := r.src.Int63() + for j := 0; i+j < len(p) && j < 7; j++ { + p[i+j] = byte(val) + val >>= 8 + } + } + return len(p), nil +} + func randomID(rd io.Reader) backend.ID { id := backend.ID{} _, err := io.ReadFull(rd, id[:]) @@ -52,10 +77,10 @@ func saveFile(t testing.TB, be Saver, filename string, n int) { } } -func fillPacks(t testing.TB, rnd *rand.Rand, be Saver, pm *packerManager, buf []byte) (bytes int) { +func fillPacks(t testing.TB, rnd *randReader, be Saver, pm *packerManager, buf []byte) (bytes int) { for i := 0; i < 100; i++ { - l := rnd.Intn(1 << 20) - seed := rnd.Int63() + l := rnd.rand.Intn(1 << 20) + seed := rnd.rand.Int63() packer, err := pm.findPacker(uint(l)) if err != nil { @@ -93,7 +118,7 @@ func fillPacks(t testing.TB, rnd *rand.Rand, be Saver, pm *packerManager, buf [] return bytes } -func flushRemainingPacks(t testing.TB, rnd *rand.Rand, be Saver, pm *packerManager) (bytes int) { +func flushRemainingPacks(t testing.TB, rnd *randReader, be Saver, pm *packerManager) (bytes int) { if pm.countPacker() > 0 { for _, packer := range pm.packs { n, err := packer.Finalize() @@ -117,7 +142,7 @@ func (f *fakeBackend) Save(h backend.Handle, data []byte) error { } func TestPackerManager(t *testing.T) { - rnd := rand.New(rand.NewSource(23)) + rnd := newRandReader(rand.NewSource(23)) be := mem.New() pm := NewPackerManager(be, crypto.NewRandomKey()) @@ -131,7 +156,7 @@ func TestPackerManager(t *testing.T) { } func BenchmarkPackerManager(t *testing.B) { - rnd := rand.New(rand.NewSource(23)) + rnd := newRandReader(rand.NewSource(23)) be := &fakeBackend{} pm := NewPackerManager(be, crypto.NewRandomKey()) From 18c302417100aa26a9adafaa7c390dd75d54274a Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 6 Mar 2016 14:20:48 +0100 Subject: [PATCH 7/8] Unexport NewPackerManager --- src/restic/repository/packer_manager.go | 4 ++-- src/restic/repository/packer_manager_test.go | 4 ++-- src/restic/repository/repository.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/restic/repository/packer_manager.go b/src/restic/repository/packer_manager.go index 9622ebb9a..de1f97c74 100644 --- a/src/restic/repository/packer_manager.go +++ b/src/restic/repository/packer_manager.go @@ -32,9 +32,9 @@ const minPackSize = 4 * 1024 * 1024 const maxPackSize = 16 * 1024 * 1024 const maxPackers = 200 -// NewPackerManager returns an new packer manager which writes temporary files +// newPackerManager returns an new packer manager which writes temporary files // to a temporary directory -func NewPackerManager(be Saver, key *crypto.Key) *packerManager { +func newPackerManager(be Saver, key *crypto.Key) *packerManager { return &packerManager{ be: be, key: key, diff --git a/src/restic/repository/packer_manager_test.go b/src/restic/repository/packer_manager_test.go index dedca4c60..70a3c816c 100644 --- a/src/restic/repository/packer_manager_test.go +++ b/src/restic/repository/packer_manager_test.go @@ -145,7 +145,7 @@ func TestPackerManager(t *testing.T) { rnd := newRandReader(rand.NewSource(23)) be := mem.New() - pm := NewPackerManager(be, crypto.NewRandomKey()) + pm := newPackerManager(be, crypto.NewRandomKey()) blobBuf := make([]byte, maxBlobSize) @@ -159,7 +159,7 @@ func BenchmarkPackerManager(t *testing.B) { rnd := newRandReader(rand.NewSource(23)) be := &fakeBackend{} - pm := NewPackerManager(be, crypto.NewRandomKey()) + pm := newPackerManager(be, crypto.NewRandomKey()) blobBuf := make([]byte, maxBlobSize) t.ResetTimer() diff --git a/src/restic/repository/repository.go b/src/restic/repository/repository.go index 5d28ee4a7..e7e2e5ada 100644 --- a/src/restic/repository/repository.go +++ b/src/restic/repository/repository.go @@ -31,7 +31,7 @@ func New(be backend.Backend) *Repository { repo := &Repository{ be: be, idx: NewMasterIndex(), - packerManager: NewPackerManager(be, nil), + packerManager: newPackerManager(be, nil), } return repo From e4a6dd8c8c87f0e0b294d87def680a00610d3d08 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 6 Mar 2016 14:21:02 +0100 Subject: [PATCH 8/8] Use newRandReader instead of rand.New() This needs to be done since for Go < 1.6 rand.Rand does not implement io.Reader. --- src/restic/repository/packer_manager_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/restic/repository/packer_manager_test.go b/src/restic/repository/packer_manager_test.go index 70a3c816c..78d91bc37 100644 --- a/src/restic/repository/packer_manager_test.go +++ b/src/restic/repository/packer_manager_test.go @@ -87,7 +87,7 @@ func fillPacks(t testing.TB, rnd *randReader, be Saver, pm *packerManager, buf [ t.Fatal(err) } - rd := rand.New(rand.NewSource(seed)) + rd := newRandReader(rand.NewSource(seed)) id := randomID(rd) buf = buf[:l] _, err = io.ReadFull(rd, buf)