diff --git a/Makefile b/Makefile index 3bdee605b..ab8baf2d4 100644 --- a/Makefile +++ b/Makefile @@ -47,10 +47,6 @@ endif install: html-ui go install $(KOPIA_BUILD_FLAGS) -tags $(KOPIA_BUILD_TAGS) -install-profiling: KOPIA_BUILD_TAGS=embedhtml,profiling -install-profiling: html-ui - go install $(KOPIA_BUILD_FLAGS) -tags $(KOPIA_BUILD_TAGS) - install-noui: go install $(KOPIA_BUILD_FLAGS) diff --git a/cli/app.go b/cli/app.go index 8efcff458..07d231bf6 100644 --- a/cli/app.go +++ b/cli/app.go @@ -6,6 +6,7 @@ "fmt" "io" "net/http" + "net/http/pprof" "os" "time" @@ -14,6 +15,8 @@ "github.com/pkg/errors" "github.com/kopia/kopia/internal/apiclient" + "github.com/kopia/kopia/internal/gather" + "github.com/kopia/kopia/internal/memtrack" "github.com/kopia/kopia/internal/passwordpersist" "github.com/kopia/kopia/repo" "github.com/kopia/kopia/repo/blob" @@ -112,6 +115,7 @@ type App struct { configPath string traceStorage bool metricsListenAddr string + enablePProf bool keyRingEnabled bool persistCredentials bool disableInternalLog bool @@ -199,6 +203,7 @@ func (c *App) setup(app *kingpin.Application) { app.Flag("config-file", "Specify the config file to use.").Default(defaultConfigFileName()).Envar("KOPIA_CONFIG_PATH").StringVar(&c.configPath) app.Flag("trace-storage", "Enables tracing of storage operations.").Default("true").Hidden().BoolVar(&c.traceStorage) app.Flag("metrics-listen-addr", "Expose Prometheus metrics on a given host:port").Hidden().StringVar(&c.metricsListenAddr) + app.Flag("enable-pprof", "Expose pprof handlers").Hidden().BoolVar(&c.enablePProf) app.Flag("timezone", "Format time according to specified time zone (local, utc, original or time zone name)").Hidden().StringVar(&timeZone) app.Flag("password", "Repository password.").Envar("KOPIA_PASSWORD").Short('p').StringVar(&c.password) app.Flag("persist-credentials", "Persist credentials").Default("true").Envar("KOPIA_PERSIST_CREDENTIALS_ON_CONNECT").BoolVar(&c.persistCredentials) @@ -384,11 +389,13 @@ type repositoryAccessMode struct { func (c *App) maybeRepositoryAction(act func(ctx context.Context, rep repo.Repository) error, mode repositoryAccessMode) func(ctx *kingpin.ParseContext) error { return func(kpc *kingpin.ParseContext) error { - ctx := c.rootContext() + ctx0 := c.rootContext() if err := c.pf.withProfiling(func() error { - c.mt.startMemoryTracking(ctx) - defer c.mt.finishMemoryTracking(ctx) + ctx, finishMemoryTracking := c.mt.startMemoryTracking(ctx0) + defer finishMemoryTracking() + + defer gather.DumpStats(ctx) if c.metricsListenAddr != "" { mux := http.NewServeMux() @@ -396,11 +403,23 @@ func (c *App) maybeRepositoryAction(act func(ctx context.Context, rep repo.Repos return errors.Wrap(err, "unable to initialize prometheus.") } + if c.enablePProf { + mux.HandleFunc("/debug/pprof/", pprof.Index) + mux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline) + mux.HandleFunc("/debug/pprof/profile", pprof.Profile) + mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol) + mux.HandleFunc("/debug/pprof/trace", pprof.Trace) + } + log(ctx).Infof("starting prometheus metrics on %v", c.metricsListenAddr) go http.ListenAndServe(c.metricsListenAddr, mux) // nolint:errcheck } + memtrack.Dump(ctx, "before openRepository") + rep, err := c.openRepository(ctx, mode.mustBeConnected) + + memtrack.Dump(ctx, "after openRepository") if err != nil && mode.mustBeConnected { return errors.Wrap(err, "open repository") } @@ -408,21 +427,29 @@ func (c *App) maybeRepositoryAction(act func(ctx context.Context, rep repo.Repos err = act(ctx, rep) if rep != nil && !mode.disableMaintenance { + memtrack.Dump(ctx, "before auto maintenance") + if merr := c.maybeRunMaintenance(ctx, rep); merr != nil { log(ctx).Errorf("error running maintenance: %v", merr) } + + memtrack.Dump(ctx, "after auto maintenance") } if rep != nil && mode.mustBeConnected { + memtrack.Dump(ctx, "before close repository") + if cerr := rep.Close(ctx); cerr != nil { return errors.Wrap(cerr, "unable to close repository") } + + memtrack.Dump(ctx, "after close repository") } return err }); err != nil { // print error in red - log(ctx).Errorf("ERROR: %v", err.Error()) + log(ctx0).Errorf("ERROR: %v", err.Error()) c.osExit(1) } diff --git a/cli/command_benchmark_compression.go b/cli/command_benchmark_compression.go index 17f3b5b41..161d777c3 100644 --- a/cli/command_benchmark_compression.go +++ b/cli/command_benchmark_compression.go @@ -81,10 +81,13 @@ type benchResult struct { compressed bytes.Buffer ) + input := bytes.NewReader(nil) + for i := 0; i < cnt; i++ { compressed.Reset() + input.Reset(data) - if err := comp.Compress(&compressed, data); err != nil { + if err := comp.Compress(&compressed, input); err != nil { log(ctx).Errorf("compression %q failed: %v", name, err) continue } diff --git a/cli/command_benchmark_crypto.go b/cli/command_benchmark_crypto.go index d26e5ac30..c21363a04 100644 --- a/cli/command_benchmark_crypto.go +++ b/cli/command_benchmark_crypto.go @@ -6,6 +6,7 @@ atunits "github.com/alecthomas/units" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/timetrack" "github.com/kopia/kopia/internal/units" "github.com/kopia/kopia/repo/content" @@ -43,13 +44,10 @@ type benchResult struct { data := make([]byte, c.blockSize) - const ( - maxEncryptionOverhead = 1024 - ) - var hashOutput [hashing.MaxHashSize]byte - encryptOutput := make([]byte, len(data)+maxEncryptionOverhead) + var encryptOutput gather.WriteBuffer + defer encryptOutput.Close() for _, ha := range hashing.SupportedAlgorithms() { for _, ea := range encryption.SupportedAlgorithms(c.deprecatedAlgorithms) { @@ -65,13 +63,14 @@ type benchResult struct { log(ctx).Infof("Benchmarking hash '%v' and encryption '%v'... (%v x %v bytes)", ha, ea, c.repeat, len(data)) + input := gather.FromSlice(data) tt := timetrack.Start() hashCount := c.repeat for i := 0; i < hashCount; i++ { - contentID := cr.HashFunction(hashOutput[:0], data) - if _, encerr := cr.Encryptor.Encrypt(encryptOutput[:0], data, contentID); encerr != nil { + contentID := cr.HashFunction(hashOutput[:0], input) + if encerr := cr.Encryptor.Encrypt(input, contentID, &encryptOutput); encerr != nil { log(ctx).Errorf("encryption failed: %v", encerr) break } diff --git a/cli/command_blob_show.go b/cli/command_blob_show.go index 607f4e23c..199cd4b69 100644 --- a/cli/command_blob_show.go +++ b/cli/command_blob_show.go @@ -8,6 +8,7 @@ "github.com/pkg/errors" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/iocopy" "github.com/kopia/kopia/repo" "github.com/kopia/kopia/repo/blob" @@ -41,31 +42,38 @@ func (c *commandBlobShow) run(ctx context.Context, rep repo.DirectRepository) er func (c *commandBlobShow) maybeDecryptBlob(ctx context.Context, w io.Writer, rep repo.DirectRepository, blobID blob.ID) error { var ( - d []byte - err error + d gather.WriteBuffer + b gather.Bytes ) - d, err = rep.BlobReader().GetBlob(ctx, blobID, 0, -1) + if err := rep.BlobReader().GetBlob(ctx, blobID, 0, -1, &d); err != nil { + return errors.Wrap(err, "error reading blob") + } + + b = d.Bytes() if c.blobShowDecrypt && canDecryptBlob(blobID) { - d, err = rep.Crypter().DecryptBLOB(d, blobID) + var tmp gather.WriteBuffer + defer tmp.Close() - if isJSONBlob(blobID) && err == nil { - var b bytes.Buffer - - if err = json.Indent(&b, d, "", " "); err != nil { - return errors.Wrap(err, "invalid JSON") - } - - d = b.Bytes() + if err := rep.Crypter().DecryptBLOB(b, blobID, &tmp); err != nil { + return errors.Wrap(err, "error decrypting blob") } + + b = tmp.Bytes() } - if err != nil { - return errors.Wrapf(err, "error getting %v", blobID) + if isJSONBlob(blobID) { + var buf bytes.Buffer + + if err := json.Indent(&buf, b.ToByteSlice(), "", " "); err != nil { + return errors.Wrap(err, "invalid JSON") + } + + b = gather.FromSlice(buf.Bytes()) } - if _, err := iocopy.Copy(w, bytes.NewReader(d)); err != nil { + if err := iocopy.JustCopy(w, b.Reader()); err != nil { return errors.Wrap(err, "error copying data") } diff --git a/cli/command_index_inspect.go b/cli/command_index_inspect.go index dc1b91eca..7248d0818 100644 --- a/cli/command_index_inspect.go +++ b/cli/command_index_inspect.go @@ -5,6 +5,7 @@ "github.com/pkg/errors" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo" "github.com/kopia/kopia/repo/blob" "github.com/kopia/kopia/repo/content" @@ -106,12 +107,14 @@ func (c *commandIndexInspect) inspectSingleIndexBlob(ctx context.Context, rep re return errors.Wrapf(err, "unable to get metadata for %v", blobID) } - data, err := rep.BlobReader().GetBlob(ctx, blobID, 0, -1) - if err != nil { + var data gather.WriteBuffer + defer data.Close() + + if err = rep.BlobReader().GetBlob(ctx, blobID, 0, -1, &data); err != nil { return errors.Wrapf(err, "unable to get data for %v", blobID) } - entries, err := content.ParseIndexBlob(ctx, blobID, data, rep.Crypter()) + entries, err := content.ParseIndexBlob(ctx, blobID, data.Bytes(), rep.Crypter()) if err != nil { return errors.Wrapf(err, "unable to recover index from %v", blobID) } diff --git a/cli/command_logs_show.go b/cli/command_logs_show.go index c6b3abcef..c890868a2 100644 --- a/cli/command_logs_show.go +++ b/cli/command_logs_show.go @@ -1,11 +1,11 @@ package cli import ( - "bytes" "context" "github.com/pkg/errors" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo" ) @@ -57,19 +57,23 @@ func (c *commandLogsShow) run(ctx context.Context, rep repo.DirectRepository) er log(ctx).Infof("Showing latest log (%v)", formatTimestamp(sessions[0].startTime)) } + var data gather.WriteBuffer + defer data.Close() + + var decrypted gather.WriteBuffer + defer decrypted.Close() + for _, s := range sessions { for _, bm := range s.segments { - data, err := rep.BlobReader().GetBlob(ctx, bm.BlobID, 0, -1) - if err != nil { + if err := rep.BlobReader().GetBlob(ctx, bm.BlobID, 0, -1, &data); err != nil { return errors.Wrap(err, "error getting log") } - data, err = rep.Crypter().DecryptBLOB(data, bm.BlobID) - if err != nil { + if err := rep.Crypter().DecryptBLOB(data.Bytes(), bm.BlobID, &decrypted); err != nil { return errors.Wrap(err, "error decrypting log") } - if err := showContentWithFlags(c.out.stdout(), bytes.NewReader(data), true, false); err != nil { + if err := showContentWithFlags(c.out.stdout(), decrypted.Bytes().Reader(), true, false); err != nil { log(ctx).Errorf("error showing log: %v", err) } } diff --git a/cli/command_repository_repair.go b/cli/command_repository_repair.go index de572192a..665aaba7e 100644 --- a/cli/command_repository_repair.go +++ b/cli/command_repository_repair.go @@ -56,7 +56,10 @@ func (c *commandRepositoryRepair) runRepairCommandWithStorage(ctx context.Contex case "auto": log(ctx).Infof("looking for format blob...") - if _, err := st.GetBlob(ctx, repo.FormatBlobID, 0, -1); err == nil { + var tmp gather.WriteBuffer + defer tmp.Close() + + if err := st.GetBlob(ctx, repo.FormatBlobID, 0, -1, &tmp); err == nil { log(ctx).Infof("format blob already exists, not recovering, pass --recover-format=yes") return nil } diff --git a/cli/command_repository_sync.go b/cli/command_repository_sync.go index 353b0b40e..58569d6f2 100644 --- a/cli/command_repository_sync.go +++ b/cli/command_repository_sync.go @@ -291,8 +291,10 @@ func sliceToChannel(ctx context.Context, md []blob.Metadata) chan blob.Metadata } func (c *commandRepositorySyncTo) syncCopyBlob(ctx context.Context, m blob.Metadata, src blob.Reader, dst blob.Storage) error { - data, err := src.GetBlob(ctx, m.BlobID, 0, -1) - if err != nil { + var data gather.WriteBuffer + defer data.Close() + + if err := src.GetBlob(ctx, m.BlobID, 0, -1, &data); err != nil { if errors.Is(err, blob.ErrBlobNotFound) { log(ctx).Infof("ignoring BLOB not found: %v", m.BlobID) return nil @@ -301,7 +303,7 @@ func (c *commandRepositorySyncTo) syncCopyBlob(ctx context.Context, m blob.Metad return errors.Wrapf(err, "error reading blob '%v' from source", m.BlobID) } - if err := dst.PutBlob(ctx, m.BlobID, gather.FromSlice(data)); err != nil { + if err := dst.PutBlob(ctx, m.BlobID, data.Bytes()); err != nil { return errors.Wrapf(err, "error writing blob '%v' to destination", m.BlobID) } @@ -331,26 +333,30 @@ func syncDeleteBlob(ctx context.Context, m blob.Metadata, dst blob.Storage) erro } func (c *commandRepositorySyncTo) ensureRepositoriesHaveSameFormatBlob(ctx context.Context, src blob.Reader, dst blob.Storage) error { - srcData, err := src.GetBlob(ctx, repo.FormatBlobID, 0, -1) - if err != nil { + var srcData gather.WriteBuffer + defer srcData.Close() + + if err := src.GetBlob(ctx, repo.FormatBlobID, 0, -1, &srcData); err != nil { return errors.Wrap(err, "error reading format blob") } - dstData, err := dst.GetBlob(ctx, repo.FormatBlobID, 0, -1) - if err != nil { + var dstData gather.WriteBuffer + defer dstData.Close() + + if err := dst.GetBlob(ctx, repo.FormatBlobID, 0, -1, &dstData); err != nil { // target does not have format blob, save it there first. if errors.Is(err, blob.ErrBlobNotFound) { if c.repositorySyncDestinationMustExist { return errors.Errorf("destination repository does not have a format blob") } - return errors.Wrap(dst.PutBlob(ctx, repo.FormatBlobID, gather.FromSlice(srcData)), "error saving format blob") + return errors.Wrap(dst.PutBlob(ctx, repo.FormatBlobID, srcData.Bytes()), "error saving format blob") } return errors.Wrap(err, "error reading destination repository format blob") } - if bytes.Equal(srcData, dstData) { + if bytes.Equal(srcData.ToByteSlice(), dstData.ToByteSlice()) { return nil } diff --git a/cli/command_show.go b/cli/command_show.go index 5e4e62c9b..e68b49f88 100644 --- a/cli/command_show.go +++ b/cli/command_show.go @@ -37,7 +37,5 @@ func (c *commandShow) run(ctx context.Context, rep repo.Repository) error { defer r.Close() //nolint:errcheck - _, err = iocopy.Copy(c.out.stdout(), r) - - return errors.Wrap(err, "unable to copy data") + return errors.Wrap(iocopy.JustCopy(c.out.stdout(), r), "unable to copy data") } diff --git a/cli/command_snapshot_verify.go b/cli/command_snapshot_verify.go index 00ce23247..a6f4fc8c0 100644 --- a/cli/command_snapshot_verify.go +++ b/cli/command_snapshot_verify.go @@ -194,9 +194,7 @@ func (v *verifier) readEntireObject(ctx context.Context, oid object.ID, path str } defer r.Close() //nolint:errcheck - _, err = iocopy.Copy(ioutil.Discard, r) - - return errors.Wrap(err, "unable to read data") + return errors.Wrap(iocopy.JustCopy(ioutil.Discard, r), "unable to read data") } func (c *commandSnapshotVerify) run(ctx context.Context, rep repo.Repository) error { diff --git a/cli/memory_tracking.go b/cli/memory_tracking.go index 187ee30fe..665c3716b 100644 --- a/cli/memory_tracking.go +++ b/cli/memory_tracking.go @@ -2,64 +2,54 @@ import ( "context" - "runtime" "sync" "time" "github.com/alecthomas/kingpin" - "github.com/kopia/kopia/repo/logging" + "github.com/kopia/kopia/internal/memtrack" ) type memoryTracker struct { trackMemoryUsage time.Duration - - memoryTrackerMutex sync.Mutex - lastHeapUsage, lastStackInUse uint64 - maxHeapUsage, maxStackInUse uint64 } func (c *memoryTracker) setup(app *kingpin.Application) { app.Flag("track-memory-usage", "Periodically force GC and log current memory usage").Hidden().DurationVar(&c.trackMemoryUsage) } -var memlog = logging.GetContextLoggerFunc("kopia/memory") +func (c *memoryTracker) startMemoryTracking(ctx context.Context) (context.Context, context.CancelFunc) { + ctx = memtrack.Attach(ctx, "memory") -func (c *memoryTracker) dumpMemoryUsage(ctx context.Context) { - runtime.GC() + var ( + closed = make(chan struct{}) + wg sync.WaitGroup + ) - var ms runtime.MemStats - - runtime.ReadMemStats(&ms) - - c.memoryTrackerMutex.Lock() - defer c.memoryTrackerMutex.Unlock() - memlog(ctx).Debugf("in use heap %v (delta %v max %v) stack %v (delta %v max %v)", - ms.HeapInuse, int64(ms.HeapInuse-c.lastHeapUsage), c.maxHeapUsage, ms.StackInuse, int64(ms.StackInuse-c.lastStackInUse), c.maxStackInUse) - - if ms.HeapInuse > c.maxHeapUsage { - c.maxHeapUsage = ms.HeapInuse - } - - if ms.StackInuse > c.maxStackInUse { - c.maxStackInUse = ms.StackInuse - } - - c.lastHeapUsage = ms.HeapInuse - c.lastStackInUse = ms.StackInuse -} - -func (c *memoryTracker) startMemoryTracking(ctx context.Context) { if c.trackMemoryUsage > 0 { + ticker := time.NewTicker(c.trackMemoryUsage) + + wg.Add(1) + go func() { + defer wg.Done() + for { - c.dumpMemoryUsage(ctx) - time.Sleep(c.trackMemoryUsage) + select { + case <-closed: + return + + case <-ticker.C: + memtrack.Dump(ctx, "periodic") + } } }() } -} -func (c *memoryTracker) finishMemoryTracking(ctx context.Context) { - c.dumpMemoryUsage(ctx) + return ctx, func() { + close(closed) + wg.Wait() + + memtrack.Dump(ctx, "final") + } } diff --git a/cli/profile.go b/cli/profile.go index 084c51965..24d997aa1 100644 --- a/cli/profile.go +++ b/cli/profile.go @@ -1,5 +1,3 @@ -// +build profiling - package cli import ( @@ -30,12 +28,15 @@ func (c *profileFlags) withProfiling(callback func() error) error { if c.profileMemory > 0 { defer profile.Start(pp, profile.MemProfileRate(c.profileMemory)).Stop() } + if c.profileCPU { defer profile.Start(pp, profile.CPUProfile).Stop() } + if c.profileBlocking { defer profile.Start(pp, profile.BlockProfile).Stop() } + if c.profileMutex { defer profile.Start(pp, profile.MutexProfile).Stop() } diff --git a/cli/profile_disabled.go b/cli/profile_disabled.go deleted file mode 100644 index 6df55e6ff..000000000 --- a/cli/profile_disabled.go +++ /dev/null @@ -1,15 +0,0 @@ -// +build !profiling - -package cli - -import "github.com/alecthomas/kingpin" - -type profileFlags struct{} - -func (c *profileFlags) setup(app *kingpin.Application) { -} - -// withProfiling runs the given callback with profiling enabled, configured according to command line flags. -func (c *profileFlags) withProfiling(callback func() error) error { - return callback() -} diff --git a/cli/show_utils.go b/cli/show_utils.go index 8d2219a39..8e9ec6290 100644 --- a/cli/show_utils.go +++ b/cli/show_utils.go @@ -33,7 +33,7 @@ func showContentWithFlags(w io.Writer, rd io.Reader, unzip, indentJSON bool) err var buf1, buf2 bytes.Buffer if indentJSON { - if _, err := iocopy.Copy(&buf1, rd); err != nil { + if err := iocopy.JustCopy(&buf1, rd); err != nil { return errors.Wrap(err, "error copying data") } @@ -44,7 +44,7 @@ func showContentWithFlags(w io.Writer, rd io.Reader, unzip, indentJSON bool) err rd = ioutil.NopCloser(&buf2) } - if _, err := iocopy.Copy(w, rd); err != nil { + if err := iocopy.JustCopy(w, rd); err != nil { return errors.Wrap(err, "error copying data") } diff --git a/internal/blobtesting/asserts.go b/internal/blobtesting/asserts.go index ee29cc8b9..428bbefca 100644 --- a/internal/blobtesting/asserts.go +++ b/internal/blobtesting/asserts.go @@ -11,6 +11,7 @@ "github.com/pkg/errors" "github.com/stretchr/testify/require" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" ) @@ -20,11 +21,14 @@ func AssertGetBlob(ctx context.Context, t *testing.T, s blob.Storage, blobID blob.ID, expected []byte) { t.Helper() - b, err := s.GetBlob(ctx, blobID, 0, -1) + var b gather.WriteBuffer + defer b.Close() + + err := s.GetBlob(ctx, blobID, 0, -1, &b) require.NoErrorf(t, err, "GetBlob(%v)", blobID) - if !bytes.Equal(b, expected) { - t.Fatalf("GetBlob(%v) returned %x, but expected %x", blobID, b, expected) + if v := b.ToByteSlice(); !bytes.Equal(v, expected) { + t.Fatalf("GetBlob(%v) returned %x, but expected %x", blobID, v, expected) } half := int64(len(expected) / 2) @@ -32,35 +36,35 @@ func AssertGetBlob(ctx context.Context, t *testing.T, s blob.Storage, blobID blo return } - b, err = s.GetBlob(ctx, blobID, 0, 0) + err = s.GetBlob(ctx, blobID, 0, 0, &b) if err != nil { t.Fatalf("GetBlob(%v) returned error %v, expected data: %v", blobID, err, expected) return } - if len(b) != 0 { - t.Fatalf("GetBlob(%v) returned non-zero length: %v", blobID, len(b)) + if b.Length() != 0 { + t.Fatalf("GetBlob(%v) returned non-zero length: %v", blobID, b.Length()) return } - b, err = s.GetBlob(ctx, blobID, 0, half) + err = s.GetBlob(ctx, blobID, 0, half, &b) if err != nil { t.Fatalf("GetBlob(%v) returned error %v, expected data: %v", blobID, err, expected) return } - if !bytes.Equal(b, expected[0:half]) { - t.Fatalf("GetBlob(%v) returned %x, but expected %x", blobID, b, expected[0:half]) + if v := b.ToByteSlice(); !bytes.Equal(v, expected[0:half]) { + t.Fatalf("GetBlob(%v) returned %x, but expected %x", blobID, v, expected[0:half]) } - b, err = s.GetBlob(ctx, blobID, half, int64(len(expected))-half) + err = s.GetBlob(ctx, blobID, half, int64(len(expected))-half, &b) if err != nil { t.Fatalf("GetBlob(%v) returned error %v, expected data: %v", blobID, err, expected) return } - if !bytes.Equal(b, expected[len(expected)-int(half):]) { - t.Fatalf("GetBlob(%v) returned %x, but expected %x", blobID, b, expected[len(expected)-int(half):]) + if v := b.ToByteSlice(); !bytes.Equal(v, expected[len(expected)-int(half):]) { + t.Fatalf("GetBlob(%v) returned %x, but expected %x", blobID, v, expected[len(expected)-int(half):]) } AssertInvalidOffsetLength(ctx, t, s, blobID, -3, 1) @@ -73,7 +77,10 @@ func AssertGetBlob(ctx context.Context, t *testing.T, s blob.Storage, blobID blo func AssertInvalidOffsetLength(ctx context.Context, t *testing.T, s blob.Storage, blobID blob.ID, offset, length int64) { t.Helper() - if _, err := s.GetBlob(ctx, blobID, offset, length); err == nil { + var tmp gather.WriteBuffer + defer tmp.Close() + + if err := s.GetBlob(ctx, blobID, offset, length, &tmp); err == nil { t.Fatalf("GetBlob(%v,%v,%v) did not return error for invalid offset/length", blobID, offset, length) } } @@ -82,9 +89,12 @@ func AssertInvalidOffsetLength(ctx context.Context, t *testing.T, s blob.Storage func AssertGetBlobNotFound(ctx context.Context, t *testing.T, s blob.Storage, blobID blob.ID) { t.Helper() - b, err := s.GetBlob(ctx, blobID, 0, -1) - if !errors.Is(err, blob.ErrBlobNotFound) || b != nil { - t.Fatalf("GetBlob(%v) returned %v, %v but expected ErrNotFound", blobID, b, err) + var b gather.WriteBuffer + defer b.Close() + + err := s.GetBlob(ctx, blobID, 0, -1, &b) + if !errors.Is(err, blob.ErrBlobNotFound) || b.Length() != 0 { + t.Fatalf("GetBlob(%v) returned %v, %v but expected ErrNotFound", blobID, b.Length(), err) } } diff --git a/internal/blobtesting/concurrent.go b/internal/blobtesting/concurrent.go index ad95758fd..4347875b3 100644 --- a/internal/blobtesting/concurrent.go +++ b/internal/blobtesting/concurrent.go @@ -54,6 +54,9 @@ func VerifyConcurrentAccess(t *testing.T, st blob.Storage, options ConcurrentAcc // start readers that will be reading random blob out of the pool for i := 0; i < options.Getters; i++ { eg.Go(func() error { + var data gather.WriteBuffer + defer data.Close() + for i := 0; i < options.Iterations; i++ { blobID := randomBlobID() offset := int64(0) @@ -64,10 +67,10 @@ func VerifyConcurrentAccess(t *testing.T, st blob.Storage, options ConcurrentAcc length = 3 } - data, err := st.GetBlob(ctx, blobID, offset, length) + err := st.GetBlob(ctx, blobID, offset, length, &data) switch { case err == nil: - if got, want := string(data), string(blobID); !strings.HasPrefix(got, want) { + if got, want := string(data.ToByteSlice()), string(blobID); !strings.HasPrefix(got, want) { return errors.Wrapf(err, "GetBlob returned invalid data for %v: %v, want prefix of %v", blobID, got, want) } diff --git a/internal/blobtesting/eventually_consistent.go b/internal/blobtesting/eventually_consistent.go index 731448c96..e33376668 100644 --- a/internal/blobtesting/eventually_consistent.go +++ b/internal/blobtesting/eventually_consistent.go @@ -12,6 +12,7 @@ "github.com/pkg/errors" "github.com/kopia/kopia/internal/clock" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" ) @@ -101,10 +102,10 @@ func (s *eventuallyConsistentStorage) randomFrontendCache() *ecFrontendCache { return s.caches[n] } -func (s *eventuallyConsistentStorage) GetBlob(ctx context.Context, id blob.ID, offset, length int64) ([]byte, error) { +func (s *eventuallyConsistentStorage) GetBlob(ctx context.Context, id blob.ID, offset, length int64, output *gather.WriteBuffer) error { // don't bother caching partial reads if length >= 0 { - return s.realStorage.GetBlob(ctx, id, offset, length) + return s.realStorage.GetBlob(ctx, id, offset, length, output) } c := s.randomFrontendCache() @@ -113,25 +114,27 @@ func (s *eventuallyConsistentStorage) GetBlob(ctx context.Context, id blob.ID, o e := c.get(id) if e != nil { if e.data == nil { - return nil, blob.ErrBlobNotFound + return blob.ErrBlobNotFound } - return append([]byte(nil), e.data...), nil + output.Append(e.data) + + return nil } // fetch from the underlying storage. - v, err := s.realStorage.GetBlob(ctx, id, offset, length) + err := s.realStorage.GetBlob(ctx, id, offset, length, output) if err != nil { if errors.Is(err, blob.ErrBlobNotFound) { c.put(id, nil) } - return nil, err + return err } - c.put(id, v) + c.put(id, output.ToByteSlice()) - return v, nil + return nil } func (s *eventuallyConsistentStorage) GetMetadata(ctx context.Context, id blob.ID) (blob.Metadata, error) { diff --git a/internal/blobtesting/faulty.go b/internal/blobtesting/faulty.go index 2cd01819a..74e0859df 100644 --- a/internal/blobtesting/faulty.go +++ b/internal/blobtesting/faulty.go @@ -6,6 +6,7 @@ "testing" "time" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" "github.com/kopia/kopia/repo/logging" ) @@ -30,12 +31,12 @@ type FaultyStorage struct { } // GetBlob implements blob.Storage. -func (s *FaultyStorage) GetBlob(ctx context.Context, id blob.ID, offset, length int64) ([]byte, error) { +func (s *FaultyStorage) GetBlob(ctx context.Context, id blob.ID, offset, length int64, output *gather.WriteBuffer) error { if err := s.getNextFault(ctx, "GetBlob", id, offset, length); err != nil { - return nil, err + return err } - return s.Base.GetBlob(ctx, id, offset, length) + return s.Base.GetBlob(ctx, id, offset, length, output) } // GetMetadata implements blob.Storage. diff --git a/internal/blobtesting/map.go b/internal/blobtesting/map.go index 5e32efc1c..2aa955795 100644 --- a/internal/blobtesting/map.go +++ b/internal/blobtesting/map.go @@ -11,6 +11,7 @@ "github.com/pkg/errors" "github.com/kopia/kopia/internal/clock" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" ) @@ -24,31 +25,35 @@ type mapStorage struct { mutex sync.RWMutex } -func (s *mapStorage) GetBlob(ctx context.Context, id blob.ID, offset, length int64) ([]byte, error) { +func (s *mapStorage) GetBlob(ctx context.Context, id blob.ID, offset, length int64, output *gather.WriteBuffer) error { s.mutex.RLock() defer s.mutex.RUnlock() + output.Reset() + data, ok := s.data[id] if ok { - data = append([]byte(nil), data...) - if length < 0 { - return data, nil + output.Append(data) + + return nil } if int(offset) > len(data) || offset < 0 { - return nil, errors.Wrapf(blob.ErrInvalidRange, "invalid offset: %v", offset) + return errors.Wrapf(blob.ErrInvalidRange, "invalid offset: %v", offset) } data = data[offset:] if int(length) > len(data) { - return nil, errors.Wrapf(blob.ErrInvalidRange, "invalid length: %v", length) + return errors.Wrapf(blob.ErrInvalidRange, "invalid length: %v", length) } - return data[0:length], nil + output.Append(data[0:length]) + + return nil } - return nil, blob.ErrBlobNotFound + return blob.ErrBlobNotFound } func (s *mapStorage) GetMetadata(ctx context.Context, id blob.ID) (blob.Metadata, error) { diff --git a/internal/buf/pool.go b/internal/buf/pool.go deleted file mode 100644 index ecda6f975..000000000 --- a/internal/buf/pool.go +++ /dev/null @@ -1,269 +0,0 @@ -// Package buf manages allocation of temporary short-term buffers. -package buf - -import ( - "context" - "runtime/debug" - "sync" - "sync/atomic" - "time" - - "go.opencensus.io/stats" - "go.opencensus.io/tag" -) - -// DisableBufferManagement is a global flag that disables memory buffer reuse, -// which can be useful in tests to reduce overall memory usage. -var DisableBufferManagement = false - -type segment struct { - mu sync.RWMutex - - nextUnallocated int // high water mark - allocatedBufCount int // how many outstanding users of the segment there are - data []byte // the underlying buffer from which we're allocating - pool *Pool -} - -// Buf represents allocated slice of memory pool. At the end of using the buffer, Release() must be called to -// reclaim memory. -type Buf struct { - Data []byte - - nextUnallocated int - previousNextUnallocated int - segment *segment // segment from which the data was allocated -} - -// IsPooled determines whether data slice is part of a pool. -func (b *Buf) IsPooled() bool { return b.segment != nil } - -// Release returns the slice back to the pool. -func (b *Buf) Release() { - if b.segment == nil { - return - } - - atomic.AddInt64(&b.segment.pool.totalReleasedBytes, int64(len(b.Data))) - atomic.AddInt64(&b.segment.pool.totalReleasedBuffers, 1) - - b.segment.mu.Lock() - defer b.segment.mu.Unlock() - - // best effort compare-and-swap, which will pop the buffer off the stack in its appropriate segment - if b.segment.nextUnallocated == b.nextUnallocated { - b.segment.nextUnallocated = b.previousNextUnallocated - } - - b.segment.allocatedBufCount-- - if b.segment.allocatedBufCount == 0 { - // last allocated Buf, we can reset 'next' to zero - b.segment.nextUnallocated = 0 - } - - b.Data = nil - b.segment = nil -} - -func (s *segment) allocate(n int) (Buf, bool) { - // quick check using shared lock - s.mu.RLock() - haveRoom := s.nextUnallocated+n <= len(s.data) - s.mu.RUnlock() - - if !haveRoom { - return Buf{}, false - } - - // we likely have space, allocate under exclusive lock - s.mu.Lock() - defer s.mu.Unlock() - - // see if we have capacity in this segment - nu := s.nextUnallocated - - if nu+n > len(s.data) { - // out of space in this segment - return Buf{}, false - } - - s.allocatedBufCount++ - s.nextUnallocated += n - - return Buf{ - Data: s.data[nu : nu+n : nu+n], - nextUnallocated: nu + n, - previousNextUnallocated: nu, - segment: s, - }, true -} - -// Pool manages allocations of short-term data buffers from a pool. -// -// Note that buffers managed by the pool are meant to be extremely short lived and are suitable -// for in-memory operations, such as encryption, compression, etc, but not for I/O buffers of any kind. -// It is EXTREMELY important to always release memory allocated from the Pool. Failure to do so will -// result in memory leaks. -// -// The pool uses N segments, with each segment tracking its high water mark usage. -// -// ation simply advances the high water mark within first segment that has capacity -// and increments per-segment refcount. -// -// On Buf.Release() the refcount is decremented and when it hits zero, the entire segment becomes instantly -// freed. -// -// As an extra optimization, when Buf.Release() is called in LIFO order, it will also lower the -// high water mark making its memory available for immediate reuse. -// -// If no segment has available capacity, the pool waits a few times until memory becomes released -// and falls back to allocating from the heap. -type Pool struct { - totalAllocatedBytes int64 - totalReleasedBytes int64 - totalAllocatedBuffers int64 - totalReleasedBuffers int64 - - poolID string - - closed chan struct{} - - tagMutators []tag.Mutator - - // this protects the slice, to be able to atomically replace it - mu sync.Mutex - segmentSize int - segments []*segment -} - -var ( - activePoolsMutex sync.Mutex - activePools = map[*Pool]string{} -) - -// NewPool creates a buffer pool, composed of fixed-length segments of specified maximum size. -func NewPool(ctx context.Context, segmentSize int, poolID string) *Pool { - p := &Pool{ - poolID: poolID, - tagMutators: []tag.Mutator{tag.Insert(tagKeyPool, poolID)}, - segmentSize: segmentSize, - closed: make(chan struct{}), - } - - activePoolsMutex.Lock() - activePools[p] = string(debug.Stack()) - activePoolsMutex.Unlock() - - go func() { - for { - select { - case <-p.closed: - return - - case <-time.After(1 * time.Second): - p.reportMetrics(ctx) - } - } - }() - - return p -} - -// Close closes the pool. -func (p *Pool) Close() { - close(p.closed) - - activePoolsMutex.Lock() - delete(activePools, p) - activePoolsMutex.Unlock() -} - -// ActivePools returns the set of active activePools. -func ActivePools() map[*Pool]string { - activePoolsMutex.Lock() - defer activePoolsMutex.Unlock() - - r := map[*Pool]string{} - for k, v := range activePools { - r[k] = v - } - - return r -} - -func (p *Pool) reportMetrics(ctx context.Context) { - allBytes := atomic.LoadInt64(&p.totalAllocatedBytes) - relBytes := atomic.LoadInt64(&p.totalReleasedBytes) - allBuffers := atomic.LoadInt64(&p.totalAllocatedBuffers) - relBuffers := atomic.LoadInt64(&p.totalReleasedBuffers) - - _ = stats.RecordWithTags( - ctx, - p.tagMutators, - metricPoolAllocatedBytes.M(allBytes), - metricPoolReleasedBytes.M(relBytes), - metricPoolOutstandingBytes.M(allBytes-relBytes), - metricPoolAllocatedBuffers.M(allBuffers), - metricPoolReleasedBuffers.M(relBuffers), - metricPoolOutstandingBuffers.M(allBuffers-relBuffers), - metricPoolNumSegments.M(int64(len(p.currentSegments()))), - ) -} - -func (p *Pool) currentSegments() []*segment { - p.mu.Lock() - defer p.mu.Unlock() - - return p.segments -} - -// SetSegmentSize sets the segment size for future segments that will be created. -func (p *Pool) SetSegmentSize(maxSize int) { - p.mu.Lock() - defer p.mu.Unlock() - - p.segmentSize = maxSize -} - -// AddSegments n segments to the pool. -func (p *Pool) AddSegments(n int) { - p.mu.Lock() - defer p.mu.Unlock() - - var newSegments []*segment - - newSegments = append(newSegments, p.segments...) - - for i := 0; i < n; i++ { - newSegments = append(newSegments, &segment{ - data: make([]byte, p.segmentSize), - pool: p, - }) - } - - p.segments = newSegments -} - -// Allocate allocates from the buffer a slice of size n. -func (p *Pool) Allocate(n int) Buf { - // requested more than the pool can cache, allocate throw-away buffer. - if p == nil || n > p.segmentSize || DisableBufferManagement { - return Buf{make([]byte, n), 0, 0, nil} - } - - atomic.AddInt64(&p.totalAllocatedBytes, int64(n)) - atomic.AddInt64(&p.totalAllocatedBuffers, 1) - - for { - // try to allocate - for _, s := range p.currentSegments() { - buf, ok := s.allocate(n) - if ok { - return buf - } - } - - // add one more segment - p.AddSegments(1) - } -} diff --git a/internal/buf/pool_metrics.go b/internal/buf/pool_metrics.go deleted file mode 100644 index 14cbaa634..000000000 --- a/internal/buf/pool_metrics.go +++ /dev/null @@ -1,78 +0,0 @@ -package buf - -import ( - "go.opencensus.io/stats" - "go.opencensus.io/stats/view" - "go.opencensus.io/tag" -) - -var tagKeyPool = tag.MustNewKey("pool") - -// buffer pool metrics. -var ( - metricPoolAllocatedBuffers = stats.Int64( - "kopia/bufferpool/allocated_buffers", - "Number of buffers allocated from a pool", - stats.UnitDimensionless, - ) - - metricPoolAllocatedBytes = stats.Int64( - "kopia/bufferpool/allocated_bytes", - "Number of bytes allocated from a pool", - stats.UnitDimensionless, - ) - - metricPoolReleasedBuffers = stats.Int64( - "kopia/bufferpool/released_buffers", - "Number of buffers released back to the pool", - stats.UnitBytes, - ) - - metricPoolReleasedBytes = stats.Int64( - "kopia/bufferpool/released_bytes", - "Number of bytes released from a pool", - stats.UnitBytes, - ) - - metricPoolOutstandingBuffers = stats.Int64( - "kopia/bufferpool/outstanding_buffers", - "Number of buffers allocated from a pool but not returned yet", - stats.UnitBytes, - ) - - metricPoolOutstandingBytes = stats.Int64( - "kopia/bufferpool/outstanding_bytes", - "Number of bytes allocated from a pool but not returned yet", - stats.UnitBytes, - ) - - metricPoolNumSegments = stats.Int64( - "kopia/bufferpool/num_segments", - "Number of segments in the pool", - stats.UnitDimensionless, - ) -) - -func aggregateByPool(m stats.Measure, agg *view.Aggregation) *view.View { - return &view.View{ - Name: m.Name(), - Aggregation: agg, - Description: m.Description(), - Measure: m, - TagKeys: []tag.Key{tagKeyPool}, - } -} - -func init() { - if err := view.Register( - aggregateByPool(metricPoolAllocatedBytes, view.LastValue()), - aggregateByPool(metricPoolOutstandingBytes, view.LastValue()), - aggregateByPool(metricPoolReleasedBytes, view.LastValue()), - aggregateByPool(metricPoolAllocatedBuffers, view.LastValue()), - aggregateByPool(metricPoolOutstandingBuffers, view.LastValue()), - aggregateByPool(metricPoolReleasedBuffers, view.LastValue()), - aggregateByPool(metricPoolNumSegments, view.LastValue()), - ); err != nil { - panic("unable to register opencensus views: " + err.Error()) - } -} diff --git a/internal/buf/pool_test.go b/internal/buf/pool_test.go deleted file mode 100644 index b681440d9..000000000 --- a/internal/buf/pool_test.go +++ /dev/null @@ -1,90 +0,0 @@ -package buf - -import ( - "context" - "runtime" - "sync" - "testing" -) - -func TestPool(t *testing.T) { - var wg sync.WaitGroup - - ctx := context.Background() - - // 20 buffers of 1 MB each - a := NewPool(ctx, 1000000, "testing-pool") - defer a.Close() - - a.AddSegments(20) - - var ms1, ms2 runtime.MemStats - - runtime.ReadMemStats(&ms1) - - repeat := 1000000 - numGoRoutines := 30 - - if runtime.GOARCH != "amd64" { - repeat = 10000 - numGoRoutines = 10 - } - - // 30 gorouties, each allocating and releasing memory 1 M times - for i := 0; i < numGoRoutines; i++ { - wg.Add(1) - - go func() { - defer wg.Done() - - for j := 0; j < repeat; j++ { - const allocSize = 100000 - - b := a.Allocate(allocSize) - - if got, want := len(b.Data), allocSize; got != want { - t.Errorf("unexpected len: %v, want %v", got, want) - } - - if got, want := cap(b.Data), allocSize; got != want { - t.Errorf("unexpected cap: %v, want %v", got, want) - } - - if !b.IsPooled() { - t.Errorf("unexpected !IsPooled()") - } - - b.Release() - } - }() - } - - wg.Wait() - runtime.ReadMemStats(&ms2) - - // amount of memory should be O(kilobytes), not 1 MB because all buffers got preallocated - if diff := ms2.TotalAlloc - ms1.TotalAlloc; diff > 1000000 { - t.Errorf("too much memory was allocated: %v", diff) - } -} - -func TestNilPool(t *testing.T) { - var a *Pool - - // allocate from nil pool - b := a.Allocate(5) - - if got, want := len(b.Data), 5; got != want { - t.Errorf("unexpected len: %v, want %v", got, want) - } - - if got, want := cap(b.Data), 5; got != want { - t.Errorf("unexpected cap: %v, want %v", got, want) - } - - if b.IsPooled() { - t.Errorf("unexpected IsPooled()") - } - - b.Release() -} diff --git a/internal/cache/persistent_lru_cache.go b/internal/cache/persistent_lru_cache.go index 4be7e492e..b2831279b 100644 --- a/internal/cache/persistent_lru_cache.go +++ b/internal/cache/persistent_lru_cache.go @@ -46,55 +46,55 @@ type PersistentCache struct { // GetOrLoad is utility function gets the provided item from the cache or invokes the provided fetch function. // The function also appends and verifies HMAC checksums using provided secret on all cached items to ensure data integrity. -func (c *PersistentCache) GetOrLoad(ctx context.Context, key string, fetch func() ([]byte, error)) ([]byte, error) { +func (c *PersistentCache) GetOrLoad(ctx context.Context, key string, fetch func(output *gather.WriteBuffer) error, output *gather.WriteBuffer) error { if c == nil { // special case - also works on non-initialized cache pointer. - return fetch() + return fetch(output) } - if b := c.Get(ctx, key, 0, -1); b != nil { - return b, nil + if c.Get(ctx, key, 0, -1, output) { + return nil } - b, err := fetch() - if err != nil { + if err := fetch(output); err != nil { stats.Record(ctx, MetricMissErrors.M(1)) - return nil, err + return err } - stats.Record(ctx, MetricMissBytes.M(int64(len(b)))) + stats.Record(ctx, MetricMissBytes.M(int64(output.Length()))) - c.Put(ctx, key, b) + c.Put(ctx, key, output.Bytes()) - return b, nil + return nil } // Get fetches the contents of a cached blob when (length < 0) or a subset of it (when length >= 0). // returns nil if not found. -func (c *PersistentCache) Get(ctx context.Context, key string, offset, length int64) []byte { +func (c *PersistentCache) Get(ctx context.Context, key string, offset, length int64, output *gather.WriteBuffer) bool { if c == nil { - return nil + return false } if length >= 0 && !c.storageProtection.SupportsPartial() { - return nil + return false } - v, err := c.cacheStorage.GetBlob(ctx, blob.ID(key), offset, length) - if err == nil { - vb, err := c.storageProtection.Verify(key, v) - if err == nil { + var tmp gather.WriteBuffer + defer tmp.Close() + + if err := c.cacheStorage.GetBlob(ctx, blob.ID(key), offset, length, &tmp); err == nil { + if err := c.storageProtection.Verify(key, tmp.Bytes(), output); err == nil { // cache hit stats.Record(ctx, MetricHitCount.M(1), - MetricHitBytes.M(int64(len(vb))), + MetricHitBytes.M(int64(output.Length())), ) // cache hit c.cacheStorage.TouchBlob(ctx, blob.ID(key), c.touchThreshold) //nolint:errcheck - return vb + return true } // delete invalid blob @@ -108,18 +108,23 @@ func (c *PersistentCache) Get(ctx context.Context, key string, offset, length in // cache miss stats.Record(ctx, MetricMissCount.M(1)) - return nil + return false } // Put adds the provided key-value pair to the cache. -func (c *PersistentCache) Put(ctx context.Context, key string, data []byte) { +func (c *PersistentCache) Put(ctx context.Context, key string, data gather.Bytes) { if c == nil { return } atomic.StoreInt32(&c.anyChange, 1) - if err := c.cacheStorage.PutBlob(ctx, blob.ID(key), gather.FromSlice(c.storageProtection.Protect(key, data))); err != nil { + var protected gather.WriteBuffer + defer protected.Close() + + c.storageProtection.Protect(key, data, &protected) + + if err := c.cacheStorage.PutBlob(ctx, blob.ID(key), protected.Bytes()); err != nil { stats.Record(ctx, MetricStoreErrors.M(1)) log(ctx).Errorf("unable to add %v to %v: %v", key, c.description, err) diff --git a/internal/cache/persistent_lru_cache_test.go b/internal/cache/persistent_lru_cache_test.go index 5cd09ddf1..d11d53a1c 100644 --- a/internal/cache/persistent_lru_cache_test.go +++ b/internal/cache/persistent_lru_cache_test.go @@ -7,8 +7,10 @@ "time" "github.com/pkg/errors" + "github.com/stretchr/testify/require" "github.com/kopia/kopia/internal/cache" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/testlogging" "github.com/kopia/kopia/internal/testutil" "github.com/kopia/kopia/repo/blob" @@ -30,26 +32,31 @@ func TestPersistentLRUCache(t *testing.T) { t.Fatal(err) } - if got := pc.Get(ctx, "key", 0, -1); got != nil { - t.Fatalf("unexpected cache hit on empty cache: %x", got) + var tmp gather.WriteBuffer + defer tmp.Close() + + if got := pc.Get(ctx, "key", 0, -1, &tmp); got { + t.Fatalf("unexpected cache hit on empty cache") } someData := bytes.Repeat([]byte{1}, 300) - pc.Put(ctx, "key1", someData) + pc.Put(ctx, "key1", gather.FromSlice(someData)) verifyBlobExists(ctx, t, cs, "key1") // sleep between adding key1 and the rest to make it easily the oldest // even if the filesystem is not very precise keeping time. time.Sleep(2 * time.Second) - pc.Put(ctx, "key2", someData) + pc.Put(ctx, "key2", gather.FromSlice(someData)) verifyBlobExists(ctx, t, cs, "key2") - pc.Put(ctx, "key3", someData) + pc.Put(ctx, "key3", gather.FromSlice(someData)) verifyBlobExists(ctx, t, cs, "key3") - pc.Put(ctx, "key4", someData) + pc.Put(ctx, "key4", gather.FromSlice(someData)) verifyBlobExists(ctx, t, cs, "key4") - if got, want := pc.Get(ctx, "key2", 0, -1), someData; !bytes.Equal(got, want) { + require.True(t, pc.Get(ctx, "key2", 0, -1, &tmp)) + + if got, want := tmp.ToByteSlice(), someData; !bytes.Equal(got, want) { t.Fatalf("invalid data retrieved from cache: %x", got) } @@ -77,8 +84,17 @@ func TestPersistentLRUCache(t *testing.T) { func verifyCached(ctx context.Context, t *testing.T, pc *cache.PersistentCache, key string, want []byte) { t.Helper() - if got := pc.Get(ctx, key, 0, -1); !bytes.Equal(got, want) { - t.Fatalf("invalid cached result for %v: %x, want %x", key, got, want) + var tmp gather.WriteBuffer + defer tmp.Close() + + if want == nil { + require.False(t, pc.Get(ctx, key, 0, -1, &tmp)) + } else { + require.True(t, pc.Get(ctx, key, 0, -1, &tmp)) + + if got := tmp.ToByteSlice(); !bytes.Equal(got, want) { + t.Fatalf("invalid cached result for %v: %x, want %x", key, got, want) + } } } diff --git a/internal/cache/storage_protection.go b/internal/cache/storage_protection.go index 41da8bf28..ac48ed0e6 100644 --- a/internal/cache/storage_protection.go +++ b/internal/cache/storage_protection.go @@ -5,6 +5,7 @@ "github.com/pkg/errors" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/hmac" "github.com/kopia/kopia/repo/encryption" ) @@ -15,18 +16,22 @@ // StorageProtection encapsulates protection (HMAC and/or encryption) applied to local cache items. type StorageProtection interface { SupportsPartial() bool - Protect(id string, b []byte) []byte - Verify(id string, b []byte) ([]byte, error) + Protect(id string, input gather.Bytes, output *gather.WriteBuffer) + Verify(id string, input gather.Bytes, output *gather.WriteBuffer) error } type nullStorageProtection struct{} -func (nullStorageProtection) Protect(id string, b []byte) []byte { - return b +func (nullStorageProtection) Protect(id string, input gather.Bytes, output *gather.WriteBuffer) { + output.Reset() + input.WriteTo(output) // nolint:errcheck } -func (nullStorageProtection) Verify(id string, b []byte) ([]byte, error) { - return b, nil +func (nullStorageProtection) Verify(id string, input gather.Bytes, output *gather.WriteBuffer) error { + output.Reset() + input.WriteTo(output) // nolint:errcheck + + return nil } func (nullStorageProtection) SupportsPartial() bool { @@ -42,13 +47,15 @@ type checksumProtection struct { Secret []byte } -func (p checksumProtection) Protect(id string, b []byte) []byte { - return hmac.Append(b, p.Secret) +func (p checksumProtection) Protect(id string, input gather.Bytes, output *gather.WriteBuffer) { + output.Reset() + hmac.Append(input, p.Secret, output) } -func (p checksumProtection) Verify(id string, b []byte) ([]byte, error) { +func (p checksumProtection) Verify(id string, input gather.Bytes, output *gather.WriteBuffer) error { + output.Reset() // nolint:wrapcheck - return hmac.VerifyAndStrip(b, p.Secret) + return hmac.VerifyAndStrip(input, p.Secret, output) } func (checksumProtection) SupportsPartial() bool { @@ -69,26 +76,26 @@ func (p authenticatedEncryptionProtection) deriveIV(id string) []byte { return contentID[:] } -func (p authenticatedEncryptionProtection) Protect(id string, b []byte) []byte { - c, err := p.e.Encrypt(nil, b, p.deriveIV(id)) - if err != nil { +func (p authenticatedEncryptionProtection) Protect(id string, input gather.Bytes, output *gather.WriteBuffer) { + output.Reset() + + if err := p.e.Encrypt(input, p.deriveIV(id), output); err != nil { panic("encryption unexpectedly failed: " + err.Error()) } - - return c } func (authenticatedEncryptionProtection) SupportsPartial() bool { return false } -func (p authenticatedEncryptionProtection) Verify(id string, b []byte) ([]byte, error) { - v, err := p.e.Decrypt(nil, b, p.deriveIV(id)) - if err != nil { - return nil, errors.Wrap(err, "unable to decrypt cache content") +func (p authenticatedEncryptionProtection) Verify(id string, input gather.Bytes, output *gather.WriteBuffer) error { + output.Reset() + + if err := p.e.Decrypt(input, p.deriveIV(id), output); err != nil { + return errors.Wrap(err, "unable to decrypt cache content") } - return v, nil + return nil } type authenticatedEncryptionProtectionKey []byte diff --git a/internal/cache/storage_protection_test.go b/internal/cache/storage_protection_test.go index 9195253e4..ea2c86db2 100644 --- a/internal/cache/storage_protection_test.go +++ b/internal/cache/storage_protection_test.go @@ -4,7 +4,10 @@ "bytes" "testing" + "github.com/stretchr/testify/require" + "github.com/kopia/kopia/internal/cache" + "github.com/kopia/kopia/internal/gather" ) func TestHMACStorageProtection(t *testing.T) { @@ -24,17 +27,30 @@ func TestEncryptionStorageProtection(t *testing.T) { func testStorageProtection(t *testing.T, sp cache.StorageProtection) { payload := []byte{0, 1, 2, 3, 4} - protected := sp.Protect("x", payload) + var protected gather.WriteBuffer + defer protected.Close() - unprotected, err := sp.Verify("x", protected) - if err != nil { - t.Fatal(err) - } + // append dummy bytes to ensure Reset is called. + protected.Append([]byte("dummy")) - if got, want := unprotected, payload; !bytes.Equal(got, want) { + sp.Protect("x", gather.FromSlice(payload), &protected) + + var unprotected gather.WriteBuffer + defer unprotected.Close() + + // append dummy bytes to ensure Reset is called. + unprotected.Append([]byte("dummy")) + + require.NoError(t, sp.Verify("x", protected.Bytes(), &unprotected)) + + if got, want := unprotected.ToByteSlice(), payload; !bytes.Equal(got, want) { t.Fatalf("invalid unprotected payload %x, wanted %x", got, want) } + pb := protected.ToByteSlice() + // flip one bit - protected[0] ^= 1 + pb[0] ^= 1 + + require.Error(t, sp.Verify("x", gather.FromSlice(pb), &unprotected)) } diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 82275d9e4..865b6100f 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -287,9 +287,7 @@ func downloadFile(ctx context.Context, f fs.File, fname string) error { defer dst.Close() //nolint:errcheck,gosec - _, err = iocopy.Copy(dst, src) - - return errors.Wrap(err, "error downloading file") + return errors.Wrap(iocopy.JustCopy(dst, src), "error downloading file") } func (c *Comparer) output(msg string, args ...interface{}) { diff --git a/internal/epoch/epoch_manager_test.go b/internal/epoch/epoch_manager_test.go index f4f93a7ef..4b4bcc2f3 100644 --- a/internal/epoch/epoch_manager_test.go +++ b/internal/epoch/epoch_manager_test.go @@ -595,13 +595,16 @@ func (te *epochManagerTestEnv) verifyCompleteIndexSet(ctx context.Context, t *te func (te *epochManagerTestEnv) getMergedIndexContents(ctx context.Context, blobIDs []blob.ID) (*fakeIndex, error) { result := &fakeIndex{} + var v gather.WriteBuffer + defer v.Close() + for _, blobID := range blobIDs { - v, err := te.unloggedst.GetBlob(ctx, blobID, 0, -1) + err := te.unloggedst.GetBlob(ctx, blobID, 0, -1, &v) if err != nil { return nil, errors.Wrap(err, "unable to get blob") } - ndx, err := parseFakeIndex(v) + ndx, err := parseFakeIndex(v.ToByteSlice()) if err != nil { return nil, errors.Wrap(err, "unable to parse fake index") } diff --git a/internal/fshasher/fshasher.go b/internal/fshasher/fshasher.go index 3098aad2e..0322673a8 100644 --- a/internal/fshasher/fshasher.go +++ b/internal/fshasher/fshasher.go @@ -131,9 +131,5 @@ func writeFile(ctx context.Context, w io.Writer, f fs.File) error { } defer r.Close() //nolint:errcheck - if _, err = iocopy.Copy(w, r); err != nil { - return err - } - - return nil + return iocopy.JustCopy(w, r) } diff --git a/internal/gather/gather_bytes.go b/internal/gather/gather_bytes.go index 48e005bbd..38ad27eb3 100644 --- a/internal/gather/gather_bytes.go +++ b/internal/gather/gather_bytes.go @@ -5,8 +5,13 @@ import ( "bytes" "io" + + "github.com/google/uuid" + "github.com/pkg/errors" ) +var invalidSliceBuf = []byte(uuid.NewString()) + // Bytes represents a sequence of bytes split into slices. type Bytes struct { Slices [][]byte @@ -16,8 +21,25 @@ type Bytes struct { sliceBuf [1][]byte } -// AppendSectionTo appends the section of the buffer to the provided slice and returns it. -func (b *Bytes) AppendSectionTo(output []byte, offset, size int) []byte { +func (b *Bytes) invalidate() { + b.sliceBuf[0] = invalidSliceBuf + b.Slices = nil +} + +func (b *Bytes) assertValid() { + if len(b.sliceBuf[0]) == len(invalidSliceBuf) && bytes.Equal(b.sliceBuf[0], invalidSliceBuf) { + panic("gather.Bytes is invalid") + } +} + +// AppendSectionTo writes the section of the buffer to the provided writer. +func (b *Bytes) AppendSectionTo(w io.Writer, offset, size int) error { + b.assertValid() + + if offset < 0 { + return errors.Errorf("invalid offset") + } + // find the index of starting slice sliceNdx := -1 @@ -44,7 +66,10 @@ func (b *Bytes) AppendSectionTo(output []byte, offset, size int) []byte { firstChunkSize = len(b.Slices[sliceNdx]) - offset } - output = append(output, b.Slices[sliceNdx][offset:offset+firstChunkSize]...) + if _, err := w.Write(b.Slices[sliceNdx][offset : offset+firstChunkSize]); err != nil { + return errors.Wrap(err, "error appending") + } + size -= firstChunkSize sliceNdx++ @@ -58,16 +83,21 @@ func (b *Bytes) AppendSectionTo(output []byte, offset, size int) []byte { l = len(s) } - output = append(output, s[0:l]...) + if _, err := w.Write(s[0:l]); err != nil { + return errors.Wrap(err, "error appending") + } + size -= l sliceNdx++ } - return output + return nil } // Length returns the combined length of all slices. func (b Bytes) Length() int { + b.assertValid() + l := 0 for _, data := range b.Slices { @@ -77,8 +107,17 @@ func (b Bytes) Length() int { return l } +// ReadAt implements io.ReaderAt interface. +func (b Bytes) ReadAt(p []byte, off int64) (n int, err error) { + b.assertValid() + + return len(p), b.AppendSectionTo(bytes.NewBuffer(p[:0]), int(off), len(p)) +} + // Reader returns a reader for the data. func (b Bytes) Reader() io.Reader { + b.assertValid() + switch len(b.Slices) { case 0: return bytes.NewReader(nil) @@ -97,8 +136,12 @@ func (b Bytes) Reader() io.Reader { } } -// GetBytes appends all bytes to the provided slice and returns it. -func (b Bytes) GetBytes(output []byte) []byte { +// ToByteSlice returns contents as a newly-allocated byte slice. +func (b Bytes) ToByteSlice() []byte { + b.assertValid() + + output := []byte{} + for _, v := range b.Slices { output = append(output, v...) } @@ -108,6 +151,8 @@ func (b Bytes) GetBytes(output []byte) []byte { // WriteTo writes contents to the specified writer and returns number of bytes written. func (b Bytes) WriteTo(w io.Writer) (int64, error) { + b.assertValid() + var totalN int64 for _, v := range b.Slices { diff --git a/internal/gather/gather_bytes_test.go b/internal/gather/gather_bytes_test.go index 9cf3a8a6a..93510b9f7 100644 --- a/internal/gather/gather_bytes_test.go +++ b/internal/gather/gather_bytes_test.go @@ -4,6 +4,8 @@ "bytes" "io/ioutil" "testing" + + "github.com/stretchr/testify/require" ) var sample1 = []byte("hello! how are you? nice to meet you.") @@ -15,17 +17,17 @@ func TestGatherBytes(t *testing.T) { sliced Bytes }{ { - whole: nil, + whole: []byte{}, sliced: Bytes{}, }, { - whole: nil, + whole: []byte{}, sliced: Bytes{Slices: [][]byte{ nil, }}, }, { - whole: nil, + whole: []byte{}, sliced: Bytes{Slices: [][]byte{ nil, {}, @@ -91,18 +93,22 @@ func TestGatherBytes(t *testing.T) { } // GetBytes - all = b.GetBytes(nil) + all = b.ToByteSlice() if !bytes.Equal(all, tc.whole) { t.Errorf("unexpected data from GetBytes() %v, want %v", string(all), string(tc.whole)) } // AppendSectionTo - test exhaustively all combinationf os start, length + var tmp WriteBuffer + defer tmp.Close() + for i := 0; i <= len(tc.whole); i++ { for j := i; j <= len(tc.whole); j++ { - result := b.AppendSectionTo(nil, i, j-i) - if !bytes.Equal(result, tc.whole[i:j]) { - t.Fatalf("invalid section") - } + tmp.Reset() + + b.AppendSectionTo(&tmp, i, j-i) + + require.Equal(t, tmp.ToByteSlice(), tc.whole[i:j]) } } } diff --git a/internal/gather/gather_write_buffer.go b/internal/gather/gather_write_buffer.go index 3e53e9457..a2dfb284e 100644 --- a/internal/gather/gather_write_buffer.go +++ b/internal/gather/gather_write_buffer.go @@ -1,10 +1,18 @@ package gather -import "sync" +import ( + "io" + "sync" + + "github.com/kopia/kopia/repo/logging" +) + +var log = logging.GetContextLoggerFunc("gather") // WriteBuffer is a write buffer for content of unknown size that manages // data in a series of byte slices of uniform size. type WriteBuffer struct { + alloc *chunkAllocator mu sync.Mutex inner Bytes } @@ -14,11 +22,31 @@ func (b *WriteBuffer) Close() { b.mu.Lock() defer b.mu.Unlock() - for _, s := range b.inner.Slices { - releaseChunk(s) + if b.alloc != nil { + for _, s := range b.inner.Slices { + b.alloc.releaseChunk(s) + } + + b.alloc = nil } - b.inner.Slices = nil + b.inner.invalidate() +} + +// MakeContiguous ensures the write buffer consists of exactly one contiguous single slice of the provided length +// and returns the slice. +func (b *WriteBuffer) MakeContiguous(length int) []byte { + b.Reset() + + b.mu.Lock() + defer b.mu.Unlock() + + b.alloc = contiguousAllocator + v := b.allocChunk()[0:length] + + b.inner.Slices = [][]byte{v} + + return v } // Reset resets buffer back to empty. @@ -26,11 +54,15 @@ func (b *WriteBuffer) Reset() { b.mu.Lock() defer b.mu.Unlock() - for _, s := range b.inner.Slices { - releaseChunk(s) + if b.alloc != nil { + for _, s := range b.inner.Slices { + b.alloc.releaseChunk(s) + } } - b.inner.Slices = nil + b.inner.invalidate() + + b.inner = Bytes{} } // Write implements io.Writer for appending to the buffer. @@ -40,11 +72,11 @@ func (b *WriteBuffer) Write(data []byte) (n int, err error) { } // AppendSectionTo appends the section of the buffer to the provided slice and returns it. -func (b *WriteBuffer) AppendSectionTo(output []byte, offset, size int) []byte { +func (b *WriteBuffer) AppendSectionTo(w io.Writer, offset, size int) error { b.mu.Lock() defer b.mu.Unlock() - return b.inner.AppendSectionTo(output, offset, size) + return b.inner.AppendSectionTo(w, offset, size) } // Length returns the combined length of all slices. @@ -55,12 +87,12 @@ func (b *WriteBuffer) Length() int { return b.inner.Length() } -// GetBytes appends all bytes to the provided slice and returns it. -func (b *WriteBuffer) GetBytes(output []byte) []byte { +// ToByteSlice appends all bytes to the provided slice and returns it. +func (b *WriteBuffer) ToByteSlice() []byte { b.mu.Lock() defer b.mu.Unlock() - return b.inner.GetBytes(output) + return b.inner.ToByteSlice() } // Bytes returns inner gather.Bytes. @@ -76,8 +108,10 @@ func (b *WriteBuffer) Append(data []byte) { b.mu.Lock() defer b.mu.Unlock() + b.inner.assertValid() + if len(b.inner.Slices) == 0 { - b.inner.sliceBuf[0] = allocChunk() + b.inner.sliceBuf[0] = b.allocChunk() b.inner.Slices = b.inner.sliceBuf[0:1] } @@ -86,7 +120,7 @@ func (b *WriteBuffer) Append(data []byte) { remaining := cap(b.inner.Slices[ndx]) - len(b.inner.Slices[ndx]) if remaining == 0 { - b.inner.Slices = append(b.inner.Slices, allocChunk()) + b.inner.Slices = append(b.inner.Slices, b.allocChunk()) ndx = len(b.inner.Slices) - 1 remaining = cap(b.inner.Slices[ndx]) - len(b.inner.Slices[ndx]) } @@ -101,6 +135,14 @@ func (b *WriteBuffer) Append(data []byte) { } } +func (b *WriteBuffer) allocChunk() []byte { + if b.alloc == nil { + b.alloc = defaultAllocator + } + + return b.alloc.allocChunk() +} + // NewWriteBuffer creates new write buffer. func NewWriteBuffer() *WriteBuffer { return &WriteBuffer{} diff --git a/internal/gather/gather_write_buffer_chunk.go b/internal/gather/gather_write_buffer_chunk.go index 9a4ed28a9..eccfe393d 100644 --- a/internal/gather/gather_write_buffer_chunk.go +++ b/internal/gather/gather_write_buffer_chunk.go @@ -1,42 +1,91 @@ package gather import ( + "context" "sync" -) -const chunkSize = 1 << 20 // 1MB chunks + "github.com/alecthomas/units" +) var ( - freeListMutex sync.Mutex - freeList [][]byte - freeListHighWaterMark int -) - -func allocChunk() []byte { - freeListMutex.Lock() - defer freeListMutex.Unlock() - - l := len(freeList) - if l == 0 { - return make([]byte, 0, chunkSize) + defaultAllocator = &chunkAllocator{ + name: "default", + chunkSize: 1 << 16, // nolint:gomnd + maxFreeListSize: 512, // nolint:gomnd } - ch := freeList[l-1] - freeList = freeList[0 : l-1] + contiguousAllocator = &chunkAllocator{ + name: "contiguous", + chunkSize: 8<<20 + 128, // nolint:gomnd + maxFreeListSize: 2, // nolint:gomnd + } +) + +type chunkAllocator struct { + name string + chunkSize int + + mu sync.Mutex + freeList [][]byte + maxFreeListSize int + freeListHighWaterMark int + allocHighWaterMark int + allocated int + freed int +} + +func (a *chunkAllocator) allocChunk() []byte { + a.mu.Lock() + defer a.mu.Unlock() + + a.allocated++ + + if tot := a.allocated - a.freed; tot > a.allocHighWaterMark { + a.allocHighWaterMark = tot + } + + l := len(a.freeList) + if l == 0 { + return make([]byte, 0, a.chunkSize) + } + + ch := a.freeList[l-1] + a.freeList = a.freeList[0 : l-1] return ch } -func releaseChunk(s []byte) { - if cap(s) != chunkSize { +func (a *chunkAllocator) releaseChunk(s []byte) { + if cap(s) != a.chunkSize { return } - freeListMutex.Lock() - defer freeListMutex.Unlock() + a.mu.Lock() + defer a.mu.Unlock() - freeList = append(freeList, s[:0]) - if len(freeList) > freeListHighWaterMark { - freeListHighWaterMark = len(freeList) + a.freed++ + + if len(a.freeList) < a.maxFreeListSize { + a.freeList = append(a.freeList, s[:0]) + } + + if len(a.freeList) > a.freeListHighWaterMark { + a.freeListHighWaterMark = len(a.freeList) } } + +func (a *chunkAllocator) dumpStats(ctx context.Context, prefix string) { + a.mu.Lock() + defer a.mu.Unlock() + + log(ctx).Infof("%v (%v) - allocated %v chunks freed %v alive %v max %v free list high water mark: %v", + prefix, + units.Base2Bytes(int64(a.chunkSize)), + a.allocated, a.freed, a.allocated-a.freed, a.allocHighWaterMark, a.freeListHighWaterMark) +} + +// DumpStats logs the allocator statistics. +func DumpStats(ctx context.Context) { + defaultAllocator.dumpStats(ctx, "default") + contiguousAllocator.dumpStats(ctx, "contig") +} diff --git a/internal/gather/gather_write_buffer_chunk_test.go b/internal/gather/gather_write_buffer_chunk_test.go index f3c4cec7b..bb00a8069 100644 --- a/internal/gather/gather_write_buffer_chunk_test.go +++ b/internal/gather/gather_write_buffer_chunk_test.go @@ -7,51 +7,54 @@ func TestWriteBufferChunk(t *testing.T) { // reset for testing - freeList = nil - freeListHighWaterMark = 0 + all := &chunkAllocator{ + chunkSize: 100, + maxFreeListSize: 10, + } - chunk1 := allocChunk() + // reset for testing + chunk1 := all.allocChunk() _ = append(chunk1, []byte("chunk1")...) if got, want := len(chunk1), 0; got != want { t.Errorf("invalid chunk len: %v, want %v", got, want) } - if got, want := cap(chunk1), chunkSize; got != want { + if got, want := cap(chunk1), all.chunkSize; got != want { t.Errorf("invalid chunk cap: %v, want %v", got, want) } - if got, want := freeListHighWaterMark, 0; got != want { + if got, want := all.freeListHighWaterMark, 0; got != want { t.Errorf("unexpected high water mark %v, want %v", got, want) } - chunk2 := allocChunk() + chunk2 := all.allocChunk() _ = append(chunk2, []byte("chunk2")...) - if got, want := freeListHighWaterMark, 0; got != want { + if got, want := all.freeListHighWaterMark, 0; got != want { t.Errorf("unexpected high water mark %v, want %v", got, want) } - releaseChunk(chunk2) + all.releaseChunk(chunk2) - if got, want := freeListHighWaterMark, 1; got != want { + if got, want := all.freeListHighWaterMark, 1; got != want { t.Errorf("unexpected high water mark %v, want %v", got, want) } - releaseChunk(chunk1) + all.releaseChunk(chunk1) - if got, want := freeListHighWaterMark, 2; got != want { + if got, want := all.freeListHighWaterMark, 2; got != want { t.Errorf("unexpected high water mark %v, want %v", got, want) } // allocate chunk3 - make sure we got the same slice as chunk1 (LIFO) - chunk3 := allocChunk() + chunk3 := all.allocChunk() if got, want := chunk3[0:6], []byte("chunk1"); !bytes.Equal(got, want) { t.Errorf("got wrong chunk data %q, want %q", string(got), string(want)) } // allocate chunk4 - make sure we got the same slice as chunk1 (LIFO) - chunk4 := allocChunk() + chunk4 := all.allocChunk() if got, want := chunk4[0:6], []byte("chunk2"); !bytes.Equal(got, want) { t.Errorf("got wrong chunk data %q, want %q", string(got), string(want)) } diff --git a/internal/gather/gather_write_buffer_test.go b/internal/gather/gather_write_buffer_test.go index 3254964ef..25d736112 100644 --- a/internal/gather/gather_write_buffer_test.go +++ b/internal/gather/gather_write_buffer_test.go @@ -8,16 +8,19 @@ func TestGatherWriteBuffer(t *testing.T) { // reset for testing - freeList = nil - freeListHighWaterMark = 0 + all := &chunkAllocator{ + chunkSize: 100, + } w := NewWriteBuffer() + w.alloc = all + defer w.Close() w.Append([]byte("hello ")) fmt.Fprintf(w, "world!") - if got, want := w.GetBytes(nil), []byte("hello world!"); !bytes.Equal(got, want) { + if got, want := w.ToByteSlice(), []byte("hello world!"); !bytes.Equal(got, want) { t.Errorf("invaldi bytes %v, want %v", string(got), string(want)) } @@ -25,9 +28,9 @@ func TestGatherWriteBuffer(t *testing.T) { t.Errorf("invalid number of slices %v, want %v", got, want) } - w.Append(bytes.Repeat([]byte("x"), chunkSize)) + w.Append(bytes.Repeat([]byte("x"), all.chunkSize)) - if got, want := w.Length(), chunkSize+12; got != want { + if got, want := w.Length(), all.chunkSize+12; got != want { t.Errorf("invalid length: %v, want %v", got, want) } @@ -37,7 +40,7 @@ func TestGatherWriteBuffer(t *testing.T) { } // write to fill the remainder of 2nd slice - w.Append(bytes.Repeat([]byte("x"), chunkSize-12)) + w.Append(bytes.Repeat([]byte("x"), all.chunkSize-12)) // still 2 slices if got, want := len(w.inner.Slices), 2; got != want { diff --git a/internal/hmac/hmac.go b/internal/hmac/hmac.go index 9157723cc..c021a78b5 100644 --- a/internal/hmac/hmac.go +++ b/internal/hmac/hmac.go @@ -4,41 +4,51 @@ import ( "crypto/hmac" "crypto/sha256" + "io" "github.com/pkg/errors" + + "github.com/kopia/kopia/internal/gather" ) // Append computes HMAC-SHA256 checksum for a given block of bytes and appends it. -func Append(data, secret []byte) []byte { +func Append(input gather.Bytes, secret []byte, output *gather.WriteBuffer) { h := hmac.New(sha256.New, secret) - h.Write(data) - return h.Sum(data) + input.WriteTo(output) // nolint:errcheck + input.WriteTo(h) // nolint:errcheck + + var hash [sha256.Size]byte + + output.Write(h.Sum(hash[:0])) // nolint:errcheck } // VerifyAndStrip verifies that given block of bytes has correct HMAC-SHA256 checksum and strips it. -func VerifyAndStrip(b, secret []byte) ([]byte, error) { - if len(b) < sha256.Size { - return nil, errors.New("invalid data - too short") +func VerifyAndStrip(input gather.Bytes, secret []byte, output *gather.WriteBuffer) error { + if input.Length() < sha256.Size { + return errors.New("invalid data - too short") } - p := len(b) - sha256.Size - data := b[0:p] - signature := b[p:] + p := input.Length() - sha256.Size h := hmac.New(sha256.New, secret) - h.Write(data) + r := input.Reader() - var sigBuf [32]byte + if _, err := io.CopyN(io.MultiWriter(h, output), r, int64(p)); err != nil { + return errors.Wrap(err, "error hashing") + } + + var sigBuf, actualSignature [sha256.Size]byte validSignature := h.Sum(sigBuf[:0]) - if len(signature) != len(validSignature) { - return nil, errors.New("invalid signature length") + n, err := r.Read(actualSignature[:]) + if err != nil || n != sha256.Size { + return errors.Wrap(err, "error reading signature") } - if hmac.Equal(validSignature, signature) { - return data, nil + if hmac.Equal(validSignature, actualSignature[:]) { + return nil } - return nil, errors.New("invalid data - corrupted") + return errors.New("invalid data - corrupted") } diff --git a/internal/iocopy/copy.go b/internal/iocopy/copy.go index 9c21001d5..63278ccec 100644 --- a/internal/iocopy/copy.go +++ b/internal/iocopy/copy.go @@ -8,21 +8,61 @@ const bufSize = 65536 -var bufferPool = sync.Pool{ - New: func() interface{} { - p := make([]byte, bufSize) +var ( + mu sync.Mutex + buffers [][]byte +) - return &p - }, +// GetBuffer allocates new temporary buffer suitable for copying data. +func GetBuffer() []byte { + mu.Lock() + defer mu.Unlock() + + if len(buffers) == 0 { + return make([]byte, bufSize) + } + + var b []byte + + n := len(buffers) - 1 + b, buffers = buffers[n], buffers[0:n] + + return b +} + +// ReleaseBuffer releases the buffer back to the pool. +func ReleaseBuffer(b []byte) { + mu.Lock() + defer mu.Unlock() + + buffers = append(buffers, b) } // Copy is equivalent to io.Copy(). func Copy(dst io.Writer, src io.Reader) (int64, error) { - // nolint:forcetypeassert - bufPtr := bufferPool.Get().(*[]byte) + // If the reader has a WriteTo method, use it to do the copy. + // Avoids an allocation and a copy. + if wt, ok := src.(io.WriterTo); ok { + // nolint:wrapcheck + return wt.WriteTo(dst) + } - defer bufferPool.Put(bufPtr) + // Similarly, if the writer has a ReadFrom method, use it to do the copy. + if rt, ok := dst.(io.ReaderFrom); ok { + // nolint:wrapcheck + return rt.ReadFrom(src) + } + + buf := GetBuffer() + defer ReleaseBuffer(buf) // nolint:wrapcheck - return io.CopyBuffer(dst, src, *bufPtr) + return io.CopyBuffer(dst, src, buf) +} + +// JustCopy is just like Copy() but does not return the number of bytes. +func JustCopy(dst io.Writer, src io.Reader) error { + _, err := Copy(dst, src) + + return err } diff --git a/internal/listcache/listcache.go b/internal/listcache/listcache.go index 7d0ae40b0..09bed05f6 100644 --- a/internal/listcache/listcache.go +++ b/internal/listcache/listcache.go @@ -40,9 +40,12 @@ func (s *listCacheStorage) saveListToCache(ctx context.Context, prefix blob.ID, return } - b := hmac.Append(data, s.hmacSecret) + var b gather.WriteBuffer + defer b.Close() - if err := s.cacheStorage.PutBlob(ctx, prefix, gather.FromSlice(b)); err != nil { + hmac.Append(gather.FromSlice(data), s.hmacSecret, &b) + + if err := s.cacheStorage.PutBlob(ctx, prefix, b.Bytes()); err != nil { log(ctx).Debugf("unable to persist list cache entry: %v", err) } } @@ -50,18 +53,22 @@ func (s *listCacheStorage) saveListToCache(ctx context.Context, prefix blob.ID, func (s *listCacheStorage) readBlobsFromCache(ctx context.Context, prefix blob.ID) *cachedList { cl := &cachedList{} - data, err := s.cacheStorage.GetBlob(ctx, prefix, 0, -1) - if err != nil { + var data gather.WriteBuffer + defer data.Close() + + if err := s.cacheStorage.GetBlob(ctx, prefix, 0, -1, &data); err != nil { return nil } - data, err = hmac.VerifyAndStrip(data, s.hmacSecret) - if err != nil { + var verified gather.WriteBuffer + defer verified.Close() + + if err := hmac.VerifyAndStrip(data.Bytes(), s.hmacSecret, &verified); err != nil { log(ctx).Debugf("warning: invalid list cache HMAC for %v, ignoring", prefix) return nil } - if err := json.Unmarshal(data, &cl); err != nil { + if err := json.NewDecoder(verified.Bytes().Reader()).Decode(&cl); err != nil { log(ctx).Debugf("warning: cant't unmarshal cached list results for %v, ignoring", prefix) return nil } diff --git a/internal/memtrack/memtrack.go b/internal/memtrack/memtrack.go new file mode 100644 index 000000000..45ac1be21 --- /dev/null +++ b/internal/memtrack/memtrack.go @@ -0,0 +1,109 @@ +// Package memtrack implements utility to log memory usage. +package memtrack + +import ( + "context" + "fmt" + "runtime" + "sync" + + "github.com/kopia/kopia/repo/logging" +) + +var log = logging.GetContextLoggerFunc("memtrack") + +type tracker struct { + name string + memoryTrackerMutex sync.Mutex + initialMemStats runtime.MemStats + previousMemStats runtime.MemStats + maxAlloc, maxHeapUsage, maxStackInUse uint64 +} + +func (c *tracker) dump(ctx context.Context, desc string) { + runtime.GC() + + var ms runtime.MemStats + + runtime.ReadMemStats(&ms) + + c.memoryTrackerMutex.Lock() + defer c.memoryTrackerMutex.Unlock() + + if ms.HeapInuse > c.maxHeapUsage { + c.maxHeapUsage = ms.HeapInuse + } + + if ms.StackInuse > c.maxStackInUse { + c.maxStackInUse = ms.StackInuse + } + + if ms.Alloc > c.maxAlloc { + c.maxAlloc = ms.Alloc + } + + log(ctx).Debugf( + "%v: %v allocated %v%v max %v, sys: %v total %v%v, allocs %v%v frees %v%v alive %v%v, goroutines %v", + c.name, + desc, + + ms.Alloc-c.initialMemStats.Alloc, + deltaString(ms.Alloc, c.previousMemStats.Alloc), + c.maxAlloc, + + ms.HeapSys, + + ms.TotalAlloc-c.initialMemStats.TotalAlloc, + deltaString(ms.TotalAlloc, c.previousMemStats.TotalAlloc), + + ms.Mallocs-c.initialMemStats.Mallocs, + deltaString(ms.Mallocs, c.previousMemStats.Mallocs), + + ms.Frees-c.initialMemStats.Frees, + deltaString(ms.Frees, c.previousMemStats.Frees), + + ms.Mallocs-ms.Frees, + deltaString(ms.Mallocs-ms.Frees, c.previousMemStats.Mallocs-c.previousMemStats.Frees), + + runtime.NumGoroutine(), + ) + + c.previousMemStats = ms +} + +type trackerKey struct{} + +// Attach creates a child context with a given tracker attached. +func Attach(ctx context.Context, name string) context.Context { + v := ctx.Value(trackerKey{}) + if v != nil { + name = v.(*tracker).name + "::" + name + } + + t := &tracker{name: name} + runtime.ReadMemStats(&t.initialMemStats) + + return context.WithValue(ctx, trackerKey{}, t) +} + +func deltaString(cur, prev uint64) string { + if cur == prev { + return "" + } + + if cur > prev { + return fmt.Sprintf("(+%v)", cur-prev) + } + + return fmt.Sprintf("(-%v)", prev-cur) +} + +// Dump logs memory usage if the current context is associated with a tracker. +func Dump(ctx context.Context, desc string) { + v := ctx.Value(trackerKey{}) + if v == nil { + return + } + + v.(*tracker).dump(ctx, desc) +} diff --git a/internal/providervalidation/providervalidation.go b/internal/providervalidation/providervalidation.go index 4fbacf95b..f06c152fe 100644 --- a/internal/providervalidation/providervalidation.go +++ b/internal/providervalidation/providervalidation.go @@ -66,13 +66,16 @@ func ValidateProvider(ctx context.Context, st blob.Storage, opt Options) error { log(ctx).Infof("Validating non-existent blob responses") + var out gather.WriteBuffer + defer out.Close() + // read non-existent full blob - if _, err := st.GetBlob(ctx, prefix1+"1", 0, -1); !errors.Is(err, blob.ErrBlobNotFound) { + if err := st.GetBlob(ctx, prefix1+"1", 0, -1, &out); !errors.Is(err, blob.ErrBlobNotFound) { return errors.Errorf("got unexpected error when reading non-existent blob: %v", err) } // read non-existent partial blob - if _, err := st.GetBlob(ctx, prefix1+"1", 0, 5); !errors.Is(err, blob.ErrBlobNotFound) { + if err := st.GetBlob(ctx, prefix1+"1", 0, 5, &out); !errors.Is(err, blob.ErrBlobNotFound) { return errors.Errorf("got unexpected error when reading non-existent partial blob: %v", err) } @@ -118,12 +121,12 @@ func ValidateProvider(ctx context.Context, st blob.Storage, opt Options) error { } for _, tc := range partialBlobCases { - v, err := st.GetBlob(ctx, prefix1+"1", tc.offset, tc.length) + err := st.GetBlob(ctx, prefix1+"1", tc.offset, tc.length, &out) if err != nil { return errors.Wrapf(err, "got unexpected error when reading partial blob @%v+%v", tc.offset, tc.length) } - if got, want := v, blobData[tc.offset:tc.offset+tc.length]; !bytes.Equal(got, want) { + if got, want := out.ToByteSlice(), blobData[tc.offset:tc.offset+tc.length]; !bytes.Equal(got, want) { return errors.Errorf("got unexpected data after reading partial blob @%v+%v: %x, wanted %x", tc.offset, tc.length, got, want) } } @@ -131,12 +134,12 @@ func ValidateProvider(ctx context.Context, st blob.Storage, opt Options) error { log(ctx).Infof("Validating full reads...") // read full blob - v, err := st.GetBlob(ctx, prefix1+"1", 0, -1) + err := st.GetBlob(ctx, prefix1+"1", 0, -1, &out) if err != nil { return errors.Wrap(err, "got unexpected error when reading partial blob") } - if got, want := v, blobData; !bytes.Equal(got, want) { + if got, want := out.ToByteSlice(), blobData; !bytes.Equal(got, want) { return errors.Errorf("got unexpected data after reading partial blob: %x, wanted %x", got, want) } @@ -261,6 +264,9 @@ func (c *concurrencyTest) pickBlob() (blob.ID, []byte, bool) { func (c *concurrencyTest) getBlobWorker(ctx context.Context, worker int) func() error { return func() error { + var out gather.WriteBuffer + defer out.Close() + for clock.Now().Before(c.deadline) { c.randomSleep() @@ -271,7 +277,7 @@ func (c *concurrencyTest) getBlobWorker(ctx context.Context, worker int) func() log(ctx).Debugf("GetBlob worker %v reading %v", worker, blobID) - v, err := c.st.GetBlob(ctx, blobID, 0, -1) + err := c.st.GetBlob(ctx, blobID, 0, -1, &out) if err != nil { if !errors.Is(err, blob.ErrBlobNotFound) || fullyWritten { return errors.Wrapf(err, "unexpected error when reading %v", blobID) @@ -282,7 +288,7 @@ func (c *concurrencyTest) getBlobWorker(ctx context.Context, worker int) func() continue } - if !bytes.Equal(v, blobData) { + if !bytes.Equal(out.ToByteSlice(), blobData) { return errors.Wrapf(err, "invalid data read for %v", blobID) } diff --git a/internal/server/api_repo.go b/internal/server/api_repo.go index eb4da5d16..732e8345b 100644 --- a/internal/server/api_repo.go +++ b/internal/server/api_repo.go @@ -8,6 +8,7 @@ "github.com/pkg/errors" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/passwordpersist" "github.com/kopia/kopia/internal/remoterepoapi" "github.com/kopia/kopia/internal/serverapi" @@ -160,8 +161,10 @@ func (s *Server) handleRepoExists(ctx context.Context, r *http.Request, body []b defer st.Close(ctx) // nolint:errcheck - _, err = st.GetBlob(ctx, repo.FormatBlobID, 0, -1) - if err != nil { + var tmp gather.WriteBuffer + defer tmp.Close() + + if err := st.GetBlob(ctx, repo.FormatBlobID, 0, -1, &tmp); err != nil { if errors.Is(err, blob.ErrBlobNotFound) { return nil, requestError(serverapi.ErrorNotInitialized, "repository not initialized") } diff --git a/internal/testutil/testutil.go b/internal/testutil/testutil.go index 2846f4d0b..509de10c3 100644 --- a/internal/testutil/testutil.go +++ b/internal/testutil/testutil.go @@ -9,8 +9,6 @@ "runtime" "strings" "testing" - - "github.com/kopia/kopia/internal/buf" ) // ProviderTest marks the test method so that it only runs in provider-tests suite. @@ -60,14 +58,6 @@ func ShouldReduceTestComplexity() bool { func MyTestMain(m *testing.M) { v := m.Run() - if ap := buf.ActivePools(); len(ap) != 0 { - for _, v := range ap { - fmt.Fprintf(os.Stderr, "test did not release pool allocated from: %v\n", v) - } - - os.Exit(1) - } - os.Exit(v) } diff --git a/repo/api_server_repository.go b/repo/api_server_repository.go index d28e3fd49..0c9b89a0b 100644 --- a/repo/api_server_repository.go +++ b/repo/api_server_repository.go @@ -13,6 +13,7 @@ "github.com/kopia/kopia/internal/apiclient" "github.com/kopia/kopia/internal/cache" "github.com/kopia/kopia/internal/clock" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/remoterepoapi" "github.com/kopia/kopia/repo/compression" "github.com/kopia/kopia/repo/content" @@ -171,16 +172,26 @@ func (r *apiServerRepository) ContentInfo(ctx context.Context, contentID content } func (r *apiServerRepository) GetContent(ctx context.Context, contentID content.ID) ([]byte, error) { - // nolint:wrapcheck - return r.contentCache.GetOrLoad(ctx, string(contentID), func() ([]byte, error) { + var tmp gather.WriteBuffer + defer tmp.Close() + + err := r.contentCache.GetOrLoad(ctx, string(contentID), func(output *gather.WriteBuffer) error { var result []byte if err := r.cli.Get(ctx, "contents/"+string(contentID), content.ErrContentNotFound, &result); err != nil { - return nil, errors.Wrap(err, "GetContent") + return errors.Wrap(err, "GetContent") } - return result, nil - }) + tmp.Write(result) // nolint:errcheck + + return nil + }, &tmp) + if err != nil { + // nolint:wrapcheck + return nil, err + } + + return tmp.ToByteSlice(), nil } func (r *apiServerRepository) WriteContent(ctx context.Context, data []byte, prefix content.ID, comp compression.HeaderID) (content.ID, error) { @@ -190,7 +201,7 @@ func (r *apiServerRepository) WriteContent(ctx context.Context, data []byte, pre var hashOutput [128]byte - contentID := prefix + content.ID(hex.EncodeToString(r.h(hashOutput[:0], data))) + contentID := prefix + content.ID(hex.EncodeToString(r.h(hashOutput[:0], gather.FromSlice(data)))) // avoid uploading the content body if it already exists. if _, err := r.ContentInfo(ctx, contentID); err == nil { @@ -211,7 +222,7 @@ func (r *apiServerRepository) WriteContent(ctx context.Context, data []byte, pre if prefix != "" { // add all prefixed contents to the cache. - r.contentCache.Put(ctx, string(contentID), data) + r.contentCache.Put(ctx, string(contentID), gather.FromSlice(data)) } return contentID, nil @@ -223,14 +234,6 @@ func (r *apiServerRepository) UpdateDescription(d string) { } func (r *apiServerRepository) Close(ctx context.Context) error { - if r.omgr != nil { - if err := r.omgr.Close(); err != nil { - return errors.Wrap(err, "error closing object manager") - } - - r.omgr = nil - } - if r.isSharedReadOnlySession && r.contentCache != nil { r.contentCache.Close(ctx) r.contentCache = nil diff --git a/repo/blob/azure/azure_storage.go b/repo/blob/azure/azure_storage.go index a7a67b047..d83525cb7 100644 --- a/repo/blob/azure/azure_storage.go +++ b/repo/blob/azure/azure_storage.go @@ -18,6 +18,7 @@ "gocloud.dev/gcerrors" "github.com/kopia/kopia/internal/clock" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/iocopy" "github.com/kopia/kopia/repo/blob" "github.com/kopia/kopia/repo/blob/retrying" @@ -38,35 +39,34 @@ type azStorage struct { uploadThrottler *iothrottler.IOThrottlerPool } -func (az *azStorage) GetBlob(ctx context.Context, b blob.ID, offset, length int64) ([]byte, error) { +func (az *azStorage) GetBlob(ctx context.Context, b blob.ID, offset, length int64, output *gather.WriteBuffer) error { if offset < 0 { - return nil, errors.Wrap(blob.ErrInvalidRange, "invalid offset") + return errors.Wrap(blob.ErrInvalidRange, "invalid offset") } - attempt := func() ([]byte, error) { + attempt := func() error { reader, err := az.bucket.NewRangeReader(ctx, az.getObjectNameString(b), offset, length, nil) if err != nil { - return nil, errors.Wrap(err, "NewRangeReader") + return errors.Wrap(err, "NewRangeReader") } defer reader.Close() //nolint:errcheck throttled, err := az.downloadThrottler.AddReader(reader) if err != nil { - return nil, errors.Wrap(err, "AddReader") + return errors.Wrap(err, "AddReader") } // nolint:wrapcheck - return ioutil.ReadAll(throttled) + return iocopy.JustCopy(output, throttled) } - fetched, err := attempt() - if err != nil { - return nil, translateError(err) + if err := attempt(); err != nil { + return translateError(err) } // nolint:wrapcheck - return blob.EnsureLengthExactly(fetched, length) + return blob.EnsureLengthExactly(output.Length(), length) } func (az *azStorage) GetMetadata(ctx context.Context, b blob.ID) (blob.Metadata, error) { @@ -123,8 +123,7 @@ func (az *azStorage) PutBlob(ctx context.Context, b blob.ID, data blob.Bytes) er return err } - _, err = iocopy.Copy(writer, throttled) - if err != nil { + if err := iocopy.JustCopy(writer, throttled); err != nil { // cancel context before closing the writer causes it to abandon the upload. cancel() diff --git a/repo/blob/azure/azure_storage_test.go b/repo/blob/azure/azure_storage_test.go index c5cc90f05..4f290a62d 100644 --- a/repo/blob/azure/azure_storage_test.go +++ b/repo/blob/azure/azure_storage_test.go @@ -14,6 +14,7 @@ "github.com/kopia/kopia/internal/blobtesting" "github.com/kopia/kopia/internal/clock" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/providervalidation" "github.com/kopia/kopia/internal/testlogging" "github.com/kopia/kopia/internal/testutil" @@ -172,7 +173,10 @@ func TestAzureStorageInvalidBlob(t *testing.T) { defer st.Close(ctx) - _, err = st.GetBlob(ctx, "xxx", 0, 30) + var tmp gather.WriteBuffer + defer tmp.Close() + + err = st.GetBlob(ctx, "xxx", 0, 30, &tmp) if err == nil { t.Errorf("unexpected success when adding to non-existent container") } diff --git a/repo/blob/b2/b2_storage.go b/repo/blob/b2/b2_storage.go index e057696eb..9b0cb7893 100644 --- a/repo/blob/b2/b2_storage.go +++ b/repo/blob/b2/b2_storage.go @@ -13,6 +13,8 @@ "github.com/pkg/errors" backblaze "gopkg.in/kothar/go-backblaze.v0" + "github.com/kopia/kopia/internal/gather" + "github.com/kopia/kopia/internal/iocopy" "github.com/kopia/kopia/repo/blob" "github.com/kopia/kopia/repo/blob/retrying" ) @@ -33,10 +35,16 @@ type b2Storage struct { uploadThrottler *iothrottler.IOThrottlerPool } -func (s *b2Storage) GetBlob(ctx context.Context, id blob.ID, offset, length int64) ([]byte, error) { +func (s *b2Storage) GetBlob(ctx context.Context, id blob.ID, offset, length int64, output *gather.WriteBuffer) error { fileName := s.getObjectNameString(id) - attempt := func() ([]byte, error) { + if offset < 0 { + return blob.ErrInvalidRange + } + + output.Reset() + + attempt := func() error { var fileRange *backblaze.FileRange if length > 0 { @@ -48,34 +56,29 @@ func (s *b2Storage) GetBlob(ctx context.Context, id blob.ID, offset, length int6 _, r, err := s.bucket.DownloadFileRangeByName(fileName, fileRange) if err != nil { - return nil, errors.Wrap(err, "DownloadFileRangeByName") + return errors.Wrap(err, "DownloadFileRangeByName") } defer r.Close() //nolint:errcheck throttled, err := s.downloadThrottler.AddReader(r) if err != nil { - return nil, errors.Wrap(err, "DownloadFileRangeByName") - } - - v, err := ioutil.ReadAll(throttled) - if err != nil { - return nil, errors.Wrap(err, "ReadAll") + return errors.Wrap(err, "DownloadFileRangeByName") } if length == 0 { - return []byte{}, nil + return nil } - return v, nil + // nolint:wrapcheck + return iocopy.JustCopy(output, throttled) } - fetched, err := attempt() - if err != nil { - return nil, translateError(err) + if err := attempt(); err != nil { + return translateError(err) } // nolint:wrapcheck - return blob.EnsureLengthExactly(fetched, length) + return blob.EnsureLengthExactly(output.Length(), length) } func (s *b2Storage) resolveFileID(fileName string) (string, error) { diff --git a/repo/blob/b2/b2_storage_test.go b/repo/blob/b2/b2_storage_test.go index 76499d0e4..11d63ef92 100644 --- a/repo/blob/b2/b2_storage_test.go +++ b/repo/blob/b2/b2_storage_test.go @@ -11,6 +11,7 @@ "github.com/kopia/kopia/internal/blobtesting" "github.com/kopia/kopia/internal/clock" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/providervalidation" "github.com/kopia/kopia/internal/testlogging" "github.com/kopia/kopia/internal/testutil" @@ -102,7 +103,10 @@ func TestB2StorageInvalidBlob(t *testing.T) { defer st.Close(ctx) - _, err = st.GetBlob(ctx, blob.ID(fmt.Sprintf("invalid-blob-%v", clock.Now().UnixNano())), 0, 30) + var tmp gather.WriteBuffer + defer tmp.Close() + + err = st.GetBlob(ctx, blob.ID(fmt.Sprintf("invalid-blob-%v", clock.Now().UnixNano())), 0, 30, &tmp) if err == nil { t.Errorf("unexpected success when requesting non-existing blob") } diff --git a/repo/blob/filesystem/filesystem_storage.go b/repo/blob/filesystem/filesystem_storage.go index b3815be17..8ac2da8dc 100644 --- a/repo/blob/filesystem/filesystem_storage.go +++ b/repo/blob/filesystem/filesystem_storage.go @@ -15,6 +15,8 @@ "github.com/pkg/errors" "github.com/kopia/kopia/internal/clock" + "github.com/kopia/kopia/internal/gather" + "github.com/kopia/kopia/internal/iocopy" "github.com/kopia/kopia/internal/retry" "github.com/kopia/kopia/repo/blob" "github.com/kopia/kopia/repo/blob/sharded" @@ -74,58 +76,59 @@ func isRetriable(err error) bool { return errors.Is(err, errRetriableInvalidLength) } -func (fs *fsImpl) GetBlobFromPath(ctx context.Context, dirPath, path string, offset, length int64) ([]byte, error) { - val, err := retry.WithExponentialBackoff(ctx, "GetBlobFromPath:"+path, func() (interface{}, error) { +func (fs *fsImpl) GetBlobFromPath(ctx context.Context, dirPath, path string, offset, length int64, output *gather.WriteBuffer) error { + err := retry.WithExponentialBackoffNoValue(ctx, "GetBlobFromPath:"+path, func() error { + output.Reset() + f, err := os.Open(path) //nolint:gosec if err != nil { //nolint:wrapcheck - return nil, err + return err } defer f.Close() //nolint:errcheck,gosec if length < 0 { // nolint:wrapcheck - return ioutil.ReadAll(f) + return iocopy.JustCopy(output, f) } if _, err = f.Seek(offset, io.SeekStart); err != nil { // do not wrap seek error, we don't want to retry on it. - return nil, errors.Errorf("seek error: %v", err) + return errors.Errorf("seek error: %v", err) } - b, err := ioutil.ReadAll(io.LimitReader(f, length)) - if err != nil { + if err := iocopy.JustCopy(output, io.LimitReader(f, length)); err != nil { //nolint:wrapcheck - return nil, err + return err } - if int64(len(b)) != length && length > 0 { + if int64(output.Length()) != length && length > 0 { if runtime.GOOS == "darwin" { if st, err := f.Stat(); err == nil && st.Size() == 0 { // this sometimes fails on macOS for unknown reasons, likely a bug in the filesystem // retry deals with this transient state. // see see https://github.com/kopia/kopia/issues/299 - return nil, errRetriableInvalidLength + return errRetriableInvalidLength } } - return nil, errors.Errorf("invalid length") + return errors.Errorf("invalid length") } - return b, nil + return nil }, isRetriable) if err != nil { if os.IsNotExist(err) { - return nil, blob.ErrBlobNotFound + return blob.ErrBlobNotFound } // nolint:wrapcheck - return nil, err + return err } // nolint:wrapcheck - return blob.EnsureLengthExactly(val.([]byte), length) + return blob.EnsureLengthExactly(output.Length(), length) } func (fs *fsImpl) GetMetadataFromPath(ctx context.Context, dirPath, path string) (blob.Metadata, error) { diff --git a/repo/blob/gcs/gcs_storage.go b/repo/blob/gcs/gcs_storage.go index 20761f8ff..b33f9abae 100644 --- a/repo/blob/gcs/gcs_storage.go +++ b/repo/blob/gcs/gcs_storage.go @@ -19,6 +19,7 @@ "google.golang.org/api/option" "github.com/kopia/kopia/internal/clock" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/iocopy" "github.com/kopia/kopia/internal/throttle" "github.com/kopia/kopia/repo/blob" @@ -41,29 +42,28 @@ type gcsStorage struct { uploadThrottler *iothrottler.IOThrottlerPool } -func (gcs *gcsStorage) GetBlob(ctx context.Context, b blob.ID, offset, length int64) ([]byte, error) { +func (gcs *gcsStorage) GetBlob(ctx context.Context, b blob.ID, offset, length int64, output *gather.WriteBuffer) error { if offset < 0 { - return nil, blob.ErrInvalidRange + return blob.ErrInvalidRange } - attempt := func() ([]byte, error) { + attempt := func() error { reader, err := gcs.bucket.Object(gcs.getObjectNameString(b)).NewRangeReader(gcs.ctx, offset, length) if err != nil { - return nil, errors.Wrap(err, "NewRangeReader") + return errors.Wrap(err, "NewRangeReader") } defer reader.Close() //nolint:errcheck // nolint:wrapcheck - return ioutil.ReadAll(reader) + return iocopy.JustCopy(output, reader) } - fetched, err := attempt() - if err != nil { - return nil, translateError(err) + if err := attempt(); err != nil { + return translateError(err) } // nolint:wrapcheck - return blob.EnsureLengthExactly(fetched, length) + return blob.EnsureLengthExactly(output.Length(), length) } func (gcs *gcsStorage) GetMetadata(ctx context.Context, b blob.ID) (blob.Metadata, error) { @@ -106,7 +106,7 @@ func (gcs *gcsStorage) PutBlob(ctx context.Context, b blob.ID, data blob.Bytes) writer.ChunkSize = writerChunkSize writer.ContentType = "application/x-kopia" - _, err := iocopy.Copy(writer, data.Reader()) + err := iocopy.JustCopy(writer, data.Reader()) if err != nil { // cancel context before closing the writer causes it to abandon the upload. cancel() diff --git a/repo/blob/logging/logging_storage.go b/repo/blob/logging/logging_storage.go index b3f5165a2..4d960907f 100644 --- a/repo/blob/logging/logging_storage.go +++ b/repo/blob/logging/logging_storage.go @@ -6,6 +6,7 @@ "time" "github.com/kopia/kopia/internal/clock" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" ) @@ -17,19 +18,19 @@ type loggingStorage struct { prefix string } -func (s *loggingStorage) GetBlob(ctx context.Context, id blob.ID, offset, length int64) ([]byte, error) { +func (s *loggingStorage) GetBlob(ctx context.Context, id blob.ID, offset, length int64, output *gather.WriteBuffer) error { t0 := clock.Now() - result, err := s.base.GetBlob(ctx, id, offset, length) + err := s.base.GetBlob(ctx, id, offset, length, output) dt := clock.Since(t0) - if len(result) < maxLoggedBlobLength { - s.printf(s.prefix+"GetBlob(%q,%v,%v)=(%v, %#v) took %v", id, offset, length, result, err, dt) + if output.Length() < maxLoggedBlobLength { + s.printf(s.prefix+"GetBlob(%q,%v,%v)=(%v, %#v) took %v", id, offset, length, output, err, dt) } else { - s.printf(s.prefix+"GetBlob(%q,%v,%v)=({%v bytes}, %#v) took %v", id, offset, length, len(result), err, dt) + s.printf(s.prefix+"GetBlob(%q,%v,%v)=({%v bytes}, %#v) took %v", id, offset, length, output.Length(), err, dt) } // nolint:wrapcheck - return result, err + return err } func (s *loggingStorage) GetMetadata(ctx context.Context, id blob.ID) (blob.Metadata, error) { diff --git a/repo/blob/rclone/rclone_storage_test.go b/repo/blob/rclone/rclone_storage_test.go index 5ba5d52d1..8cd27e015 100644 --- a/repo/blob/rclone/rclone_storage_test.go +++ b/repo/blob/rclone/rclone_storage_test.go @@ -15,6 +15,7 @@ "github.com/kopia/kopia/internal/blobtesting" "github.com/kopia/kopia/internal/clock" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/testlogging" "github.com/kopia/kopia/internal/testutil" "github.com/kopia/kopia/repo/blob" @@ -77,7 +78,10 @@ func TestRCloneStorage(t *testing.T) { // described in https://github.com/kopia/kopia/issues/624 for i := 0; i < 100; i++ { eg.Go(func() error { - if _, err := st.GetBlob(ctx, blob.ID(uuid.New().String()), 0, -1); !errors.Is(err, blob.ErrBlobNotFound) { + var tmp gather.WriteBuffer + defer tmp.Close() + + if err := st.GetBlob(ctx, blob.ID(uuid.New().String()), 0, -1, &tmp); !errors.Is(err, blob.ErrBlobNotFound) { return errors.Errorf("unexpected error when downloading non-existent blob: %v", err) } diff --git a/repo/blob/readonly/readonly_storage.go b/repo/blob/readonly/readonly_storage.go index 66104129f..d265b1b4d 100644 --- a/repo/blob/readonly/readonly_storage.go +++ b/repo/blob/readonly/readonly_storage.go @@ -7,6 +7,7 @@ "github.com/pkg/errors" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" ) @@ -18,9 +19,9 @@ type readonlyStorage struct { base blob.Storage } -func (s readonlyStorage) GetBlob(ctx context.Context, id blob.ID, offset, length int64) ([]byte, error) { +func (s readonlyStorage) GetBlob(ctx context.Context, id blob.ID, offset, length int64, output *gather.WriteBuffer) error { // nolint:wrapcheck - return s.base.GetBlob(ctx, id, offset, length) + return s.base.GetBlob(ctx, id, offset, length, output) } func (s readonlyStorage) GetMetadata(ctx context.Context, id blob.ID) (blob.Metadata, error) { diff --git a/repo/blob/retrying/retrying_storage.go b/repo/blob/retrying/retrying_storage.go index 69191561e..829c7a183 100644 --- a/repo/blob/retrying/retrying_storage.go +++ b/repo/blob/retrying/retrying_storage.go @@ -7,6 +7,7 @@ "fmt" "time" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/retry" "github.com/kopia/kopia/repo/blob" ) @@ -16,16 +17,14 @@ type retryingStorage struct { blob.Storage } -func (s retryingStorage) GetBlob(ctx context.Context, id blob.ID, offset, length int64) ([]byte, error) { - v, err := retry.WithExponentialBackoff(ctx, fmt.Sprintf("GetBlob(%v,%v,%v)", id, offset, length), func() (interface{}, error) { - // nolint:wrapcheck - return s.Storage.GetBlob(ctx, id, offset, length) - }, isRetriable) - if err != nil { - return nil, err // nolint:wrapcheck - } +func (s retryingStorage) GetBlob(ctx context.Context, id blob.ID, offset, length int64, output *gather.WriteBuffer) error { + // nolint:wrapcheck + return retry.WithExponentialBackoffNoValue(ctx, fmt.Sprintf("GetBlob(%v,%v,%v)", id, offset, length), func() error { + output.Reset() - return v.([]byte), nil + // nolint:wrapcheck + return s.Storage.GetBlob(ctx, id, offset, length, output) + }, isRetriable) } func (s retryingStorage) GetMetadata(ctx context.Context, id blob.ID) (blob.Metadata, error) { diff --git a/repo/blob/retrying/retrying_storage_test.go b/repo/blob/retrying/retrying_storage_test.go index 0a3a53002..72765a10d 100644 --- a/repo/blob/retrying/retrying_storage_test.go +++ b/repo/blob/retrying/retrying_storage_test.go @@ -52,19 +52,22 @@ func TestRetrying(t *testing.T) { require.NoError(t, rs.SetTime(ctx, blobID, clock.Now())) - _, err := rs.GetBlob(ctx, blobID, 0, -1) + var tmp gather.WriteBuffer + defer tmp.Close() + + err := rs.GetBlob(ctx, blobID, 0, -1, &tmp) require.NoError(t, err) _, err = rs.GetMetadata(ctx, blobID) require.NoError(t, err) - if _, err = rs.GetBlob(ctx, blobID, 4, 10000); !errors.Is(err, blob.ErrInvalidRange) { + if err = rs.GetBlob(ctx, blobID, 4, 10000, &tmp); !errors.Is(err, blob.ErrInvalidRange) { t.Fatalf("unexpected error: %v", err) } require.NoError(t, rs.DeleteBlob(ctx, blobID)) - if _, err = rs.GetBlob(ctx, blobID, 0, -1); !errors.Is(err, blob.ErrBlobNotFound) { + if err = rs.GetBlob(ctx, blobID, 0, -1, &tmp); !errors.Is(err, blob.ErrBlobNotFound) { t.Fatalf("unexpected error: %v", err) } diff --git a/repo/blob/s3/s3_storage.go b/repo/blob/s3/s3_storage.go index 3d1688f44..04de84d66 100644 --- a/repo/blob/s3/s3_storage.go +++ b/repo/blob/s3/s3_storage.go @@ -18,6 +18,8 @@ "github.com/minio/minio-go/v7/pkg/credentials" "github.com/pkg/errors" + "github.com/kopia/kopia/internal/gather" + "github.com/kopia/kopia/internal/iocopy" "github.com/kopia/kopia/repo/blob" "github.com/kopia/kopia/repo/blob/retrying" ) @@ -36,47 +38,52 @@ type s3Storage struct { uploadThrottler *iothrottler.IOThrottlerPool } -func (s *s3Storage) GetBlob(ctx context.Context, b blob.ID, offset, length int64) ([]byte, error) { - attempt := func() ([]byte, error) { +func (s *s3Storage) GetBlob(ctx context.Context, b blob.ID, offset, length int64, output *gather.WriteBuffer) error { + output.Reset() + + attempt := func() error { var opt minio.GetObjectOptions if length > 0 { if err := opt.SetRange(offset, offset+length-1); err != nil { - return nil, errors.Wrap(blob.ErrInvalidRange, "unable to set range") + return errors.Wrap(blob.ErrInvalidRange, "unable to set range") + } + } + + if length == 0 { + // zero-length ranges require special handling, set non-zero range and + // we won't be trying to read the response anyway. + if err := opt.SetRange(0, 1); err != nil { + return errors.Wrap(blob.ErrInvalidRange, "unable to set range") } } o, err := s.cli.GetObject(ctx, s.BucketName, s.getObjectNameString(b), opt) if err != nil { - return nil, errors.Wrap(err, "GetObject") + return errors.Wrap(err, "GetObject") } defer o.Close() //nolint:errcheck throttled, err := s.downloadThrottler.AddReader(o) if err != nil { - return nil, errors.Wrap(err, "AddReader") - } - - v, err := ioutil.ReadAll(throttled) - if err != nil { - return nil, errors.Wrap(err, "ReadAll") + return errors.Wrap(err, "AddReader") } if length == 0 { - return []byte{}, nil + return nil } - return v, nil + // nolint:wrapcheck + return iocopy.JustCopy(output, throttled) } - fetched, err := attempt() - if err != nil { - return nil, translateError(err) + if err := attempt(); err != nil { + return translateError(err) } // nolint:wrapcheck - return blob.EnsureLengthExactly(fetched, length) + return blob.EnsureLengthExactly(output.Length(), length) } func translateError(err error) error { diff --git a/repo/blob/sftp/sftp_storage.go b/repo/blob/sftp/sftp_storage.go index 1a0135d88..6e5226972 100644 --- a/repo/blob/sftp/sftp_storage.go +++ b/repo/blob/sftp/sftp_storage.go @@ -20,6 +20,8 @@ "golang.org/x/crypto/ssh" "golang.org/x/crypto/ssh/knownhosts" + "github.com/kopia/kopia/internal/gather" + "github.com/kopia/kopia/internal/iocopy" "github.com/kopia/kopia/internal/retry" "github.com/kopia/kopia/repo/blob" "github.com/kopia/kopia/repo/blob/retrying" @@ -202,53 +204,48 @@ func (s *sftpImpl) closeAllConnections(ctx context.Context) { s.allConn = nil } -func (s *sftpImpl) GetBlobFromPath(ctx context.Context, dirPath, fullPath string, offset, length int64) ([]byte, error) { - v, err := s.usingClient(ctx, "GetBlobFromPath", func(cli *sftp.Client) (interface{}, error) { +func (s *sftpImpl) GetBlobFromPath(ctx context.Context, dirPath, fullPath string, offset, length int64, output *gather.WriteBuffer) error { + return s.usingClientNoResult(ctx, "GetBlobFromPath", func(cli *sftp.Client) error { r, err := cli.Open(fullPath) if isNotExist(err) { - return nil, blob.ErrBlobNotFound + return blob.ErrBlobNotFound } if err != nil { - return nil, errors.Wrapf(err, "unrecognized error when opening SFTP file %v", fullPath) + return errors.Wrapf(err, "unrecognized error when opening SFTP file %v", fullPath) } defer r.Close() //nolint:errcheck if length < 0 { // read entire blob + output.Reset() + // nolint:wrapcheck - return ioutil.ReadAll(r) + return iocopy.JustCopy(output, r) } // parial read, seek to the provided offset and read given number of bytes. if _, err = r.Seek(offset, io.SeekStart); err != nil { - return nil, errors.Wrapf(blob.ErrInvalidRange, "seek error: %v", err) + return errors.Wrapf(blob.ErrInvalidRange, "seek error: %v", err) } - b := make([]byte, length) - - if _, err := r.Read(b); err != nil { + if err := iocopy.JustCopy(output, io.LimitReader(r, length)); err != nil { var se *sftp.StatusError if errors.As(err, &se) { - return nil, blob.ErrInvalidRange + return blob.ErrInvalidRange } if errors.Is(err, io.EOF) { - return nil, blob.ErrInvalidRange + return blob.ErrInvalidRange } - return nil, errors.Wrap(err, "read error") + return errors.Wrap(err, "read error") } // nolint:wrapcheck - return blob.EnsureLengthExactly(b, length) + return blob.EnsureLengthExactly(output.Length(), length) }) - if err != nil { - return nil, err - } - - return v.([]byte), nil } func (s *sftpImpl) GetMetadataFromPath(ctx context.Context, dirPath, fullPath string) (blob.Metadata, error) { diff --git a/repo/blob/sharded/sharded.go b/repo/blob/sharded/sharded.go index 5f99dbeda..f52d9d779 100644 --- a/repo/blob/sharded/sharded.go +++ b/repo/blob/sharded/sharded.go @@ -11,6 +11,7 @@ "github.com/pkg/errors" "golang.org/x/sync/errgroup" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/parallelwork" "github.com/kopia/kopia/repo/blob" ) @@ -19,7 +20,7 @@ // Impl must be implemented by underlying provided. type Impl interface { - GetBlobFromPath(ctx context.Context, dirPath, filePath string, offset, length int64) ([]byte, error) + GetBlobFromPath(ctx context.Context, dirPath, filePath string, offset, length int64, output *gather.WriteBuffer) error GetMetadataFromPath(ctx context.Context, dirPath, filePath string) (blob.Metadata, error) PutBlobInPath(ctx context.Context, dirPath, filePath string, dataSlices blob.Bytes) error SetTimeInPath(ctx context.Context, dirPath, filePath string, t time.Time) error @@ -38,11 +39,11 @@ type Storage struct { } // GetBlob implements blob.Storage. -func (s Storage) GetBlob(ctx context.Context, blobID blob.ID, offset, length int64) ([]byte, error) { +func (s Storage) GetBlob(ctx context.Context, blobID blob.ID, offset, length int64, output *gather.WriteBuffer) error { dirPath, filePath := s.GetShardedPathAndFilePath(blobID) // nolint:wrapcheck - return s.Impl.GetBlobFromPath(ctx, dirPath, filePath, offset, length) + return s.Impl.GetBlobFromPath(ctx, dirPath, filePath, offset, length, output) } func (s Storage) getBlobIDFromFileName(name string) (blob.ID, bool) { diff --git a/repo/blob/storage.go b/repo/blob/storage.go index 609d193c9..3941fe8ef 100644 --- a/repo/blob/storage.go +++ b/repo/blob/storage.go @@ -9,6 +9,8 @@ "github.com/pkg/errors" "golang.org/x/sync/errgroup" + + "github.com/kopia/kopia/internal/gather" ) // ErrSetTimeUnsupported is returned by implementations of Storage that don't support SetTime. @@ -32,7 +34,7 @@ type Reader interface { // If length>0, the the function retrieves a range of bytes [offset,offset+length) // If length<0, the entire blob must be fetched. // Returns ErrInvalidRange if the fetched blob length is invalid. - GetBlob(ctx context.Context, blobID ID, offset, length int64) ([]byte, error) + GetBlob(ctx context.Context, blobID ID, offset, length int64, output *gather.WriteBuffer) error // GetMetadata returns Metadata about single blob. GetMetadata(ctx context.Context, blobID ID) (Metadata, error) @@ -156,31 +158,16 @@ func IterateAllPrefixesInParallel(ctx context.Context, parallelism int, st Stora // EnsureLengthExactly validates that length of the given slice is exactly the provided value. // and returns ErrInvalidRange if the length is of the slice if not. // As a special case length < 0 disables validation. -func EnsureLengthExactly(b []byte, length int64) ([]byte, error) { +func EnsureLengthExactly(gotLength int, length int64) error { if length < 0 { - return b, nil + return nil } - if len(b) != int(length) { - return nil, errors.Wrapf(ErrInvalidRange, "invalid length %v, expected %v", len(b), length) + if gotLength != int(length) { + return errors.Wrapf(ErrInvalidRange, "invalid length %v, expected %v", gotLength, length) } - return b, nil -} - -// EnsureLengthAndTruncate validates that length of the given slice is at least the provided value -// and returns ErrInvalidRange if the length is of the slice if not. -// As a special case length < 0 disables validation. -func EnsureLengthAndTruncate(b []byte, length int64) ([]byte, error) { - if length < 0 { - return b, nil - } - - if len(b) < int(length) { - return nil, errors.Wrapf(ErrInvalidRange, "invalid length %v, expected at least %v", len(b), length) - } - - return b[0:length], nil + return nil } // IDsFromMetadata returns IDs for blobs in Metadata slice. diff --git a/repo/blob/storage_test.go b/repo/blob/storage_test.go index 0f9bac30c..fb12a2f15 100644 --- a/repo/blob/storage_test.go +++ b/repo/blob/storage_test.go @@ -101,39 +101,10 @@ func TestIterateAllPrefixesInParallel(t *testing.T) { } func TestEnsureLengthExactly(t *testing.T) { - v, err := blob.EnsureLengthExactly([]byte{1, 2, 3}, 3) - require.NoError(t, err) - require.Equal(t, []byte{1, 2, 3}, v) - - v, err = blob.EnsureLengthExactly([]byte{1, 2, 3}, -1) - require.NoError(t, err) - require.Equal(t, []byte{1, 2, 3}, v) - - v, err = blob.EnsureLengthExactly([]byte{1, 2, 3}, 4) - require.Error(t, err) - require.Nil(t, v) - - v, err = blob.EnsureLengthExactly([]byte{1, 2, 3}, 2) - require.Error(t, err) - require.Nil(t, v) -} - -func TestEnsureLengthAndTruncate(t *testing.T) { - v, err := blob.EnsureLengthAndTruncate([]byte{1, 2, 3}, 3) - require.NoError(t, err) - require.Equal(t, []byte{1, 2, 3}, v) - - v, err = blob.EnsureLengthAndTruncate([]byte{1, 2, 3}, -1) - require.NoError(t, err) - require.Equal(t, []byte{1, 2, 3}, v) - - v, err = blob.EnsureLengthAndTruncate([]byte{1, 2, 3}, 4) - require.Error(t, err) - require.Nil(t, v) - - v, err = blob.EnsureLengthAndTruncate([]byte{1, 2, 3}, 2) - require.NoError(t, err) - require.Equal(t, []byte{1, 2}, v) + require.NoError(t, blob.EnsureLengthExactly(3, 3)) + require.NoError(t, blob.EnsureLengthExactly(3, -1)) + require.Error(t, blob.EnsureLengthExactly(3, 4)) + require.Error(t, blob.EnsureLengthExactly(3, 2)) } func TestIDsFromMetadata(t *testing.T) { diff --git a/repo/blob/webdav/webdav_storage.go b/repo/blob/webdav/webdav_storage.go index f2b02628b..7a3646d8a 100644 --- a/repo/blob/webdav/webdav_storage.go +++ b/repo/blob/webdav/webdav_storage.go @@ -5,6 +5,7 @@ "bytes" "context" "fmt" + "io" "math/rand" "net/http" "os" @@ -15,6 +16,8 @@ "github.com/pkg/errors" "github.com/studio-b12/gowebdav" + "github.com/kopia/kopia/internal/gather" + "github.com/kopia/kopia/internal/iocopy" "github.com/kopia/kopia/internal/retry" "github.com/kopia/kopia/internal/tlsutil" "github.com/kopia/kopia/repo/blob" @@ -46,18 +49,36 @@ type davStorageImpl struct { cli *gowebdav.Client } -func (d *davStorageImpl) GetBlobFromPath(ctx context.Context, dirPath, path string, offset, length int64) ([]byte, error) { - data, err := d.cli.Read(path) - if err != nil { - return nil, d.translateError(err) +func (d *davStorageImpl) GetBlobFromPath(ctx context.Context, dirPath, path string, offset, length int64, output *gather.WriteBuffer) error { + output.Reset() + + if offset < 0 { + return blob.ErrInvalidRange } - if int(offset) > len(data) || offset < 0 { - return nil, errors.Wrap(blob.ErrInvalidRange, "invalid offset") + s, err := d.cli.ReadStream(path) + if err != nil { + return d.translateError(err) + } + + defer s.Close() // nolint:errcheck + + if length < 0 { + // nolint:wrapcheck + return iocopy.JustCopy(output, s) + } + + // this is horrible, but gowebdav does not support seeking (yet). + if err := iocopy.JustCopy(io.Discard, io.LimitReader(s, offset)); err != nil { + return errors.Wrap(err, "error discarding data from stream") + } + + if err := iocopy.JustCopy(output, io.LimitReader(s, length)); err != nil { + return errors.Wrap(err, "error reading stream") } // nolint:wrapcheck - return blob.EnsureLengthAndTruncate(data[offset:], length) + return blob.EnsureLengthExactly(output.Length(), length) } func (d *davStorageImpl) GetMetadataFromPath(ctx context.Context, dirPath, path string) (blob.Metadata, error) { diff --git a/repo/compression/compressor.go b/repo/compression/compressor.go index 478c48308..1655e5575 100644 --- a/repo/compression/compressor.go +++ b/repo/compression/compressor.go @@ -5,6 +5,7 @@ "bytes" "encoding/binary" "fmt" + "io" "github.com/pkg/errors" ) @@ -17,8 +18,8 @@ // Compressor implements compression and decompression of a byte slice. type Compressor interface { HeaderID() HeaderID - Compress(output *bytes.Buffer, input []byte) error - Decompress(output *bytes.Buffer, input []byte) error + Compress(output io.Writer, input io.Reader) error + Decompress(output io.Writer, input io.Reader, withHeader bool) error } // maps of registered compressors by header ID and name. @@ -50,13 +51,22 @@ func compressionHeader(id HeaderID) []byte { return b } -// IDFromHeader retrieves compression ID from content header. -func IDFromHeader(b []byte) (HeaderID, error) { - if len(b) < compressionHeaderSize { - return 0, errors.Errorf("invalid size: %v", len(b)) +// DecompressByHeader decodes compression header from the provided input and decompresses the remainder. +func DecompressByHeader(output io.Writer, input io.Reader) error { + var b [compressionHeaderSize]byte + + if _, err := io.ReadFull(input, b[:]); err != nil { + return errors.Wrap(err, "error reading compression header") } - return HeaderID(binary.BigEndian.Uint32(b[0:compressionHeaderSize])), nil + compressorID := HeaderID(binary.BigEndian.Uint32(b[0:compressionHeaderSize])) + + compressor := ByHeaderID[compressorID] + if compressor == nil { + return errors.Errorf("unsupported compressor %x", compressorID) + } + + return errors.Wrap(compressor.Decompress(output, input, false), "error decompressing") } func mustSucceed(err error) { @@ -65,16 +75,15 @@ func mustSucceed(err error) { } } -func verifyCompressionHeader(got, want []byte) error { - if !bytes.HasPrefix(got, want) { - var gotHeader []byte - if len(got) >= len(want) { - gotHeader = got[0:len(want)] - } else { - gotHeader = got - } +func verifyCompressionHeader(reader io.Reader, want []byte) error { + var actual [compressionHeaderSize]byte - return errors.Errorf("invalid compression header, expected %x but got %x (len %v)", want, gotHeader, len(got)) + if _, err := io.ReadFull(reader, actual[:]); err != nil { + return errors.Wrap(err, "error reading compression header") + } + + if !bytes.Equal(actual[:], want) { + return errors.Errorf("invalid compression header, expected %x but got %x", want, actual[:]) } return nil diff --git a/repo/compression/compressor_deflate.go b/repo/compression/compressor_deflate.go index 073897031..d54bc2c7a 100644 --- a/repo/compression/compressor_deflate.go +++ b/repo/compression/compressor_deflate.go @@ -1,7 +1,7 @@ package compression import ( - "bytes" + "io" "sync" "github.com/klauspost/compress/flate" @@ -19,7 +19,7 @@ func init() { func newDeflateCompressor(id HeaderID, level int) Compressor { return &deflateCompressor{id, compressionHeader(id), sync.Pool{ New: func() interface{} { - v, err := flate.NewWriter(bytes.NewBuffer(nil), level) + v, err := flate.NewWriter(io.Discard, level) if err != nil { panic("unable to create deflate compressor") } @@ -39,7 +39,7 @@ func (c *deflateCompressor) HeaderID() HeaderID { return c.id } -func (c *deflateCompressor) Compress(output *bytes.Buffer, input []byte) error { +func (c *deflateCompressor) Compress(output io.Writer, input io.Reader) error { if _, err := output.Write(c.header); err != nil { return errors.Wrap(err, "unable to write header") } @@ -50,7 +50,7 @@ func (c *deflateCompressor) Compress(output *bytes.Buffer, input []byte) error { w.Reset(output) - if _, err := w.Write(input); err != nil { + if err := iocopy.JustCopy(w, input); err != nil { return errors.Wrap(err, "compression error") } @@ -61,14 +61,16 @@ func (c *deflateCompressor) Compress(output *bytes.Buffer, input []byte) error { return nil } -func (c *deflateCompressor) Decompress(output *bytes.Buffer, input []byte) error { - if err := verifyCompressionHeader(input, c.header); err != nil { - return err +func (c *deflateCompressor) Decompress(output io.Writer, input io.Reader, withHeader bool) error { + if withHeader { + if err := verifyCompressionHeader(input, c.header); err != nil { + return err + } } - r := flate.NewReader(bytes.NewReader(input[compressionHeaderSize:])) + r := flate.NewReader(input) - if _, err := iocopy.Copy(output, r); err != nil { + if err := iocopy.JustCopy(output, r); err != nil { return errors.Wrap(err, "decompression error") } diff --git a/repo/compression/compressor_gzip.go b/repo/compression/compressor_gzip.go index d41018c6b..56589bea8 100644 --- a/repo/compression/compressor_gzip.go +++ b/repo/compression/compressor_gzip.go @@ -1,8 +1,8 @@ package compression import ( - "bytes" "compress/gzip" + "io" "sync" "github.com/pkg/errors" @@ -19,7 +19,7 @@ func init() { func newGZipCompressor(id HeaderID, level int) Compressor { return &gzipCompressor{id, compressionHeader(id), sync.Pool{ New: func() interface{} { - w, err := gzip.NewWriterLevel(bytes.NewBuffer(nil), level) + w, err := gzip.NewWriterLevel(io.Discard, level) mustSucceed(err) return w }, @@ -36,7 +36,7 @@ func (c *gzipCompressor) HeaderID() HeaderID { return c.id } -func (c *gzipCompressor) Compress(output *bytes.Buffer, input []byte) error { +func (c *gzipCompressor) Compress(output io.Writer, input io.Reader) error { if _, err := output.Write(c.header); err != nil { return errors.Wrap(err, "unable to write header") } @@ -47,7 +47,7 @@ func (c *gzipCompressor) Compress(output *bytes.Buffer, input []byte) error { w.Reset(output) - if _, err := w.Write(input); err != nil { + if err := iocopy.JustCopy(w, input); err != nil { return errors.Wrap(err, "compression error") } @@ -58,18 +58,20 @@ func (c *gzipCompressor) Compress(output *bytes.Buffer, input []byte) error { return nil } -func (c *gzipCompressor) Decompress(output *bytes.Buffer, b []byte) error { - if err := verifyCompressionHeader(b, c.header); err != nil { - return err +func (c *gzipCompressor) Decompress(output io.Writer, input io.Reader, withHeader bool) error { + if withHeader { + if err := verifyCompressionHeader(input, c.header); err != nil { + return err + } } - r, err := gzip.NewReader(bytes.NewReader(b[compressionHeaderSize:])) + r, err := gzip.NewReader(input) if err != nil { return errors.Wrap(err, "unable to open gzip stream") } defer r.Close() //nolint:errcheck - if _, err := iocopy.Copy(output, r); err != nil { + if err := iocopy.JustCopy(output, r); err != nil { return errors.Wrap(err, "decompression error") } diff --git a/repo/compression/compressor_lz4.go b/repo/compression/compressor_lz4.go index 2a382cbfc..5ce929769 100644 --- a/repo/compression/compressor_lz4.go +++ b/repo/compression/compressor_lz4.go @@ -1,7 +1,7 @@ package compression import ( - "bytes" + "io" "sync" "github.com/pierrec/lz4" @@ -17,7 +17,7 @@ func init() { func newLZ4Compressor(id HeaderID) Compressor { return &lz4Compressor{id, compressionHeader(id), sync.Pool{ New: func() interface{} { - return lz4.NewWriter(bytes.NewBuffer(nil)) + return lz4.NewWriter(io.Discard) }, }} } @@ -32,7 +32,7 @@ func (c *lz4Compressor) HeaderID() HeaderID { return c.id } -func (c *lz4Compressor) Compress(output *bytes.Buffer, input []byte) error { +func (c *lz4Compressor) Compress(output io.Writer, input io.Reader) error { if _, err := output.Write(c.header); err != nil { return errors.Wrap(err, "unable to write header") } @@ -43,7 +43,7 @@ func (c *lz4Compressor) Compress(output *bytes.Buffer, input []byte) error { w.Reset(output) - if _, err := w.Write(input); err != nil { + if err := iocopy.JustCopy(w, input); err != nil { return errors.Wrap(err, "compression error") } @@ -54,14 +54,16 @@ func (c *lz4Compressor) Compress(output *bytes.Buffer, input []byte) error { return nil } -func (c *lz4Compressor) Decompress(output *bytes.Buffer, input []byte) error { - if err := verifyCompressionHeader(input, c.header); err != nil { - return err +func (c *lz4Compressor) Decompress(output io.Writer, input io.Reader, withHeader bool) error { + if withHeader { + if err := verifyCompressionHeader(input, c.header); err != nil { + return err + } } - r := lz4.NewReader(bytes.NewReader(input[compressionHeaderSize:])) + r := lz4.NewReader(input) - if _, err := iocopy.Copy(output, r); err != nil { + if err := iocopy.JustCopy(output, r); err != nil { return errors.Wrap(err, "decompression error") } diff --git a/repo/compression/compressor_pgzip.go b/repo/compression/compressor_pgzip.go index b8437e6de..b9ede99b7 100644 --- a/repo/compression/compressor_pgzip.go +++ b/repo/compression/compressor_pgzip.go @@ -2,6 +2,7 @@ import ( "bytes" + "io" "sync" "github.com/klauspost/pgzip" @@ -36,7 +37,7 @@ func (c *pgzipCompressor) HeaderID() HeaderID { return c.id } -func (c *pgzipCompressor) Compress(output *bytes.Buffer, input []byte) error { +func (c *pgzipCompressor) Compress(output io.Writer, input io.Reader) error { if _, err := output.Write(c.header); err != nil { return errors.Wrap(err, "unable to write header") } @@ -47,7 +48,7 @@ func (c *pgzipCompressor) Compress(output *bytes.Buffer, input []byte) error { w.Reset(output) - if _, err := w.Write(input); err != nil { + if err := iocopy.JustCopy(w, input); err != nil { return errors.Wrap(err, "compression error") } @@ -58,18 +59,20 @@ func (c *pgzipCompressor) Compress(output *bytes.Buffer, input []byte) error { return nil } -func (c *pgzipCompressor) Decompress(output *bytes.Buffer, input []byte) error { - if err := verifyCompressionHeader(input, c.header); err != nil { - return err +func (c *pgzipCompressor) Decompress(output io.Writer, input io.Reader, withHeader bool) error { + if withHeader { + if err := verifyCompressionHeader(input, c.header); err != nil { + return err + } } - r, err := pgzip.NewReader(bytes.NewReader(input[compressionHeaderSize:])) + r, err := pgzip.NewReader(input) if err != nil { return errors.Wrap(err, "unable to open gzip stream") } defer r.Close() //nolint:errcheck - if _, err := iocopy.Copy(output, r); err != nil { + if err := iocopy.JustCopy(output, r); err != nil { return errors.Wrap(err, "decompression error") } diff --git a/repo/compression/compressor_s2.go b/repo/compression/compressor_s2.go index 92671ad65..9a87b8c36 100644 --- a/repo/compression/compressor_s2.go +++ b/repo/compression/compressor_s2.go @@ -1,7 +1,7 @@ package compression import ( - "bytes" + "io" "sync" "github.com/klauspost/compress/s2" @@ -25,7 +25,7 @@ func init() { func newS2Compressor(id HeaderID, opts ...s2.WriterOption) Compressor { return &s2Compressor{id, compressionHeader(id), sync.Pool{ New: func() interface{} { - return s2.NewWriter(bytes.NewBuffer(nil), opts...) + return s2.NewWriter(io.Discard, opts...) }, }} } @@ -40,7 +40,7 @@ func (c *s2Compressor) HeaderID() HeaderID { return c.id } -func (c *s2Compressor) Compress(output *bytes.Buffer, input []byte) error { +func (c *s2Compressor) Compress(output io.Writer, input io.Reader) error { if _, err := output.Write(c.header); err != nil { return errors.Wrap(err, "unable to write header") } @@ -51,7 +51,7 @@ func (c *s2Compressor) Compress(output *bytes.Buffer, input []byte) error { w.Reset(output) - if _, err := w.Write(input); err != nil { + if err := iocopy.JustCopy(w, input); err != nil { return errors.Wrap(err, "compression error") } @@ -62,14 +62,16 @@ func (c *s2Compressor) Compress(output *bytes.Buffer, input []byte) error { return nil } -func (c *s2Compressor) Decompress(output *bytes.Buffer, input []byte) error { - if err := verifyCompressionHeader(input, c.header); err != nil { - return err +func (c *s2Compressor) Decompress(output io.Writer, input io.Reader, withHeader bool) error { + if withHeader { + if err := verifyCompressionHeader(input, c.header); err != nil { + return err + } } - r := s2.NewReader(bytes.NewReader(input[compressionHeaderSize:])) + r := s2.NewReader(input) - if _, err := iocopy.Copy(output, r); err != nil { + if err := iocopy.JustCopy(output, r); err != nil { return errors.Wrap(err, "decompression error") } diff --git a/repo/compression/compressor_test.go b/repo/compression/compressor_test.go index 5150b199d..c465fdbdb 100644 --- a/repo/compression/compressor_test.go +++ b/repo/compression/compressor_test.go @@ -22,27 +22,27 @@ func TestCompressor(t *testing.T) { var cData bytes.Buffer - if err := comp.Compress(&cData, data); err != nil { + if err := comp.Compress(&cData, bytes.NewReader(data)); err != nil { t.Fatalf("compression error %v", err) return } if cData.Len() >= len(data) { - t.Errorf("compression not effective for all-zero data") + t.Errorf("compression not effective for all-zero data (len: %v, expected less than %v)", cData.Len(), len(data)) } for id2, comp2 := range ByHeaderID { if id != id2 { var dData bytes.Buffer - if err2 := comp2.Decompress(&dData, cData.Bytes()); err2 == nil { + if err2 := comp2.Decompress(&dData, bytes.NewReader(cData.Bytes()), true); err2 == nil { t.Errorf("compressor %x was able to decompress results of %x", id2, id) } } } var data2 bytes.Buffer - if err := comp.Decompress(&data2, cData.Bytes()); err != nil { + if err := comp.Decompress(&data2, bytes.NewReader(cData.Bytes()), true); err != nil { t.Fatalf("decompression error %v", err) } @@ -60,7 +60,7 @@ func TestCompressor(t *testing.T) { var cData bytes.Buffer - err := comp.Compress(&cData, data) + err := comp.Compress(&cData, bytes.NewReader(data)) if err != nil { t.Fatalf("compression error %v", err) return @@ -71,7 +71,7 @@ func TestCompressor(t *testing.T) { } var data2 bytes.Buffer - if err := comp.Decompress(&data2, cData.Bytes()); err != nil { + if err := comp.Decompress(&data2, &cData, true); err != nil { t.Fatalf("decompression error %v", err) } @@ -134,10 +134,13 @@ func compressionBenchmark(b *testing.B, comp Compressor, input []byte, output *b b.Helper() b.ReportAllocs() + rdr := bytes.NewReader(input) + for i := 0; i < b.N; i++ { output.Reset() + rdr.Reset(input) - if err := comp.Compress(output, input); err != nil { + if err := comp.Compress(output, rdr); err != nil { b.Fatalf("compression error %v", err) return } @@ -148,10 +151,14 @@ func decompressionBenchmark(b *testing.B, comp Compressor, input []byte, output b.Helper() b.ReportAllocs() + rdr := bytes.NewReader(input) + for i := 0; i < b.N; i++ { output.Reset() - if err := comp.Decompress(output, input); err != nil { + rdr.Reset(input) + + if err := comp.Decompress(output, rdr, true); err != nil { b.Fatalf("compression error %v", err) return } diff --git a/repo/compression/compressor_zstd.go b/repo/compression/compressor_zstd.go index 1d801a2c8..96cd59d72 100644 --- a/repo/compression/compressor_zstd.go +++ b/repo/compression/compressor_zstd.go @@ -1,7 +1,7 @@ package compression import ( - "bytes" + "io" "sync" "github.com/klauspost/compress/zstd" @@ -20,7 +20,7 @@ func init() { func newZstdCompressor(id HeaderID, level zstd.EncoderLevel) Compressor { return &zstdCompressor{id, compressionHeader(id), sync.Pool{ New: func() interface{} { - w, err := zstd.NewWriter(bytes.NewBuffer(nil), zstd.WithEncoderLevel(level)) + w, err := zstd.NewWriter(io.Discard, zstd.WithEncoderLevel(level)) mustSucceed(err) return w }, @@ -37,7 +37,7 @@ func (c *zstdCompressor) HeaderID() HeaderID { return c.id } -func (c *zstdCompressor) Compress(output *bytes.Buffer, input []byte) error { +func (c *zstdCompressor) Compress(output io.Writer, input io.Reader) error { if _, err := output.Write(c.header); err != nil { return errors.Wrap(err, "unable to write header") } @@ -48,7 +48,7 @@ func (c *zstdCompressor) Compress(output *bytes.Buffer, input []byte) error { w.Reset(output) - if _, err := w.Write(input); err != nil { + if err := iocopy.JustCopy(w, input); err != nil { return errors.Wrap(err, "compression error") } @@ -59,18 +59,20 @@ func (c *zstdCompressor) Compress(output *bytes.Buffer, input []byte) error { return nil } -func (c *zstdCompressor) Decompress(output *bytes.Buffer, input []byte) error { - if err := verifyCompressionHeader(input, c.header); err != nil { - return err +func (c *zstdCompressor) Decompress(output io.Writer, input io.Reader, withHeader bool) error { + if withHeader { + if err := verifyCompressionHeader(input, c.header); err != nil { + return err + } } - r, err := zstd.NewReader(bytes.NewReader(input[compressionHeaderSize:])) + r, err := zstd.NewReader(input) if err != nil { return errors.Wrap(err, "unable to open zstd stream") } defer r.Close() - if _, err := iocopy.Copy(output, r); err != nil { + if err := iocopy.JustCopy(output, r); err != nil { return errors.Wrap(err, "decompression error") } diff --git a/repo/connect.go b/repo/connect.go index 55dcbd12c..e2a9082c6 100644 --- a/repo/connect.go +++ b/repo/connect.go @@ -7,6 +7,7 @@ "github.com/pkg/errors" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" "github.com/kopia/kopia/repo/content" ) @@ -28,8 +29,10 @@ func Connect(ctx context.Context, configFile string, st blob.Storage, password s opt = &ConnectOptions{} } - formatBytes, err := st.GetBlob(ctx, FormatBlobID, 0, -1) - if err != nil { + var formatBytes gather.WriteBuffer + defer formatBytes.Close() + + if err := st.GetBlob(ctx, FormatBlobID, 0, -1, &formatBytes); err != nil { if errors.Is(err, blob.ErrBlobNotFound) { return ErrRepositoryNotInitialized } @@ -37,7 +40,7 @@ func Connect(ctx context.Context, configFile string, st blob.Storage, password s return errors.Wrap(err, "unable to read format blob") } - f, err := parseFormatBlob(formatBytes) + f, err := parseFormatBlob(formatBytes.ToByteSlice()) if err != nil { return err } diff --git a/repo/content/blob_crypto.go b/repo/content/blob_crypto.go index b496d93ac..95711246e 100644 --- a/repo/content/blob_crypto.go +++ b/repo/content/blob_crypto.go @@ -7,6 +7,7 @@ "github.com/pkg/errors" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" "github.com/kopia/kopia/repo/encryption" "github.com/kopia/kopia/repo/hashing" @@ -49,10 +50,10 @@ func (c *Crypter) getIndexBlobIV(s blob.ID) ([]byte, error) { // EncryptBLOB encrypts the given data using crypter-defined key and returns a name that should // be used to save the blob in thre repository. -func (c *Crypter) EncryptBLOB(data []byte, prefix blob.ID, sessionID SessionID) (blob.ID, []byte, error) { +func (c *Crypter) EncryptBLOB(payload gather.Bytes, prefix blob.ID, sessionID SessionID, output *gather.WriteBuffer) (blob.ID, error) { var hashOutput [hashing.MaxHashSize]byte - hash := c.HashFunction(hashOutput[:0], data) + hash := c.HashFunction(hashOutput[:0], payload) blobID := prefix + blob.ID(hex.EncodeToString(hash)) if sessionID != "" { @@ -61,29 +62,31 @@ func (c *Crypter) EncryptBLOB(data []byte, prefix blob.ID, sessionID SessionID) iv, err := c.getIndexBlobIV(blobID) if err != nil { - return "", nil, err + return "", err } - data2, err := c.Encryptor.Encrypt(nil, data, iv) - if err != nil { - return "", nil, errors.Wrapf(err, "error encrypting BLOB %v", blobID) + output.Reset() + + if err := c.Encryptor.Encrypt(payload, iv, output); err != nil { + return "", errors.Wrapf(err, "error encrypting BLOB %v", blobID) } - return blobID, data2, nil + return blobID, nil } // DecryptBLOB decrypts the provided data using provided blobID to derive initialization vector. -func (c *Crypter) DecryptBLOB(payload []byte, blobID blob.ID) ([]byte, error) { +func (c *Crypter) DecryptBLOB(payload gather.Bytes, blobID blob.ID, output *gather.WriteBuffer) error { iv, err := c.getIndexBlobIV(blobID) if err != nil { - return nil, errors.Wrap(err, "unable to get index blob IV") + return errors.Wrap(err, "unable to get index blob IV") } + output.Reset() + // Decrypt will verify the payload. - payload, err = c.Encryptor.Decrypt(nil, payload, iv) - if err != nil { - return nil, errors.Wrapf(err, "error decrypting BLOB %v", blobID) + if err := c.Encryptor.Decrypt(payload, iv, output); err != nil { + return errors.Wrapf(err, "error decrypting BLOB %v", blobID) } - return payload, nil + return nil } diff --git a/repo/content/builder.go b/repo/content/builder.go index c058c12ec..8b2473fcc 100644 --- a/repo/content/builder.go +++ b/repo/content/builder.go @@ -1,7 +1,6 @@ package content import ( - "bytes" "crypto/rand" "hash/fnv" "io" @@ -10,6 +9,8 @@ "sync" "github.com/pkg/errors" + + "github.com/kopia/kopia/internal/gather" ) const randomSuffixSize = 32 // number of random bytes to append at the end to make the index blob unique @@ -166,36 +167,49 @@ func (b packIndexBuilder) shard(maxShardSize int) []packIndexBuilder { return result } -func (b packIndexBuilder) buildShards(indexVersion int, stable bool, shardSize int) ([][]byte, error) { +func (b packIndexBuilder) buildShards(indexVersion int, stable bool, shardSize int) ([]gather.Bytes, func(), error) { if shardSize == 0 { - return nil, errors.Errorf("invalid shard size") + return nil, nil, errors.Errorf("invalid shard size") } var ( shardedBuilders = b.shard(shardSize) - dataShards [][]byte + dataShardsBuf []*gather.WriteBuffer + dataShards []gather.Bytes randomSuffix [32]byte ) - for _, s := range shardedBuilders { - var buf bytes.Buffer + closeShards := func() { + for _, ds := range dataShardsBuf { + ds.Close() + } + } - if err := s.BuildStable(&buf, indexVersion); err != nil { - return nil, errors.Wrap(err, "error building index shard") + for _, s := range shardedBuilders { + buf := gather.NewWriteBuffer() + + if err := s.BuildStable(buf, indexVersion); err != nil { + closeShards() + + return nil, nil, errors.Wrap(err, "error building index shard") } if !stable { if _, err := rand.Read(randomSuffix[:]); err != nil { - return nil, errors.Wrap(err, "error getting random bytes for suffix") + closeShards() + + return nil, nil, errors.Wrap(err, "error getting random bytes for suffix") } if _, err := buf.Write(randomSuffix[:]); err != nil { - return nil, errors.Wrap(err, "error writing extra random suffix to ensure indexes are always globally unique") + closeShards() + + return nil, nil, errors.Wrap(err, "error writing extra random suffix to ensure indexes are always globally unique") } } dataShards = append(dataShards, buf.Bytes()) } - return dataShards, nil + return dataShards, closeShards, nil } diff --git a/repo/content/committed_content_index.go b/repo/content/committed_content_index.go index 8d0735f5f..8e15071f8 100644 --- a/repo/content/committed_content_index.go +++ b/repo/content/committed_content_index.go @@ -12,6 +12,7 @@ "golang.org/x/sync/errgroup" "github.com/kopia/kopia/internal/clock" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" "github.com/kopia/kopia/repo/logging" ) @@ -35,14 +36,14 @@ type committedContentIndex struct { indexVersion int // fetchOne loads one index blob - fetchOne func(ctx context.Context, blobID blob.ID) ([]byte, error) + fetchOne func(ctx context.Context, blobID blob.ID, output *gather.WriteBuffer) error log logging.Logger } type committedContentIndexCache interface { hasIndexBlobID(ctx context.Context, indexBlob blob.ID) (bool, error) - addContentToCache(ctx context.Context, indexBlob blob.ID, data []byte) error + addContentToCache(ctx context.Context, indexBlob blob.ID, data gather.Bytes) error openIndex(ctx context.Context, indexBlob blob.ID) (packIndex, error) expireUnused(ctx context.Context, used []blob.ID) error } @@ -79,7 +80,7 @@ func (c *committedContentIndex) shouldIgnore(id Info) bool { return !id.Timestamp().After(c.deletionWatermark) } -func (c *committedContentIndex) addIndexBlob(ctx context.Context, indexBlobID blob.ID, data []byte, use bool) error { +func (c *committedContentIndex) addIndexBlob(ctx context.Context, indexBlobID blob.ID, data gather.Bytes, use bool) error { // ensure we bump revision number AFTER this function // doing it prematurely might confuse callers of revision() who may cache // a set of old contents and associate it with new revision, before new contents @@ -273,13 +274,17 @@ func (c *committedContentIndex) fetchIndexBlobs(ctx context.Context, indexBlobs eg, ctx := errgroup.WithContext(ctx) for i := 0; i < parallelFetches; i++ { eg.Go(func() error { + var data gather.WriteBuffer + defer data.Close() + for indexBlobID := range ch { - data, err := c.fetchOne(ctx, indexBlobID) - if err != nil { + data.Reset() + + if err := c.fetchOne(ctx, indexBlobID, &data); err != nil { return errors.Wrapf(err, "error loading index blob %v", indexBlobID) } - if err := c.addIndexBlob(ctx, indexBlobID, data, false); err != nil { + if err := c.addIndexBlob(ctx, indexBlobID, data.Bytes(), false); err != nil { return errors.Wrap(err, "unable to add to committed content cache") } } @@ -318,7 +323,7 @@ func (c *committedContentIndex) missingIndexBlobs(ctx context.Context, blobs []b func newCommittedContentIndex(caching *CachingOptions, v1PerContentOverhead uint32, indexVersion int, - fetchOne func(ctx context.Context, blobID blob.ID) ([]byte, error), + fetchOne func(ctx context.Context, blobID blob.ID, output *gather.WriteBuffer) error, baseLog logging.Logger, ) *committedContentIndex { log := logging.WithPrefix("[committed-content-index] ", baseLog) diff --git a/repo/content/committed_content_index_cache_test.go b/repo/content/committed_content_index_cache_test.go index 62d6060f1..eb2ccb06f 100644 --- a/repo/content/committed_content_index_cache_test.go +++ b/repo/content/committed_content_index_cache_test.go @@ -8,6 +8,7 @@ "github.com/stretchr/testify/require" "github.com/kopia/kopia/internal/faketime" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/testlogging" "github.com/kopia/kopia/internal/testutil" "github.com/kopia/kopia/repo/blob" @@ -113,7 +114,7 @@ func testCache(t *testing.T, cache committedContentIndexCache, fakeTime *faketim } } -func mustBuildPackIndex(t *testing.T, b packIndexBuilder) []byte { +func mustBuildPackIndex(t *testing.T, b packIndexBuilder) gather.Bytes { t.Helper() var buf bytes.Buffer @@ -121,5 +122,5 @@ func mustBuildPackIndex(t *testing.T, b packIndexBuilder) []byte { t.Fatal(err) } - return buf.Bytes() + return gather.FromSlice(buf.Bytes()) } diff --git a/repo/content/committed_content_index_disk_cache.go b/repo/content/committed_content_index_disk_cache.go index 7bf4f24c8..df82023bc 100644 --- a/repo/content/committed_content_index_disk_cache.go +++ b/repo/content/committed_content_index_disk_cache.go @@ -12,6 +12,7 @@ "golang.org/x/exp/mmap" "github.com/kopia/kopia/internal/cache" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" "github.com/kopia/kopia/repo/logging" ) @@ -80,7 +81,7 @@ func (c *diskCommittedContentIndexCache) hasIndexBlobID(ctx context.Context, ind return false, errors.Wrapf(err, "error checking %v", indexBlobID) } -func (c *diskCommittedContentIndexCache) addContentToCache(ctx context.Context, indexBlobID blob.ID, data []byte) error { +func (c *diskCommittedContentIndexCache) addContentToCache(ctx context.Context, indexBlobID blob.ID, data gather.Bytes) error { exists, err := c.hasIndexBlobID(ctx, indexBlobID) if err != nil { return err @@ -90,7 +91,7 @@ func (c *diskCommittedContentIndexCache) addContentToCache(ctx context.Context, return nil } - tmpFile, err := writeTempFileAtomic(c.dirname, data) + tmpFile, err := writeTempFileAtomic(c.dirname, data.ToByteSlice()) if err != nil { return err } diff --git a/repo/content/committed_content_index_mem_cache.go b/repo/content/committed_content_index_mem_cache.go index d4680a200..c10577da3 100644 --- a/repo/content/committed_content_index_mem_cache.go +++ b/repo/content/committed_content_index_mem_cache.go @@ -7,6 +7,7 @@ "github.com/pkg/errors" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" ) @@ -23,11 +24,11 @@ func (m *memoryCommittedContentIndexCache) hasIndexBlobID(ctx context.Context, i return m.contents[indexBlobID] != nil, nil } -func (m *memoryCommittedContentIndexCache) addContentToCache(ctx context.Context, indexBlobID blob.ID, data []byte) error { +func (m *memoryCommittedContentIndexCache) addContentToCache(ctx context.Context, indexBlobID blob.ID, data gather.Bytes) error { m.mu.Lock() defer m.mu.Unlock() - ndx, err := openPackIndex(bytes.NewReader(data), m.v1PerContentOverhead) + ndx, err := openPackIndex(bytes.NewReader(data.ToByteSlice()), m.v1PerContentOverhead) if err != nil { return err } diff --git a/repo/content/committed_read_manager.go b/repo/content/committed_read_manager.go index 6319054cf..3344f41f5 100644 --- a/repo/content/committed_read_manager.go +++ b/repo/content/committed_read_manager.go @@ -1,7 +1,6 @@ package content import ( - "bytes" "context" "os" "path/filepath" @@ -11,10 +10,10 @@ "github.com/pkg/errors" - "github.com/kopia/kopia/internal/buf" "github.com/kopia/kopia/internal/cache" "github.com/kopia/kopia/internal/clock" "github.com/kopia/kopia/internal/epoch" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/listcache" "github.com/kopia/kopia/internal/ownwrites" "github.com/kopia/kopia/repo/blob" @@ -52,7 +51,7 @@ // indexBlobManager is the API of index blob manager as used by content manager. type indexBlobManager interface { - writeIndexBlobs(ctx context.Context, data [][]byte, sessionID SessionID) ([]blob.Metadata, error) + writeIndexBlobs(ctx context.Context, data []gather.Bytes, sessionID SessionID) ([]blob.Metadata, error) listActiveIndexBlobs(ctx context.Context) ([]IndexBlobInfo, time.Time, error) compact(ctx context.Context, opts CompactOptions) error flushCache(ctx context.Context) @@ -95,7 +94,6 @@ type SharedManager struct { repositoryFormatBytes []byte indexVersion int indexShardSize int - encryptionBufferPool *buf.Pool // logger where logs should be written log logging.Logger @@ -112,59 +110,65 @@ func (sm *SharedManager) Crypter() *Crypter { return sm.crypter } -func (sm *SharedManager) readPackFileLocalIndex(ctx context.Context, packFile blob.ID, packFileLength int64) ([]byte, error) { +func (sm *SharedManager) readPackFileLocalIndex(ctx context.Context, packFile blob.ID, packFileLength int64, output *gather.WriteBuffer) error { + var err error + if packFileLength >= indexRecoverPostambleSize { - data, err := sm.attemptReadPackFileLocalIndex(ctx, packFile, packFileLength-indexRecoverPostambleSize, indexRecoverPostambleSize) - if err == nil { - sm.log.Debugf("recovered %v index bytes from blob %v using optimized method", len(data), packFile) - return data, nil + if err = sm.attemptReadPackFileLocalIndex(ctx, packFile, packFileLength-indexRecoverPostambleSize, indexRecoverPostambleSize, output); err == nil { + sm.log.Debugf("recovered %v index bytes from blob %v using optimized method", output.Length(), packFile) + return nil } sm.log.Debugf("unable to recover using optimized method: %v", err) } - data, err := sm.attemptReadPackFileLocalIndex(ctx, packFile, 0, -1) - if err == nil { - sm.log.Debugf("recovered %v index bytes from blob %v using full blob read", len(data), packFile) - return data, nil + if err = sm.attemptReadPackFileLocalIndex(ctx, packFile, 0, -1, output); err == nil { + sm.log.Debugf("recovered %v index bytes from blob %v using full blob read", output.Length(), packFile) + + return nil } - return nil, err + return err } -func (sm *SharedManager) attemptReadPackFileLocalIndex(ctx context.Context, packFile blob.ID, offset, length int64) ([]byte, error) { - payload, err := sm.st.GetBlob(ctx, packFile, offset, length) +func (sm *SharedManager) attemptReadPackFileLocalIndex(ctx context.Context, packFile blob.ID, offset, length int64, output *gather.WriteBuffer) error { + var payload gather.WriteBuffer + defer payload.Close() + + output.Reset() + + err := sm.st.GetBlob(ctx, packFile, offset, length, &payload) if err != nil { - return nil, errors.Wrapf(err, "error getting blob %v", packFile) + return errors.Wrapf(err, "error getting blob %v", packFile) } - postamble := findPostamble(payload) + postamble := findPostamble(payload.Bytes().ToByteSlice()) if postamble == nil { - return nil, errors.Errorf("unable to find valid postamble in file %v", packFile) + return errors.Errorf("unable to find valid postamble in file %v", packFile) } if uint32(offset) > postamble.localIndexOffset { - return nil, errors.Errorf("not enough data read during optimized attempt %v", packFile) + return errors.Errorf("not enough data read during optimized attempt %v", packFile) } postamble.localIndexOffset -= uint32(offset) - if uint64(postamble.localIndexOffset+postamble.localIndexLength) > uint64(len(payload)) { + if uint64(postamble.localIndexOffset+postamble.localIndexLength) > uint64(payload.Length()) { // invalid offset/length - return nil, errors.Errorf("unable to find valid local index in file %v - invalid offset/length", packFile) + return errors.Errorf("unable to find valid local index in file %v - invalid offset/length", packFile) } - encryptedLocalIndexBytes := payload[postamble.localIndexOffset : postamble.localIndexOffset+postamble.localIndexLength] - if encryptedLocalIndexBytes == nil { - return nil, errors.Errorf("unable to find valid local index in file %v", packFile) + var encryptedLocalIndexBytes gather.WriteBuffer + defer encryptedLocalIndexBytes.Close() + + if err := payload.AppendSectionTo(&encryptedLocalIndexBytes, int(postamble.localIndexOffset), int(postamble.localIndexLength)); err != nil { + // should never happen + return errors.Wrap(err, "error appending to local index bytes") } - localIndexBytes, err := sm.decryptAndVerify(encryptedLocalIndexBytes, postamble.localIndexIV) - if err != nil { - return nil, errors.Wrap(err, "unable to decrypt local index") - } - - return localIndexBytes, nil + return errors.Wrap( + sm.decryptAndVerify(encryptedLocalIndexBytes.Bytes(), postamble.localIndexIV, output), + "unable to decrypt local index") } func (sm *SharedManager) loadPackIndexesLocked(ctx context.Context) error { @@ -225,56 +229,58 @@ func (sm *SharedManager) getCacheForContentID(id ID) contentCache { return sm.contentCache } -func (sm *SharedManager) decryptContentAndVerify(payload []byte, bi Info) ([]byte, error) { - sm.Stats.readContent(len(payload)) +func (sm *SharedManager) decryptContentAndVerify(payload gather.Bytes, bi Info, output *gather.WriteBuffer) error { + sm.Stats.readContent(payload.Length()) var hashBuf [hashing.MaxHashSize]byte iv, err := getPackedContentIV(hashBuf[:], bi.GetContentID()) if err != nil { - return nil, err + return err } // reserved for future use if k := bi.GetEncryptionKeyID(); k != 0 { - return nil, errors.Errorf("unsupported encryption key ID: %v", k) + return errors.Errorf("unsupported encryption key ID: %v", k) } - decrypted, err := sm.decryptAndVerify(payload, iv) - if err != nil { - return nil, errors.Wrapf(err, "invalid checksum at %v offset %v length %v", bi.GetPackBlobID(), bi.GetPackOffset(), len(payload)) + h := bi.GetCompressionHeaderID() + if h == 0 { + return errors.Wrapf( + sm.decryptAndVerify(payload, iv, output), + "invalid checksum at %v offset %v length %v/%v", bi.GetPackBlobID(), bi.GetPackOffset(), bi.GetPackedLength(), payload.Length()) } - if h := bi.GetCompressionHeaderID(); h != 0 { - c := compression.ByHeaderID[h] - if c == nil { - return nil, errors.Errorf("unsupported compressor %x", h) - } + var tmp gather.WriteBuffer + defer tmp.Close() - out := bytes.NewBuffer(nil) - - if err := c.Decompress(out, decrypted); err != nil { - return nil, errors.Wrap(err, "error decompressing") - } - - return out.Bytes(), nil + if err := sm.decryptAndVerify(payload, iv, &tmp); err != nil { + return errors.Wrapf(err, "invalid checksum at %v offset %v length %v/%v", bi.GetPackBlobID(), bi.GetPackOffset(), bi.GetPackedLength(), payload.Length()) } - return decrypted, nil + c := compression.ByHeaderID[h] + if c == nil { + return errors.Errorf("unsupported compressor %x", h) + } + + if err := c.Decompress(output, tmp.Bytes().Reader(), true); err != nil { + return errors.Wrap(err, "error decompressing") + } + + return nil } -func (sm *SharedManager) decryptAndVerify(encrypted, iv []byte) ([]byte, error) { - decrypted, err := sm.crypter.Encryptor.Decrypt(nil, encrypted, iv) - if err != nil { +func (sm *SharedManager) decryptAndVerify(encrypted gather.Bytes, iv []byte, output *gather.WriteBuffer) error { + if err := sm.crypter.Encryptor.Decrypt(encrypted, iv, output); err != nil { sm.Stats.foundInvalidContent() - return nil, errors.Wrap(err, "decrypt") + return errors.Wrap(err, "decrypt") } sm.Stats.foundValidContent() - sm.Stats.decrypted(len(decrypted)) + sm.Stats.decrypted(output.Length()) // already verified - return decrypted, nil + return nil } // IndexBlobs returns the list of active index blobs. @@ -464,7 +470,6 @@ func (sm *SharedManager) release(ctx context.Context) error { sm.contentCache.close(ctx) sm.metadataCache.close(ctx) - sm.encryptionBufferPool.Close() if sm.internalLogger != nil { sm.internalLogger.Close(ctx) @@ -556,7 +561,6 @@ func NewSharedManager(ctx context.Context, st blob.Storage, f *FormattingOptions repositoryFormatBytes: opts.RepositoryFormatBytes, checkInvariantsOnUnlock: os.Getenv("KOPIA_VERIFY_INVARIANTS") != "", writeFormatVersion: int32(f.Version), - encryptionBufferPool: buf.NewPool(ctx, defaultEncryptionBufferPoolSegmentSize+crypter.Encryptor.Overhead()+maxCompressionOverheadPerContent, "content-manager-encryption"), indexVersion: actualIndexVersion, indexShardSize: defaultIndexShardSize, internalLogManager: ilm, diff --git a/repo/content/content_cache.go b/repo/content/content_cache.go index 6df89d12f..8f12e09ad 100644 --- a/repo/content/content_cache.go +++ b/repo/content/content_cache.go @@ -3,6 +3,7 @@ import ( "context" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" ) @@ -10,5 +11,5 @@ type contentCache interface { close(ctx context.Context) - getContent(ctx context.Context, cacheKey cacheKey, blobID blob.ID, offset, length int64) ([]byte, error) + getContent(ctx context.Context, cacheKey cacheKey, blobID blob.ID, offset, length int64, output *gather.WriteBuffer) error } diff --git a/repo/content/content_cache_data.go b/repo/content/content_cache_data.go index f666eef3d..d52df8456 100644 --- a/repo/content/content_cache_data.go +++ b/repo/content/content_cache_data.go @@ -6,6 +6,7 @@ "github.com/pkg/errors" "github.com/kopia/kopia/internal/cache" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" ) @@ -24,14 +25,14 @@ func adjustCacheKey(cacheKey cacheKey) cacheKey { return cacheKey } -func (c *contentCacheForData) getContent(ctx context.Context, cacheKey cacheKey, blobID blob.ID, offset, length int64) ([]byte, error) { +func (c *contentCacheForData) getContent(ctx context.Context, cacheKey cacheKey, blobID blob.ID, offset, length int64, output *gather.WriteBuffer) error { cacheKey = adjustCacheKey(cacheKey) // nolint:wrapcheck - return c.pc.GetOrLoad(ctx, string(cacheKey), func() ([]byte, error) { + return c.pc.GetOrLoad(ctx, string(cacheKey), func(output *gather.WriteBuffer) error { // nolint:wrapcheck - return c.st.GetBlob(ctx, blobID, offset, length) - }) + return c.st.GetBlob(ctx, blobID, offset, length, output) + }, output) } func (c *contentCacheForData) close(ctx context.Context) { diff --git a/repo/content/content_cache_metadata.go b/repo/content/content_cache_metadata.go index aff09bddc..2fc149e3a 100644 --- a/repo/content/content_cache_metadata.go +++ b/repo/content/content_cache_metadata.go @@ -11,6 +11,7 @@ "golang.org/x/sync/errgroup" "github.com/kopia/kopia/internal/cache" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" ) @@ -41,8 +42,10 @@ func (c *contentCacheForMetadata) sync(ctx context.Context) error { <-sem }() - _, err := c.getContent(ctx, "dummy", bm.BlobID, 0, 1) - return err + var tmp gather.WriteBuffer + defer tmp.Close() + + return c.getContent(ctx, "dummy", bm.BlobID, 0, 1, &tmp) }) return nil @@ -62,48 +65,60 @@ func (c *contentCacheForMetadata) mutexForBlob(blobID blob.ID) *sync.Mutex { return &c.shardedMutexes[mutexID] } -func (c *contentCacheForMetadata) getContent(ctx context.Context, cacheKey cacheKey, blobID blob.ID, offset, length int64) ([]byte, error) { +func (c *contentCacheForMetadata) getContent(ctx context.Context, cacheKey cacheKey, blobID blob.ID, offset, length int64, output *gather.WriteBuffer) error { // try getting from cache first - if v := c.pc.Get(ctx, string(blobID), offset, length); v != nil { - return v, nil + if c.pc.Get(ctx, string(blobID), offset, length, output) { + return nil } m := c.mutexForBlob(blobID) m.Lock() defer m.Unlock() + var blobData gather.WriteBuffer + defer blobData.Close() + // read the entire blob - blobData, err := c.st.GetBlob(ctx, blobID, 0, -1) + err := c.st.GetBlob(ctx, blobID, 0, -1, &blobData) if err != nil { stats.Record(ctx, cache.MetricMissErrors.M(1)) } else { - stats.Record(ctx, cache.MetricMissBytes.M(int64(len(blobData)))) + stats.Record(ctx, cache.MetricMissBytes.M(int64(blobData.Length()))) } if errors.Is(err, blob.ErrBlobNotFound) { // not found in underlying storage // nolint:wrapcheck - return nil, err + return err } if err != nil { // nolint:wrapcheck - return nil, err + return err } // store the whole blob in the cache. - c.pc.Put(ctx, string(blobID), blobData) + c.pc.Put(ctx, string(blobID), blobData.Bytes()) if offset == 0 && length == -1 { - return blobData, nil + _, err := blobData.Bytes().WriteTo(output) + + return errors.Wrap(err, "error copying results") } - if offset < 0 || offset+length > int64(len(blobData)) { - return nil, errors.Errorf("invalid (offset=%v,length=%v) for blob %q of size %v", offset, length, blobID, len(blobData)) + if offset < 0 || offset+length > int64(blobData.Length()) { + return errors.Errorf("invalid (offset=%v,length=%v) for blob %q of size %v", offset, length, blobID, blobData.Length()) } - return blobData[offset : offset+length], nil + output.Reset() + + if err := blobData.AppendSectionTo(output, int(offset), int(length)); err != nil { + // should never happen + return errors.Wrap(err, "error appending to result") + } + + return nil } func (c *contentCacheForMetadata) close(ctx context.Context) { diff --git a/repo/content/content_cache_passthrough.go b/repo/content/content_cache_passthrough.go index 021e11b84..f7a694ba5 100644 --- a/repo/content/content_cache_passthrough.go +++ b/repo/content/content_cache_passthrough.go @@ -3,6 +3,7 @@ import ( "context" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" ) @@ -13,7 +14,7 @@ type passthroughContentCache struct { func (c passthroughContentCache) close(ctx context.Context) {} -func (c passthroughContentCache) getContent(ctx context.Context, cacheKey cacheKey, blobID blob.ID, offset, length int64) ([]byte, error) { +func (c passthroughContentCache) getContent(ctx context.Context, cacheKey cacheKey, blobID blob.ID, offset, length int64, output *gather.WriteBuffer) error { // nolint:wrapcheck - return c.st.GetBlob(ctx, blobID, offset, length) + return c.st.GetBlob(ctx, blobID, offset, length, output) } diff --git a/repo/content/content_cache_test.go b/repo/content/content_cache_test.go index 86411e2bd..48986902d 100644 --- a/repo/content/content_cache_test.go +++ b/repo/content/content_cache_test.go @@ -69,13 +69,16 @@ func TestCacheExpiration(t *testing.T) { defer cc.close(ctx) - _, err = cc.getContent(ctx, "00000a", "content-4k", 0, -1) // 4k + var tmp gather.WriteBuffer + defer tmp.Close() + + err = cc.getContent(ctx, "00000a", "content-4k", 0, -1, &tmp) // 4k require.NoError(t, err) - _, err = cc.getContent(ctx, "00000b", "content-4k", 0, -1) // 4k + err = cc.getContent(ctx, "00000b", "content-4k", 0, -1, &tmp) // 4k require.NoError(t, err) - _, err = cc.getContent(ctx, "00000c", "content-4k", 0, -1) // 4k + err = cc.getContent(ctx, "00000c", "content-4k", 0, -1, &tmp) // 4k require.NoError(t, err) - _, err = cc.getContent(ctx, "00000d", "content-4k", 0, -1) // 4k + err = cc.getContent(ctx, "00000d", "content-4k", 0, -1, &tmp) // 4k require.NoError(t, err) // wait for a sweep @@ -97,7 +100,7 @@ func TestCacheExpiration(t *testing.T) { } for _, tc := range cases { - _, got := cc.getContent(ctx, cacheKey(tc.blobID), "content-4k", 0, -1) + got := cc.getContent(ctx, cacheKey(tc.blobID), "content-4k", 0, -1, &tmp) if want := tc.expectedError; !errors.Is(got, want) { t.Errorf("unexpected error when getting content %v: %v wanted %v", tc.blobID, got, want) } else { @@ -154,15 +157,18 @@ func verifyContentCache(t *testing.T, cc contentCache, cacheStorage blob.Storage {"xf0f0f6", "content-1", -1, 5, nil, errors.Errorf("invalid offset: -1: invalid blob offset or length")}, } + var v gather.WriteBuffer + defer v.Close() + for _, tc := range cases { - v, err := cc.getContent(ctx, tc.cacheKey, tc.blobID, tc.offset, tc.length) + err := cc.getContent(ctx, tc.cacheKey, tc.blobID, tc.offset, tc.length, &v) if (err != nil) != (tc.err != nil) { t.Errorf("unexpected error for %v: %+v, wanted %+v", tc.cacheKey, err, tc.err) } else if err != nil && err.Error() != tc.err.Error() { t.Errorf("unexpected error for %v: %q, wanted %q", tc.cacheKey, err.Error(), tc.err.Error()) } - if !bytes.Equal(v, tc.expected) { - t.Errorf("unexpected data for %v: %x, wanted %x", tc.cacheKey, v, tc.expected) + if got := v.ToByteSlice(); !bytes.Equal(got, tc.expected) { + t.Errorf("unexpected data for %v: %x, wanted %x", tc.cacheKey, got, tc.expected) } } @@ -172,20 +178,23 @@ func verifyContentCache(t *testing.T, cc contentCache, cacheStorage blob.Storage t.Run("DataCorruption", func(t *testing.T) { const cacheKey = "f0f0f1x" - d, err := cacheStorage.GetBlob(ctx, cacheKey, 0, -1) - require.NoError(t, err) + var tmp gather.WriteBuffer + defer tmp.Close() + + require.NoError(t, cacheStorage.GetBlob(ctx, cacheKey, 0, -1, &tmp)) // corrupt the data and write back - d[0] ^= 1 + b := tmp.Bytes() + b.Slices[0][0] ^= 1 - require.NoError(t, cacheStorage.PutBlob(ctx, cacheKey, gather.FromSlice(d))) + require.NoError(t, cacheStorage.PutBlob(ctx, cacheKey, b)) - v, err := cc.getContent(ctx, "xf0f0f1", "content-1", 1, 5) + err := cc.getContent(ctx, "xf0f0f1", "content-1", 1, 5, &tmp) if err != nil { t.Fatalf("error in getContent: %v", err) } - if got, want := v, []byte{2, 3, 4, 5, 6}; !reflect.DeepEqual(v, want) { + if got, want := tmp.ToByteSlice(), []byte{2, 3, 4, 5, 6}; !reflect.DeepEqual(got, want) { t.Errorf("invalid result when reading corrupted data: %v, wanted %v", got, want) } }) @@ -248,12 +257,14 @@ func TestCacheFailureToWrite(t *testing.T) { }, } - v, err := cc.getContent(ctx, "aa", "content-1", 0, 3) - if err != nil { + var v gather.WriteBuffer + defer v.Close() + + if err = cc.getContent(ctx, "aa", "content-1", 0, 3, &v); err != nil { t.Errorf("write failure wasn't ignored: %v", err) } - if got, want := v, []byte{1, 2, 3}; !reflect.DeepEqual(got, want) { + if got, want := v.ToByteSlice(), []byte{1, 2, 3}; !reflect.DeepEqual(got, want) { t.Errorf("unexpected value retrieved from cache: %v, want: %v", got, want) } @@ -292,13 +303,13 @@ func TestCacheFailureToRead(t *testing.T) { }, } - for i := 0; i < 2; i++ { - v, err := cc.getContent(ctx, "aa", "content-1", 0, 3) - if err != nil { - t.Errorf("read failure wasn't ignored: %v", err) - } + var v gather.WriteBuffer + defer v.Close() - if got, want := v, []byte{1, 2, 3}; !reflect.DeepEqual(got, want) { + for i := 0; i < 2; i++ { + require.NoError(t, cc.getContent(ctx, "aa", "content-1", 0, 3, &v)) + + if got, want := v.ToByteSlice(), []byte{1, 2, 3}; !reflect.DeepEqual(got, want) { t.Errorf("unexpected value retrieved from cache: %v, want: %v", got, want) } } diff --git a/repo/content/content_formatter_test.go b/repo/content/content_formatter_test.go index 2f17d331a..745b9eb95 100644 --- a/repo/content/content_formatter_test.go +++ b/repo/content/content_formatter_test.go @@ -9,7 +9,10 @@ "testing" "time" + "github.com/stretchr/testify/require" + "github.com/kopia/kopia/internal/blobtesting" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/testlogging" "github.com/kopia/kopia/repo/blob" "github.com/kopia/kopia/repo/encryption" @@ -66,19 +69,19 @@ func TestFormatters(t *testing.T) { return } - contentID := cr.HashFunction(nil, data) + contentID := cr.HashFunction(nil, gather.FromSlice(data)) - cipherText, err := cr.Encryptor.Encrypt(nil, data, contentID) - if err != nil || cipherText == nil { - t.Errorf("invalid response from Encrypt: %v %v", cipherText, err) - } + var cipherText gather.WriteBuffer + defer cipherText.Close() - plainText, err := cr.Encryptor.Decrypt(nil, cipherText, contentID) - if err != nil || plainText == nil { - t.Errorf("invalid response from Decrypt: %v %v", plainText, err) - } + require.NoError(t, cr.Encryptor.Encrypt(gather.FromSlice(data), contentID, &cipherText)) - h1 := sha1.Sum(plainText) + var plainText gather.WriteBuffer + defer plainText.Close() + + require.NoError(t, cr.Encryptor.Decrypt(cipherText.Bytes(), contentID, &plainText)) + + h1 := sha1.Sum(plainText.ToByteSlice()) if !bytes.Equal(h0[:], h1[:]) { t.Errorf("Encrypt()/Decrypt() does not round-trip: %x %x", h0, h1) @@ -91,9 +94,8 @@ func TestFormatters(t *testing.T) { } } +// nolint:thelper func verifyEndToEndFormatter(ctx context.Context, t *testing.T, hashAlgo, encryptionAlgo string) { - t.Helper() - data := blobtesting.DataMap{} keyTime := map[blob.ID]time.Time{} st := blobtesting.NewMapStorage(data, keyTime, nil) @@ -132,7 +134,7 @@ func verifyEndToEndFormatter(ctx context.Context, t *testing.T, hashAlgo, encryp b2, err := bm.GetContent(ctx, contentID) if err != nil { - t.Errorf("unable to read content %q: %v", contentID, err) + t.Fatalf("unable to read content %q: %v", contentID, err) return } @@ -147,7 +149,7 @@ func verifyEndToEndFormatter(ctx context.Context, t *testing.T, hashAlgo, encryp b3, err := bm.GetContent(ctx, contentID) if err != nil { - t.Errorf("unable to read content after flush %q: %v", contentID, err) + t.Fatalf("unable to read content after flush %q: %v", contentID, err) return } diff --git a/repo/content/content_index_recovery.go b/repo/content/content_index_recovery.go index 51cfee6ab..686baddc7 100644 --- a/repo/content/content_index_recovery.go +++ b/repo/content/content_index_recovery.go @@ -1,7 +1,6 @@ package content import ( - "bytes" "context" "encoding/binary" "hash/crc32" @@ -15,12 +14,14 @@ // RecoverIndexFromPackBlob attempts to recover index blob entries from a given pack file. // Pack file length may be provided (if known) to reduce the number of bytes that are read from the storage. func (bm *WriteManager) RecoverIndexFromPackBlob(ctx context.Context, packFile blob.ID, packFileLength int64, commit bool) ([]Info, error) { - localIndexBytes, err := bm.readPackFileLocalIndex(ctx, packFile, packFileLength) - if err != nil { + var localIndexBytes gather.WriteBuffer + defer localIndexBytes.Close() + + if err := bm.readPackFileLocalIndex(ctx, packFile, packFileLength, &localIndexBytes); err != nil { return nil, err } - ndx, err := openPackIndex(bytes.NewReader(localIndexBytes), uint32(bm.crypter.Encryptor.Overhead())) + ndx, err := openPackIndex(localIndexBytes.Bytes(), uint32(bm.crypter.Encryptor.Overhead())) if err != nil { return nil, errors.Errorf("unable to open index in file %v", packFile) } @@ -160,46 +161,51 @@ func decodePostamble(payload []byte) *packContentPostamble { } } -func (sm *SharedManager) buildLocalIndex(pending packIndexBuilder) ([]byte, error) { - var buf bytes.Buffer - if err := pending.Build(&buf, sm.indexVersion); err != nil { - return nil, errors.Wrap(err, "unable to build local index") +func (sm *SharedManager) buildLocalIndex(pending packIndexBuilder, output *gather.WriteBuffer) error { + if err := pending.Build(output, sm.indexVersion); err != nil { + return errors.Wrap(err, "unable to build local index") } - return buf.Bytes(), nil + return nil } // writePackFileIndexRecoveryData appends data designed to help with recovery of pack index in case it gets damaged or lost. -func (sm *SharedManager) writePackFileIndexRecoveryData(buf *gather.WriteBuffer, pending packIndexBuilder) error { +func (sm *SharedManager) writePackFileIndexRecoveryData(pending packIndexBuilder, output *gather.WriteBuffer) error { // build, encrypt and append local index - localIndexOffset := buf.Length() + localIndexOffset := output.Length() - localIndex, err := sm.buildLocalIndex(pending) - if err != nil { + var localIndex gather.WriteBuffer + defer localIndex.Close() + + if err := sm.buildLocalIndex(pending, &localIndex); err != nil { return err } - localIndexIV := sm.hashData(nil, localIndex) + localIndexIV := sm.hashData(nil, localIndex.Bytes()) - encryptedLocalIndex, err := sm.crypter.Encryptor.Encrypt(nil, localIndex, localIndexIV) - if err != nil { + var encryptedLocalIndex gather.WriteBuffer + defer encryptedLocalIndex.Close() + + if err := sm.crypter.Encryptor.Encrypt(localIndex.Bytes(), localIndexIV, &encryptedLocalIndex); err != nil { return errors.Wrap(err, "encryption error") } postamble := packContentPostamble{ localIndexIV: localIndexIV, localIndexOffset: uint32(localIndexOffset), - localIndexLength: uint32(len(encryptedLocalIndex)), + localIndexLength: uint32(encryptedLocalIndex.Length()), } - buf.Append(encryptedLocalIndex) + if _, err := encryptedLocalIndex.Bytes().WriteTo(output); err != nil { + return errors.Wrap(err, "error copying encrypted index to buffer") + } postambleBytes, err := postamble.toBytes() if err != nil { return err } - buf.Append(postambleBytes) + output.Append(postambleBytes) return nil } diff --git a/repo/content/content_manager.go b/repo/content/content_manager.go index ac8b61946..d1f721226 100644 --- a/repo/content/content_manager.go +++ b/repo/content/content_manager.go @@ -31,8 +31,6 @@ FormatLogModule = "kopia/format" - defaultEncryptionBufferPoolSegmentSize = 8 << 20 // 8 MB - packBlobIDLength = 16 defaultIndexShardSize = 16e6 // slightly less than 2^24, which lets index use 24-bit/3-byte indexes @@ -219,7 +217,7 @@ func (bm *WriteManager) maybeRetryWritingFailedPacksUnlocked(ctx context.Context return nil } -func (bm *WriteManager) addToPackUnlocked(ctx context.Context, contentID ID, data []byte, isDeleted bool, comp compression.HeaderID) error { +func (bm *WriteManager) addToPackUnlocked(ctx context.Context, contentID ID, data gather.Bytes, isDeleted bool, comp compression.HeaderID) error { // see if the current index is old enough to cause automatic flush. if err := bm.maybeFlushBasedOnTimeUnlocked(ctx); err != nil { return errors.Wrap(err, "unable to flush old pending writes") @@ -265,10 +263,10 @@ func (bm *WriteManager) addToPackUnlocked(ctx context.Context, contentID ID, dat PackOffset: uint32(pp.currentPackData.Length()), TimestampSeconds: bm.timeNow().Unix(), FormatVersion: byte(bm.writeFormatVersion), - OriginalLength: uint32(len(data)), + OriginalLength: uint32(data.Length()), } - actualComp, err := bm.maybeCompressAndEncryptDataForPacking(pp.currentPackData, data, contentID, comp) + actualComp, err := bm.maybeCompressAndEncryptDataForPacking(data, contentID, comp, pp.currentPackData) if err != nil { return errors.Wrapf(err, "unable to encrypt %q", contentID) } @@ -369,11 +367,13 @@ func (bm *WriteManager) flushPackIndexesLocked(ctx context.Context) error { } if len(bm.packIndexBuilder) > 0 { - dataShards, err := bm.packIndexBuilder.buildShards(bm.indexVersion, true, bm.indexShardSize) + dataShards, closeShards, err := bm.packIndexBuilder.buildShards(bm.indexVersion, true, bm.indexShardSize) if err != nil { return errors.Wrap(err, "unable to build pack index") } + defer closeShards() + // we must hold a lock between writing an index and adding index blob to committed contents index // otherwise it is possible for concurrent compaction or refresh to forget about the blob we have just // written @@ -393,7 +393,7 @@ func (bm *WriteManager) flushPackIndexesLocked(ctx context.Context) error { // and will be visible to others, including blob GC. for i, indexBlobMD := range indexBlobMDs { - bm.onUpload(int64(len(dataShards[i]))) + bm.onUpload(int64(dataShards[i].Length())) if err := bm.committedContents.addIndexBlob(ctx, indexBlobMD.BlobID, dataShards[i], true); err != nil { return errors.Wrap(err, "unable to add committed content") @@ -556,12 +556,14 @@ func (bm *WriteManager) RewriteContent(ctx context.Context, contentID ID) error return err } - data, err := bm.getContentDataUnlocked(ctx, pp, bi) - if err != nil { + var data gather.WriteBuffer + defer data.Close() + + if err := bm.getContentDataUnlocked(ctx, pp, bi, &data); err != nil { return err } - return bm.addToPackUnlocked(ctx, contentID, data, bi.GetDeleted(), bi.GetCompressionHeaderID()) + return bm.addToPackUnlocked(ctx, contentID, data.Bytes(), bi.GetDeleted(), bi.GetCompressionHeaderID()) } // UndeleteContent rewrites the content with the given ID if the content exists @@ -579,12 +581,14 @@ func (bm *WriteManager) UndeleteContent(ctx context.Context, contentID ID) error return nil } - data, err := bm.getContentDataUnlocked(ctx, pp, bi) - if err != nil { + var data gather.WriteBuffer + defer data.Close() + + if err := bm.getContentDataUnlocked(ctx, pp, bi, &data); err != nil { return err } - return bm.addToPackUnlocked(ctx, contentID, data, false, bi.GetCompressionHeaderID()) + return bm.addToPackUnlocked(ctx, contentID, data.Bytes(), false, bi.GetCompressionHeaderID()) } func packPrefixForContentID(contentID ID) blob.ID { @@ -652,7 +656,7 @@ func (bm *WriteManager) WriteContent(ctx context.Context, data []byte, prefix ID var hashOutput [hashing.MaxHashSize]byte - contentID := prefix + ID(hex.EncodeToString(bm.hashData(hashOutput[:0], data))) + contentID := prefix + ID(hex.EncodeToString(bm.hashData(hashOutput[:0], gather.FromSlice(data)))) // content already tracked if _, bi, err := bm.getContentInfo(ctx, contentID); err == nil { @@ -666,7 +670,7 @@ func (bm *WriteManager) WriteContent(ctx context.Context, data []byte, prefix ID bm.log.Debugf("write-content %v new", contentID) } - err := bm.addToPackUnlocked(ctx, contentID, data, false, comp) + err := bm.addToPackUnlocked(ctx, contentID, gather.FromSlice(data), false, comp) return contentID, err } @@ -692,8 +696,15 @@ func (bm *WriteManager) GetContent(ctx context.Context, contentID ID) (v []byte, return nil, err } + var tmp gather.WriteBuffer + defer tmp.Close() + // Return content even if it is bi.GetDeleted() so it can be recovered during GC among others. - return bm.getContentDataUnlocked(ctx, pp, bi) + if err := bm.getContentDataUnlocked(ctx, pp, bi, &tmp); err != nil { + return nil, err + } + + return tmp.ToByteSlice(), nil } func (bm *WriteManager) getOverlayContentInfo(contentID ID) (*pendingPackInfo, Info, bool) { diff --git a/repo/content/content_manager_indexes.go b/repo/content/content_manager_indexes.go index ab68f7de5..602fa22b7 100644 --- a/repo/content/content_manager_indexes.go +++ b/repo/content/content_manager_indexes.go @@ -1,13 +1,13 @@ package content import ( - "bytes" "context" "time" "github.com/pkg/errors" "github.com/kopia/kopia/internal/clock" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" ) @@ -67,13 +67,15 @@ func (sm *SharedManager) CompactIndexes(ctx context.Context, opt CompactOptions) } // ParseIndexBlob loads entries in a given index blob and returns them. -func ParseIndexBlob(ctx context.Context, blobID blob.ID, data []byte, crypter *Crypter) ([]Info, error) { - data, err := crypter.DecryptBLOB(data, blobID) - if err != nil { +func ParseIndexBlob(ctx context.Context, blobID blob.ID, encrypted gather.Bytes, crypter *Crypter) ([]Info, error) { + var data gather.WriteBuffer + defer data.Close() + + if err := crypter.DecryptBLOB(encrypted, blobID, &data); err != nil { return nil, errors.Wrap(err, "unable to decrypt index blob") } - index, err := openPackIndex(bytes.NewReader(data), uint32(crypter.Encryptor.Overhead())) + index, err := openPackIndex(data.Bytes(), uint32(crypter.Encryptor.Overhead())) if err != nil { return nil, errors.Wrapf(err, "unable to open index blob") } diff --git a/repo/content/content_manager_lock_free.go b/repo/content/content_manager_lock_free.go index 24448b205..06ebb1ba3 100644 --- a/repo/content/content_manager_lock_free.go +++ b/repo/content/content_manager_lock_free.go @@ -1,7 +1,6 @@ package content import ( - "bytes" "context" "crypto/aes" cryptorand "crypto/rand" @@ -17,14 +16,9 @@ "github.com/kopia/kopia/repo/hashing" ) -// maxCompressionOverheadPerContent is the maximum amount of overhead any compressor -// would need for non-compressible data of maximum size. -// The consequences of getting this wrong are not fatal - just an unnecessary memory allocation. -const maxCompressionOverheadPerContent = 16384 - const indexBlobCompactionWarningThreshold = 1000 -func (sm *SharedManager) maybeCompressAndEncryptDataForPacking(output *gather.WriteBuffer, data []byte, contentID ID, comp compression.HeaderID) (compression.HeaderID, error) { +func (sm *SharedManager) maybeCompressAndEncryptDataForPacking(data gather.Bytes, contentID ID, comp compression.HeaderID, output *gather.WriteBuffer) (compression.HeaderID, error) { var hashOutput [hashing.MaxHashSize]byte iv, err := getPackedContentIV(hashOutput[:], contentID) @@ -38,39 +32,32 @@ func (sm *SharedManager) maybeCompressAndEncryptDataForPacking(output *gather.Wr return NoCompression, errors.Errorf("compression is not enabled for this repository.") } - // allocate temporary buffer to hold the compressed bytes. - tmp := sm.encryptionBufferPool.Allocate(len(data) + maxCompressionOverheadPerContent) - defer tmp.Release() + var tmp gather.WriteBuffer + defer tmp.Close() + // allocate temporary buffer to hold the compressed bytes. c := compression.ByHeaderID[comp] if c == nil { return NoCompression, errors.Errorf("unsupported compressor %x", comp) } - cbuf := bytes.NewBuffer(tmp.Data[:0]) - if err = c.Compress(cbuf, data); err != nil { + if err = c.Compress(&tmp, data.Reader()); err != nil { return NoCompression, errors.Wrap(err, "compression error") } - if cd := cbuf.Bytes(); len(cd) >= len(data) { + if cd := tmp.Length(); cd >= data.Length() { // data was not compressible enough. comp = NoCompression } else { - data = cd + data = tmp.Bytes() } } - b := sm.encryptionBufferPool.Allocate(len(data) + sm.crypter.Encryptor.Overhead()) - defer b.Release() - - cipherText, err := sm.crypter.Encryptor.Encrypt(b.Data[:0], data, iv) - if err != nil { + if err := sm.crypter.Encryptor.Encrypt(data, iv, output); err != nil { return NoCompression, errors.Wrap(err, "unable to encrypt") } - sm.Stats.encrypted(len(data)) - - output.Append(cipherText) + sm.Stats.encrypted(data.Length()) return comp, nil } @@ -101,22 +88,21 @@ func ValidatePrefix(prefix ID) error { return errors.Errorf("invalid prefix, must be a empty or single letter between 'g' and 'z'") } -func (bm *WriteManager) getContentDataUnlocked(ctx context.Context, pp *pendingPackInfo, bi Info) ([]byte, error) { - var payload []byte +func (bm *WriteManager) getContentDataUnlocked(ctx context.Context, pp *pendingPackInfo, bi Info, output *gather.WriteBuffer) error { + var payload gather.WriteBuffer + defer payload.Close() if pp != nil && pp.packBlobID == bi.GetPackBlobID() { // we need to use a lock here in case somebody else writes to the pack at the same time. - payload = pp.currentPackData.AppendSectionTo(nil, int(bi.GetPackOffset()), int(bi.GetPackedLength())) - } else { - var err error - - payload, err = bm.getCacheForContentID(bi.GetContentID()).getContent(ctx, cacheKey(bi.GetContentID()), bi.GetPackBlobID(), int64(bi.GetPackOffset()), int64(bi.GetPackedLength())) - if err != nil { - return nil, errors.Wrap(err, "getCacheForContentID") + if err := pp.currentPackData.AppendSectionTo(&payload, int(bi.GetPackOffset()), int(bi.GetPackedLength())); err != nil { + // should never happen + return errors.Wrap(err, "error appending pending content data to buffer") } + } else if err := bm.getCacheForContentID(bi.GetContentID()).getContent(ctx, cacheKey(bi.GetContentID()), bi.GetPackBlobID(), int64(bi.GetPackOffset()), int64(bi.GetPackedLength()), &payload); err != nil { + return errors.Wrap(err, "error getting cached content") } - return bm.decryptContentAndVerify(payload, bi) + return bm.decryptContentAndVerify(payload.Bytes(), bi, output) } func (bm *WriteManager) preparePackDataContent(pp *pendingPackInfo) (packIndexBuilder, error) { @@ -157,7 +143,7 @@ func (bm *WriteManager) preparePackDataContent(pp *pendingPackInfo) (packIndexBu } } - err := bm.writePackFileIndexRecoveryData(pp.currentPackData, packFileIndex) + err := bm.writePackFileIndexRecoveryData(packFileIndex, pp.currentPackData) return packFileIndex, err } @@ -178,10 +164,10 @@ func (bm *WriteManager) writePackFileNotLocked(ctx context.Context, packFile blo return errors.Wrap(bm.st.PutBlob(ctx, packFile, data), "error writing pack file") } -func (sm *SharedManager) hashData(output, data []byte) []byte { +func (sm *SharedManager) hashData(output []byte, data gather.Bytes) []byte { // Hash the content and compute encryption key. contentID := sm.crypter.HashFunction(output, data) - sm.Stats.hashedContent(len(data)) + sm.Stats.hashedContent(data.Length()) return contentID } @@ -198,9 +184,12 @@ func CreateCrypter(f *FormattingOptions) (*Crypter, error) { return nil, errors.Wrap(err, "unable to create encryptor") } - contentID := h(nil, nil) + contentID := h(nil, gather.FromSlice(nil)) - _, err = e.Encrypt(nil, nil, contentID) + var tmp gather.WriteBuffer + defer tmp.Close() + + err = e.Encrypt(gather.FromSlice(nil), contentID, &tmp) if err != nil { return nil, errors.Wrap(err, "invalid encryptor") } diff --git a/repo/content/content_manager_test.go b/repo/content/content_manager_test.go index 5e05d52c7..50f87fa11 100644 --- a/repo/content/content_manager_test.go +++ b/repo/content/content_manager_test.go @@ -1979,7 +1979,7 @@ func verifyContentManagerDataSet(ctx context.Context, t *testing.T, mgr *WriteMa for contentID, originalPayload := range dataSet { v, err := mgr.GetContent(ctx, contentID) if err != nil { - t.Errorf("unable to read content %q: %v", contentID, err) + t.Fatalf("unable to read content %q: %v", contentID, err) continue } diff --git a/repo/content/encrypted_blob_mgr.go b/repo/content/encrypted_blob_mgr.go index 2d1a10c68..027f962f3 100644 --- a/repo/content/encrypted_blob_mgr.go +++ b/repo/content/encrypted_blob_mgr.go @@ -17,22 +17,27 @@ type encryptedBlobMgr struct { log logging.Logger } -func (m *encryptedBlobMgr) getEncryptedBlob(ctx context.Context, blobID blob.ID) ([]byte, error) { - payload, err := m.indexBlobCache.getContent(ctx, cacheKey(blobID), blobID, 0, -1) - if err != nil { - return nil, errors.Wrap(err, "getContent") +func (m *encryptedBlobMgr) getEncryptedBlob(ctx context.Context, blobID blob.ID, output *gather.WriteBuffer) error { + var payload gather.WriteBuffer + defer payload.Close() + + if err := m.indexBlobCache.getContent(ctx, cacheKey(blobID), blobID, 0, -1, &payload); err != nil { + return errors.Wrap(err, "getContent") } - return m.crypter.DecryptBLOB(payload, blobID) + return m.crypter.DecryptBLOB(payload.Bytes(), blobID, output) } -func (m *encryptedBlobMgr) encryptAndWriteBlob(ctx context.Context, data []byte, prefix blob.ID, sessionID SessionID) (blob.Metadata, error) { - blobID, data2, err := m.crypter.EncryptBLOB(data, prefix, sessionID) +func (m *encryptedBlobMgr) encryptAndWriteBlob(ctx context.Context, data gather.Bytes, prefix blob.ID, sessionID SessionID) (blob.Metadata, error) { + var data2 gather.WriteBuffer + defer data2.Close() + + blobID, err := m.crypter.EncryptBLOB(data, prefix, sessionID, &data2) if err != nil { return blob.Metadata{}, errors.Wrap(err, "error encrypting") } - err = m.st.PutBlob(ctx, blobID, gather.FromSlice(data2)) + err = m.st.PutBlob(ctx, blobID, data2.Bytes()) if err != nil { m.log.Debugf("write-index-blob %v failed %v", blobID, err) return blob.Metadata{}, errors.Wrapf(err, "error writing blob %v", blobID) diff --git a/repo/content/index_blob_manager_v0.go b/repo/content/index_blob_manager_v0.go index 8fe29e1d0..ef951b42c 100644 --- a/repo/content/index_blob_manager_v0.go +++ b/repo/content/index_blob_manager_v0.go @@ -9,6 +9,7 @@ "github.com/pkg/errors" "golang.org/x/sync/errgroup" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" "github.com/kopia/kopia/repo/logging" ) @@ -143,7 +144,7 @@ func (m *indexBlobManagerV0) registerCompaction(ctx context.Context, inputs, out return errors.Wrap(err, "unable to marshal log entry bytes") } - compactionLogBlobMetadata, err := m.enc.encryptAndWriteBlob(ctx, logEntryBytes, compactionLogBlobPrefix, "") + compactionLogBlobMetadata, err := m.enc.encryptAndWriteBlob(ctx, gather.FromSlice(logEntryBytes), compactionLogBlobPrefix, "") if err != nil { return errors.Wrap(err, "unable to write compaction log") } @@ -165,11 +166,11 @@ func (m *indexBlobManagerV0) registerCompaction(ctx context.Context, inputs, out return nil } -func (m *indexBlobManagerV0) getIndexBlob(ctx context.Context, blobID blob.ID) ([]byte, error) { - return m.enc.getEncryptedBlob(ctx, blobID) +func (m *indexBlobManagerV0) getIndexBlob(ctx context.Context, blobID blob.ID, output *gather.WriteBuffer) error { + return m.enc.getEncryptedBlob(ctx, blobID, output) } -func (m *indexBlobManagerV0) writeIndexBlobs(ctx context.Context, dataShards [][]byte, sessionID SessionID) ([]blob.Metadata, error) { +func (m *indexBlobManagerV0) writeIndexBlobs(ctx context.Context, dataShards []gather.Bytes, sessionID SessionID) ([]blob.Metadata, error) { var result []blob.Metadata for _, data := range dataShards { @@ -187,8 +188,11 @@ func (m *indexBlobManagerV0) writeIndexBlobs(ctx context.Context, dataShards [][ func (m *indexBlobManagerV0) getCompactionLogEntries(ctx context.Context, blobs []blob.Metadata) (map[blob.ID]*compactionLogEntry, error) { results := map[blob.ID]*compactionLogEntry{} + var data gather.WriteBuffer + defer data.Close() + for _, cb := range blobs { - data, err := m.enc.getEncryptedBlob(ctx, cb.BlobID) + err := m.enc.getEncryptedBlob(ctx, cb.BlobID, &data) if errors.Is(err, blob.ErrBlobNotFound) { continue @@ -200,7 +204,7 @@ func (m *indexBlobManagerV0) getCompactionLogEntries(ctx context.Context, blobs le := &compactionLogEntry{} - if err := json.Unmarshal(data, le); err != nil { + if err := json.NewDecoder(data.Bytes().Reader()).Decode(le); err != nil { return nil, errors.Wrap(err, "unable to read compaction log entry %q") } @@ -215,8 +219,13 @@ func (m *indexBlobManagerV0) getCompactionLogEntries(ctx context.Context, blobs func (m *indexBlobManagerV0) getCleanupEntries(ctx context.Context, latestServerBlobTime time.Time, blobs []blob.Metadata) (map[blob.ID]*cleanupEntry, error) { results := map[blob.ID]*cleanupEntry{} + var data gather.WriteBuffer + defer data.Close() + for _, cb := range blobs { - data, err := m.enc.getEncryptedBlob(ctx, cb.BlobID) + data.Reset() + + err := m.enc.getEncryptedBlob(ctx, cb.BlobID, &data) if errors.Is(err, blob.ErrBlobNotFound) { continue @@ -228,7 +237,7 @@ func (m *indexBlobManagerV0) getCleanupEntries(ctx context.Context, latestServer le := &cleanupEntry{} - if err := json.Unmarshal(data, le); err != nil { + if err := json.NewDecoder(data.Bytes().Reader()).Decode(le); err != nil { return nil, errors.Wrap(err, "unable to read compaction log entry %q") } @@ -336,7 +345,7 @@ func (m *indexBlobManagerV0) delayCleanupBlobs(ctx context.Context, blobIDs []bl return errors.Wrap(err, "unable to marshal cleanup log bytes") } - if _, err := m.enc.encryptAndWriteBlob(ctx, payload, cleanupBlobPrefix, ""); err != nil { + if _, err := m.enc.encryptAndWriteBlob(ctx, gather.FromSlice(payload), cleanupBlobPrefix, ""); err != nil { return errors.Wrap(err, "unable to cleanup log") } @@ -456,11 +465,13 @@ func (m *indexBlobManagerV0) compactIndexBlobs(ctx context.Context, indexBlobs [ // we must do it after all input blobs have been merged, otherwise we may resurrect contents. m.dropContentsFromBuilder(bld, opt) - dataShards, err := bld.buildShards(m.indexVersion, false, m.indexShardSize) + dataShards, cleanupShards, err := bld.buildShards(m.indexVersion, false, m.indexShardSize) if err != nil { return errors.Wrap(err, "unable to build an index") } + defer cleanupShards() + compactedIndexBlobs, err := m.writeIndexBlobs(ctx, dataShards, "") if err != nil { return errors.Wrap(err, "unable to write compacted indexes") @@ -498,12 +509,15 @@ func (m *indexBlobManagerV0) dropContentsFromBuilder(bld packIndexBuilder, opt C } func addIndexBlobsToBuilder(ctx context.Context, enc *encryptedBlobMgr, bld packIndexBuilder, indexBlobID blob.ID) error { - data, err := enc.getEncryptedBlob(ctx, indexBlobID) + var data gather.WriteBuffer + defer data.Close() + + err := enc.getEncryptedBlob(ctx, indexBlobID, &data) if err != nil { return errors.Wrapf(err, "error getting index %q", indexBlobID) } - index, err := openPackIndex(bytes.NewReader(data), uint32(enc.crypter.Encryptor.Overhead())) + index, err := openPackIndex(bytes.NewReader(data.ToByteSlice()), uint32(enc.crypter.Encryptor.Overhead())) if err != nil { return errors.Wrapf(err, "unable to open index blob %q", indexBlobID) } diff --git a/repo/content/index_blob_manager_v0_test.go b/repo/content/index_blob_manager_v0_test.go index 346bde984..a6cb0bca0 100644 --- a/repo/content/index_blob_manager_v0_test.go +++ b/repo/content/index_blob_manager_v0_test.go @@ -17,6 +17,7 @@ "github.com/kopia/kopia/internal/blobtesting" "github.com/kopia/kopia/internal/clock" "github.com/kopia/kopia/internal/faketime" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/ownwrites" "github.com/kopia/kopia/internal/testlogging" "github.com/kopia/kopia/internal/testutil" @@ -613,15 +614,15 @@ type fakeIndexData struct { func writeFakeIndex(ctx context.Context, t *testing.T, m *indexBlobManagerV0, ndx map[string]fakeContentIndexEntry) ([]blob.Metadata, error) { t.Helper() - j, err := json.Marshal(fakeIndexData{ + var tmp gather.WriteBuffer + defer tmp.Close() + + require.NoError(t, json.NewEncoder(&tmp).Encode(fakeIndexData{ RandomID: rand.Int63(), Entries: ndx, - }) - if err != nil { - return nil, errors.Wrap(err, "json error") - } + })) - bms, err := m.writeIndexBlobs(ctx, [][]byte{j}, "") + bms, err := m.writeIndexBlobs(ctx, []gather.Bytes{tmp.Bytes()}, "") if err != nil { return nil, errors.Wrap(err, "error writing blob") } @@ -655,8 +656,11 @@ func getAllFakeContentsInternal(ctx context.Context, t *testing.T, m *indexBlobM allContents := map[string]fakeContentIndexEntry{} + var bb gather.WriteBuffer + defer bb.Close() + for _, bi := range blobs { - bb, err := m.getIndexBlob(ctx, bi.BlobID) + err := m.getIndexBlob(ctx, bi.BlobID, &bb) if errors.Is(err, blob.ErrBlobNotFound) { return nil, nil, errGetAllFakeContentsRetry } @@ -667,8 +671,8 @@ func getAllFakeContentsInternal(ctx context.Context, t *testing.T, m *indexBlobM var indexData fakeIndexData - if err := json.Unmarshal(bb, &indexData); err != nil { - t.Logf("invalid JSON %v: %v", string(bb), err) + if err := json.NewDecoder(bb.Bytes().Reader()).Decode(&indexData); err != nil { + t.Logf("invalid JSON %v: %v", string(bb.ToByteSlice()), err) return nil, nil, errors.Wrap(err, "error unmarshaling") } @@ -722,7 +726,7 @@ func mustWriteIndexBlob(t *testing.T, m *indexBlobManagerV0, data string) blob.M t.Logf("writing index blob %q", data) - blobMDs, err := m.writeIndexBlobs(testlogging.Context(t), [][]byte{[]byte(data)}, "") + blobMDs, err := m.writeIndexBlobs(testlogging.Context(t), []gather.Bytes{gather.FromSlice([]byte(data))}, "") if err != nil { t.Fatalf("failed to write index blob: %v", err) } diff --git a/repo/content/index_blob_manager_v1.go b/repo/content/index_blob_manager_v1.go index e9cdd1031..1f92c8fc0 100644 --- a/repo/content/index_blob_manager_v1.go +++ b/repo/content/index_blob_manager_v1.go @@ -65,11 +65,13 @@ func (m *indexBlobManagerV1) compactEpoch(ctx context.Context, blobIDs []blob.ID } } - dataShards, err := tmpbld.buildShards(m.indexVersion, true, m.indexShardSize) + dataShards, cleanupShards, err := tmpbld.buildShards(m.indexVersion, true, m.indexShardSize) if err != nil { return errors.Wrap(err, "unable to build index dataShards") } + defer cleanupShards() + var rnd [8]byte if _, err := rand.Read(rnd[:]); err != nil { @@ -78,13 +80,18 @@ func (m *indexBlobManagerV1) compactEpoch(ctx context.Context, blobIDs []blob.ID sessionID := fmt.Sprintf("s%x-c%v", rnd[:], len(dataShards)) + var data2 gather.WriteBuffer + defer data2.Close() + for _, data := range dataShards { - blobID, data2, err := m.enc.crypter.EncryptBLOB(data, outputPrefix, SessionID(sessionID)) + data2.Reset() + + blobID, err := m.enc.crypter.EncryptBLOB(data, outputPrefix, SessionID(sessionID), &data2) if err != nil { return errors.Wrap(err, "error encrypting") } - if err := m.st.PutBlob(ctx, blobID, gather.FromSlice(data2)); err != nil { + if err := m.st.PutBlob(ctx, blobID, data2.Bytes()); err != nil { return errors.Wrap(err, "error writing index blob") } } @@ -92,18 +99,22 @@ func (m *indexBlobManagerV1) compactEpoch(ctx context.Context, blobIDs []blob.ID return nil } -func (m *indexBlobManagerV1) writeIndexBlobs(ctx context.Context, dataShards [][]byte, sessionID SessionID) ([]blob.Metadata, error) { +func (m *indexBlobManagerV1) writeIndexBlobs(ctx context.Context, dataShards []gather.Bytes, sessionID SessionID) ([]blob.Metadata, error) { shards := map[blob.ID]blob.Bytes{} sessionID = SessionID(fmt.Sprintf("%v-c%v", sessionID, len(dataShards))) for _, data := range dataShards { - unprefixedBlobID, data2, err := m.enc.crypter.EncryptBLOB(data, "", sessionID) + // important - we're intentionally using data2 in the inner loop scheduling multiple Close() + data2 := gather.NewWriteBuffer() + defer data2.Close() + + unprefixedBlobID, err := m.enc.crypter.EncryptBLOB(data, "", sessionID, data2) if err != nil { return nil, errors.Wrap(err, "error encrypting") } - shards[unprefixedBlobID] = gather.FromSlice(data2) + shards[unprefixedBlobID] = data2.Bytes() } // nolint:wrapcheck diff --git a/repo/content/internal_logger.go b/repo/content/internal_logger.go index ecd2f5f75..1c1b1d6e7 100644 --- a/repo/content/internal_logger.go +++ b/repo/content/internal_logger.go @@ -1,7 +1,6 @@ package content import ( - "bytes" "compress/gzip" "context" "crypto/rand" @@ -36,19 +35,28 @@ func (m *internalLogManager) Close(ctx context.Context) { m.wg.Wait() } -func (m *internalLogManager) encryptAndWriteLogBlob(prefix blob.ID, data []byte) { - blobID, encrypted, err := m.bc.EncryptBLOB(data, prefix, "") +func (m *internalLogManager) encryptAndWriteLogBlob(prefix blob.ID, data gather.Bytes, closeFunc func()) { + encrypted := gather.NewWriteBuffer() + // Close happens in a goroutine + + blobID, err := m.bc.EncryptBLOB(data, prefix, "", encrypted) if err != nil { + encrypted.Close() + // this should not happen, also nothing can be done about this, we're not in a place where we can return error, log it. return } + b := encrypted.Bytes() + m.wg.Add(1) go func() { defer m.wg.Done() + defer encrypted.Close() + defer closeFunc() - if err := m.st.PutBlob(m.ctx, blobID, gather.FromSlice(encrypted)); err != nil { + if err := m.st.PutBlob(m.ctx, blobID, b); err != nil { // nothing can be done about this, we're not in a place where we can return error, log it. return } @@ -75,7 +83,7 @@ type internalLogger struct { m *internalLogManager mu sync.Mutex - buf *bytes.Buffer + buf *gather.WriteBuffer gzw *gzip.Writer startTime int64 // unix timestamp of the first log prefix blob.ID @@ -92,10 +100,10 @@ func (l *internalLogger) enable() { // Close closes the log session and saves any pending log. func (l *internalLogger) Close(ctx context.Context) { l.mu.Lock() - data := l.flushAndResetLocked() + data, closeFunc := l.flushAndResetLocked() l.mu.Unlock() - l.maybeEncryptAndWriteChunkUnlocked(data) + l.maybeEncryptAndWriteChunkUnlocked(data, closeFunc) } func (l *internalLogger) nowString() string { @@ -109,8 +117,8 @@ func (l *internalLogger) add(level, msg string, args []interface{}) { l.maybeEncryptAndWriteChunkUnlocked(l.addLineAndMaybeFlush(line)) } -func (l *internalLogger) maybeEncryptAndWriteChunkUnlocked(data []byte) { - if data == nil { +func (l *internalLogger) maybeEncryptAndWriteChunkUnlocked(data gather.Bytes, closeFunc func()) { + if data.Length() == 0 { return } @@ -124,10 +132,10 @@ func (l *internalLogger) maybeEncryptAndWriteChunkUnlocked(data []byte) { prefix := blob.ID(fmt.Sprintf("%v_%v_%v_%v_", l.prefix, l.startTime, endTime, atomic.AddInt32(&l.nextChunkNumber, 1))) l.mu.Unlock() - l.m.encryptAndWriteLogBlob(prefix, data) + l.m.encryptAndWriteLogBlob(prefix, data, closeFunc) } -func (l *internalLogger) addLineAndMaybeFlush(line string) []byte { +func (l *internalLogger) addLineAndMaybeFlush(line string) (payload gather.Bytes, closeFunc func()) { l.mu.Lock() defer l.mu.Unlock() @@ -136,8 +144,8 @@ func (l *internalLogger) addLineAndMaybeFlush(line string) []byte { _, err := io.WriteString(w, line) l.logUnexpectedError(err) - if l.buf.Len() < l.m.flushThreshold { - return nil + if l.buf.Length() < l.m.flushThreshold { + return gather.Bytes{}, func() {} } return l.flushAndResetLocked() @@ -145,7 +153,7 @@ func (l *internalLogger) addLineAndMaybeFlush(line string) []byte { func (l *internalLogger) ensureWriterInitializedLocked() io.Writer { if l.gzw == nil { - l.buf = new(bytes.Buffer) + l.buf = gather.NewWriteBuffer() l.gzw = gzip.NewWriter(l.buf) l.startTime = l.m.timeFunc().Unix() } @@ -153,20 +161,21 @@ func (l *internalLogger) ensureWriterInitializedLocked() io.Writer { return l.gzw } -func (l *internalLogger) flushAndResetLocked() []byte { +func (l *internalLogger) flushAndResetLocked() (payload gather.Bytes, closeFunc func()) { if l.gzw == nil { - return nil + return gather.Bytes{}, func() {} } l.logUnexpectedError(l.gzw.Flush()) l.logUnexpectedError(l.gzw.Close()) + closeBuf := l.buf.Close res := l.buf.Bytes() l.buf = nil l.gzw = nil - return res + return res, closeBuf } func (l *internalLogger) logUnexpectedError(err error) { diff --git a/repo/content/sessions.go b/repo/content/sessions.go index a06c5e8bd..84bc060e5 100644 --- a/repo/content/sessions.go +++ b/repo/content/sessions.go @@ -107,14 +107,17 @@ func (bm *WriteManager) writeSessionMarkerLocked(ctx context.Context) error { return errors.Wrap(err, "unable to serialize session marker payload") } - sessionBlobID, encrypted, err := bm.crypter.EncryptBLOB(js, BlobIDPrefixSession, bm.currentSessionInfo.ID) + var encrypted gather.WriteBuffer + defer encrypted.Close() + + sessionBlobID, err := bm.crypter.EncryptBLOB(gather.FromSlice(js), BlobIDPrefixSession, bm.currentSessionInfo.ID, &encrypted) if err != nil { return errors.Wrap(err, "unable to encrypt session marker") } - bm.onUpload(int64(len(encrypted))) + bm.onUpload(int64(encrypted.Length())) - if err := bm.st.PutBlob(ctx, sessionBlobID, gather.FromSlice(encrypted)); err != nil { + if err := bm.st.PutBlob(ctx, sessionBlobID, encrypted.Bytes()); err != nil { return errors.Wrapf(err, "unable to write session marker: %v", string(sessionBlobID)) } @@ -148,7 +151,16 @@ func (bm *WriteManager) ListActiveSessions(ctx context.Context) (map[SessionID]* m := map[SessionID]*SessionInfo{} + var payload gather.WriteBuffer + defer payload.Close() + + var decrypted gather.WriteBuffer + defer decrypted.Close() + for _, b := range blobs { + payload.Reset() + decrypted.Reset() + sid := SessionIDFromBlobID(b.BlobID) if sid == "" { return nil, errors.Errorf("found invalid session blob %v", b.BlobID) @@ -156,7 +168,7 @@ func (bm *WriteManager) ListActiveSessions(ctx context.Context) (map[SessionID]* si := &SessionInfo{} - payload, err := bm.st.GetBlob(ctx, b.BlobID, 0, -1) + err := bm.st.GetBlob(ctx, b.BlobID, 0, -1, &payload) if err != nil { if errors.Is(err, blob.ErrBlobNotFound) { continue @@ -165,12 +177,12 @@ func (bm *WriteManager) ListActiveSessions(ctx context.Context) (map[SessionID]* return nil, errors.Wrapf(err, "error loading session: %v", b.BlobID) } - payload, err = bm.crypter.DecryptBLOB(payload, b.BlobID) + err = bm.crypter.DecryptBLOB(payload.Bytes(), b.BlobID, &decrypted) if err != nil { return nil, errors.Wrapf(err, "error decrypting session: %v", b.BlobID) } - if err := json.Unmarshal(payload, si); err != nil { + if err := json.NewDecoder(decrypted.Bytes().Reader()).Decode(si); err != nil { return nil, errors.Wrapf(err, "error parsing session: %v", b.BlobID) } diff --git a/repo/encryption/aead_helpers.go b/repo/encryption/aead_helpers.go index 0292d61b2..dac64c6f5 100644 --- a/repo/encryption/aead_helpers.go +++ b/repo/encryption/aead_helpers.go @@ -5,41 +5,47 @@ "crypto/rand" "github.com/pkg/errors" + + "github.com/kopia/kopia/internal/gather" ) // aeadSealWithRandomNonce returns AEAD-sealed content prepended with random nonce. -func aeadSealWithRandomNonce(result []byte, a cipher.AEAD, plaintext, contentID []byte) ([]byte, error) { - resultLen := len(plaintext) + a.NonceSize() + a.Overhead() +func aeadSealWithRandomNonce(a cipher.AEAD, plaintext gather.Bytes, contentID []byte, output *gather.WriteBuffer) error { + resultLen := plaintext.Length() + a.NonceSize() + a.Overhead() - if cap(result) < resultLen { - // result slice too small, make a new one - result = make([]byte, 0, resultLen) - } + var tmp gather.WriteBuffer + defer tmp.Close() - result = result[0:a.NonceSize()] + buf := tmp.MakeContiguous(resultLen) + nonce, rest := buf[0:a.NonceSize()], buf[a.NonceSize():a.NonceSize()] - n, err := rand.Read(result) + n, err := rand.Read(nonce) if err != nil { - return nil, errors.Wrap(err, "unable to initialize nonce") + return errors.Wrap(err, "unable to initialize nonce") } if n != a.NonceSize() { - return nil, errors.Errorf("did not read exactly %v bytes, got %v", a.NonceSize(), n) + return errors.Errorf("did not read exactly %v bytes, got %v", a.NonceSize(), n) } - return a.Seal(result, result[0:a.NonceSize()], plaintext, contentID), nil + a.Seal(rest, nonce, plaintext.ToByteSlice(), contentID) + output.Append(buf) + + return nil } // aeadOpenPrefixedWithNonce opens AEAD-protected content, assuming first bytes are the nonce. -func aeadOpenPrefixedWithNonce(output []byte, a cipher.AEAD, ciphertext, contentID []byte) ([]byte, error) { - if len(ciphertext) < a.NonceSize() { - return nil, errors.Errorf("ciphertext too short") +func aeadOpenPrefixedWithNonce(a cipher.AEAD, ciphertext gather.Bytes, contentID []byte, output *gather.WriteBuffer) error { + if ciphertext.Length() < a.NonceSize()+a.Overhead() { + return errors.Errorf("ciphertext too short: %v", ciphertext.Length()) } - v, err := a.Open(output[:0], ciphertext[0:a.NonceSize()], ciphertext[a.NonceSize():], contentID) - if err != nil { - return nil, errors.Errorf("unable to decrypt content") + input := ciphertext.ToByteSlice() + outbuf := output.MakeContiguous(ciphertext.Length() - a.NonceSize() - a.Overhead()) + + if _, err := a.Open(outbuf[:0], input[0:a.NonceSize()], input[a.NonceSize():], contentID); err != nil { + return errors.Errorf("unable to decrypt content") } - return v, nil + return nil } diff --git a/repo/encryption/aes256_gcm_hmac_sha256_encryptor.go b/repo/encryption/aes256_gcm_hmac_sha256_encryptor.go index 020d935c9..d1427ade5 100644 --- a/repo/encryption/aes256_gcm_hmac_sha256_encryptor.go +++ b/repo/encryption/aes256_gcm_hmac_sha256_encryptor.go @@ -9,6 +9,8 @@ "sync" "github.com/pkg/errors" + + "github.com/kopia/kopia/internal/gather" ) const aes256GCMHmacSha256Overhead = 28 @@ -42,22 +44,22 @@ func (e aes256GCMHmacSha256) aeadForContent(contentID []byte) (cipher.AEAD, erro return cipher.NewGCM(c) } -func (e aes256GCMHmacSha256) Decrypt(output, input, contentID []byte) ([]byte, error) { +func (e aes256GCMHmacSha256) Decrypt(input gather.Bytes, contentID []byte, output *gather.WriteBuffer) error { a, err := e.aeadForContent(contentID) if err != nil { - return nil, err + return err } - return aeadOpenPrefixedWithNonce(output, a, input, contentID) + return aeadOpenPrefixedWithNonce(a, input, contentID, output) } -func (e aes256GCMHmacSha256) Encrypt(output, input, contentID []byte) ([]byte, error) { +func (e aes256GCMHmacSha256) Encrypt(input gather.Bytes, contentID []byte, output *gather.WriteBuffer) error { a, err := e.aeadForContent(contentID) if err != nil { - return nil, err + return err } - return aeadSealWithRandomNonce(output, a, input, contentID) + return aeadSealWithRandomNonce(a, input, contentID, output) } func (e aes256GCMHmacSha256) Overhead() int { diff --git a/repo/encryption/chacha20_poly1305_hmac_sha256_encryptor.go b/repo/encryption/chacha20_poly1305_hmac_sha256_encryptor.go index a8953f645..f820e8307 100644 --- a/repo/encryption/chacha20_poly1305_hmac_sha256_encryptor.go +++ b/repo/encryption/chacha20_poly1305_hmac_sha256_encryptor.go @@ -9,6 +9,8 @@ "github.com/pkg/errors" "golang.org/x/crypto/chacha20poly1305" + + "github.com/kopia/kopia/internal/gather" ) const chacha20poly1305hmacSha256EncryptorOverhead = 28 @@ -38,22 +40,22 @@ func (e chacha20poly1305hmacSha256Encryptor) aeadForContent(contentID []byte) (c return chacha20poly1305.New(key) } -func (e chacha20poly1305hmacSha256Encryptor) Decrypt(output, input, contentID []byte) ([]byte, error) { +func (e chacha20poly1305hmacSha256Encryptor) Decrypt(input gather.Bytes, contentID []byte, output *gather.WriteBuffer) error { a, err := e.aeadForContent(contentID) if err != nil { - return nil, err + return err } - return aeadOpenPrefixedWithNonce(output, a, input, contentID) + return aeadOpenPrefixedWithNonce(a, input, contentID, output) } -func (e chacha20poly1305hmacSha256Encryptor) Encrypt(output, input, contentID []byte) ([]byte, error) { +func (e chacha20poly1305hmacSha256Encryptor) Encrypt(input gather.Bytes, contentID []byte, output *gather.WriteBuffer) error { a, err := e.aeadForContent(contentID) if err != nil { - return nil, err + return err } - return aeadSealWithRandomNonce(output, a, input, contentID) + return aeadSealWithRandomNonce(a, input, contentID, output) } func (e chacha20poly1305hmacSha256Encryptor) Overhead() int { diff --git a/repo/encryption/encryption.go b/repo/encryption/encryption.go index 027cb3ace..346caf056 100644 --- a/repo/encryption/encryption.go +++ b/repo/encryption/encryption.go @@ -8,6 +8,8 @@ "github.com/pkg/errors" "golang.org/x/crypto/hkdf" + + "github.com/kopia/kopia/internal/gather" ) const ( @@ -20,12 +22,12 @@ type Encryptor interface { // Encrypt appends the encrypted bytes corresponding to the given plaintext to a given slice. // Must not clobber the input slice and return ciphertext with additional padding and checksum. - Encrypt(output, plainText, contentID []byte) ([]byte, error) + Encrypt(plainText gather.Bytes, contentID []byte, output *gather.WriteBuffer) error // Decrypt appends the unencrypted bytes corresponding to the given ciphertext to a given slice. // Must not clobber the input slice. If IsAuthenticated() == true, Decrypt will perform // authenticity check before decrypting. - Decrypt(output, cipherText, contentID []byte) ([]byte, error) + Decrypt(cipherText gather.Bytes, contentID []byte, output *gather.WriteBuffer) error // Overhead is the number of bytes of overhead added by Encrypt() Overhead() int diff --git a/repo/encryption/encryption_test.go b/repo/encryption/encryption_test.go index 682f94d16..f57587d93 100644 --- a/repo/encryption/encryption_test.go +++ b/repo/encryption/encryption_test.go @@ -7,6 +7,9 @@ mathrand "math/rand" "testing" + "github.com/stretchr/testify/require" + + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/encryption" ) @@ -40,68 +43,54 @@ func TestRoundTrip(t *testing.T) { t.Fatal(err) } - cipherText1, err := e.Encrypt(nil, data, contentID1) - if err != nil || cipherText1 == nil { - t.Errorf("invalid response from Encrypt: %v %v", cipherText1, err) + var cipherText1 gather.WriteBuffer + defer cipherText1.Close() + + var cipherText1b gather.WriteBuffer + defer cipherText1b.Close() + + require.NoError(t, e.Encrypt(gather.FromSlice(data), contentID1, &cipherText1)) + require.NoError(t, e.Encrypt(gather.FromSlice(data), contentID1, &cipherText1b)) + + if v := cipherText1.ToByteSlice(); bytes.Equal(v, cipherText1b.ToByteSlice()) { + t.Errorf("multiple Encrypt returned the same ciphertext: %x", v) } - cipherText1b, err2 := e.Encrypt(nil, data, contentID1) - if err2 != nil || cipherText1b == nil { - t.Errorf("invalid response from Encrypt: %v %v", cipherText1, err2) + var plainText1 gather.WriteBuffer + defer plainText1.Close() + + require.NoError(t, e.Decrypt(cipherText1.Bytes(), contentID1, &plainText1)) + + if v := plainText1.ToByteSlice(); !bytes.Equal(v, data) { + t.Errorf("Encrypt()/Decrypt() does not round-trip: %x %x", v, data) } - if bytes.Equal(cipherText1, cipherText1b) { - t.Errorf("multiple Encrypt returned the same ciphertext: %x", cipherText1) + var cipherText2 gather.WriteBuffer + defer cipherText2.Close() + + require.NoError(t, e.Encrypt(gather.FromSlice(data), contentID2, &cipherText2)) + + var plainText2 gather.WriteBuffer + defer plainText2.Close() + + require.NoError(t, e.Decrypt(cipherText2.Bytes(), contentID2, &plainText2)) + + if v := plainText2.ToByteSlice(); !bytes.Equal(v, data) { + t.Errorf("Encrypt()/Decrypt() does not round-trip: %x %x", v, data) } - plainText1, err := e.Decrypt(nil, cipherText1, contentID1) - if err != nil || plainText1 == nil { - t.Errorf("invalid response from Decrypt: %v %v", plainText1, err) - } - - if !bytes.Equal(plainText1, data) { - t.Errorf("Encrypt()/Decrypt() does not round-trip: %x %x", plainText1, data) - } - - plaintextOutput := make([]byte, 0, 256) - - plainText1a, err := e.Decrypt(plaintextOutput, cipherText1, contentID1) - if err != nil || plainText1 == nil { - t.Errorf("invalid response from Decrypt: %v %v", plainText1, err) - } - - if !bytes.Equal(plainText1a, plaintextOutput[0:len(plainText1a)]) { - t.Errorf("Decrypt() does not use output buffer") - } - - cipherText2, err := e.Encrypt(nil, data, contentID2) - if err != nil || cipherText2 == nil { - t.Errorf("invalid response from Encrypt: %v %v", cipherText2, err) - } - - plainText2, err := e.Decrypt(nil, cipherText2, contentID2) - if err != nil || plainText2 == nil { - t.Errorf("invalid response from Decrypt: %v %v", plainText2, err) - } - - if !bytes.Equal(plainText2, data) { - t.Errorf("Encrypt()/Decrypt() does not round-trip: %x %x", plainText2, data) - } - - if bytes.Equal(cipherText1, cipherText2) { - t.Errorf("ciphertexts should be different, were %x", cipherText1) + if v := cipherText1.ToByteSlice(); bytes.Equal(v, cipherText2.ToByteSlice()) { + t.Errorf("ciphertexts should be different, were %x", v) } // decrypt using wrong content ID - if _, err := e.Decrypt(nil, cipherText2, contentID1); err == nil { - t.Fatalf("expected decrypt to fail for authenticated encryption") - } + require.Error(t, e.Decrypt(cipherText2.Bytes(), contentID1, &plainText2)) // flip some bits in the cipherText - cipherText2[mathrand.Intn(len(cipherText2))] ^= byte(1 + mathrand.Intn(254)) - if _, err := e.Decrypt(nil, cipherText2, contentID1); err == nil { - t.Errorf("expected decrypt failure on invalid ciphertext, got success") - } + b := cipherText2.Bytes() + b.Slices[0][mathrand.Intn(b.Length())] ^= byte(1 + mathrand.Intn(254)) + + require.Error(t, e.Decrypt(b, contentID1, &plainText2)) }) } } @@ -153,12 +142,11 @@ func verifyCiphertextSamples(t *testing.T, masterKey, contentID, payload []byte, ct := samples[encryptionAlgo] if ct == "" { - v, err := enc.Encrypt(nil, payload, contentID) - if err != nil { - t.Fatal(err) - } + var v gather.WriteBuffer + defer v.Close() + require.NoError(t, enc.Encrypt(gather.FromSlice(payload), contentID, &v)) - t.Errorf("missing ciphertext sample for %q: %q,", encryptionAlgo, hex.EncodeToString(v)) + t.Errorf("missing ciphertext sample for %q: %q,", encryptionAlgo, hex.EncodeToString(payload)) } else { b, err := hex.DecodeString(ct) if err != nil { @@ -166,14 +154,13 @@ func verifyCiphertextSamples(t *testing.T, masterKey, contentID, payload []byte, continue } - plainText, err := enc.Decrypt(nil, b, contentID) - if err != nil { - t.Errorf("unable to decrypt %v: %v", encryptionAlgo, err) - continue - } + var plainText gather.WriteBuffer + defer plainText.Close() - if !bytes.Equal(plainText, payload) { - t.Errorf("invalid plaintext after decryption %x, want %x", plainText, payload) + require.NoError(t, enc.Decrypt(gather.FromSlice(b), contentID, &plainText)) + + if v := plainText.ToByteSlice(); !bytes.Equal(v, payload) { + t.Errorf("invalid plaintext after decryption %x, want %x", v, payload) } } } diff --git a/repo/format_block.go b/repo/format_block.go index d47df24f1..5a5682b14 100644 --- a/repo/format_block.go +++ b/repo/format_block.go @@ -107,11 +107,15 @@ func recoverFormatBlobWithLength(ctx context.Context, st blob.Storage, blobID bl } // try prefix - prefixChunk, err := st.GetBlob(ctx, blobID, 0, chunkLength) - if err != nil { + var tmp gather.WriteBuffer + defer tmp.Close() + + if err := st.GetBlob(ctx, blobID, 0, chunkLength, &tmp); err != nil { return nil, errors.Wrapf(err, "error getting blob %v prefix", blobID) } + prefixChunk := tmp.ToByteSlice() + l := decodeInt16(prefixChunk) if l <= maxChecksummedFormatBytesLength && l+lengthOfRecoverBlockLength < len(prefixChunk) { if b, ok := verifyFormatBlobChecksum(prefixChunk[lengthOfRecoverBlockLength : lengthOfRecoverBlockLength+l]); ok { @@ -120,11 +124,12 @@ func recoverFormatBlobWithLength(ctx context.Context, st blob.Storage, blobID bl } // try the suffix - suffixChunk, err := st.GetBlob(ctx, blobID, length-chunkLength, chunkLength) - if err != nil { + if err := st.GetBlob(ctx, blobID, length-chunkLength, chunkLength, &tmp); err != nil { return nil, errors.Wrapf(err, "error getting blob %v suffix", blobID) } + suffixChunk := tmp.ToByteSlice() + l = decodeInt16(suffixChunk[len(suffixChunk)-lengthOfRecoverBlockLength:]) if l <= maxChecksummedFormatBytesLength && l+lengthOfRecoverBlockLength < len(suffixChunk) { if b, ok := verifyFormatBlobChecksum(suffixChunk[len(suffixChunk)-lengthOfRecoverBlockLength-l : len(suffixChunk)-lengthOfRecoverBlockLength]); ok { diff --git a/repo/grpc_repository_client.go b/repo/grpc_repository_client.go index 6f13a4ecc..4953359f2 100644 --- a/repo/grpc_repository_client.go +++ b/repo/grpc_repository_client.go @@ -19,6 +19,7 @@ "github.com/kopia/kopia/internal/cache" "github.com/kopia/kopia/internal/clock" "github.com/kopia/kopia/internal/ctxutil" + "github.com/kopia/kopia/internal/gather" apipb "github.com/kopia/kopia/internal/grpcapi" "github.com/kopia/kopia/internal/retry" "github.com/kopia/kopia/internal/tlsutil" @@ -522,23 +523,28 @@ func unhandledSessionResponse(resp *apipb.SessionResponse) error { } func (r *grpcRepositoryClient) GetContent(ctx context.Context, contentID content.ID) ([]byte, error) { - b, err := r.contentCache.GetOrLoad(ctx, string(contentID), func() ([]byte, error) { + var b gather.WriteBuffer + defer b.Close() + + err := r.contentCache.GetOrLoad(ctx, string(contentID), func(output *gather.WriteBuffer) error { v, err := r.maybeRetry(ctx, func(ctx context.Context, sess *grpcInnerSession) (interface{}, error) { return sess.GetContent(ctx, contentID) }) if err != nil { - return nil, err + return err } - return v.([]byte), nil - }) + _, err = output.Write(v.([]byte)) + + // nolint:wrapcheck + return err + }, &b) if err == nil && contentID.HasPrefix() { r.recent.add(contentID) } - // nolint:wrapcheck - return b, err + return b.ToByteSlice(), err } func (r *grpcInnerSession) GetContent(ctx context.Context, contentID content.ID) ([]byte, error) { @@ -583,7 +589,7 @@ func (r *grpcRepositoryClient) doWrite(ctx context.Context, contentID content.ID if prefix != "" { // add all prefixed contents to the cache. - r.contentCache.Put(ctx, string(contentID), data) + r.contentCache.Put(ctx, string(contentID), gather.FromSlice(data)) } if v.(content.ID) != contentID { @@ -605,7 +611,7 @@ func (r *grpcRepositoryClient) WriteContent(ctx context.Context, data []byte, pr var hashOutput [128]byte - contentID := prefix + content.ID(hex.EncodeToString(r.h(hashOutput[:0], data))) + contentID := prefix + content.ID(hex.EncodeToString(r.h(hashOutput[:0], gather.FromSlice(data)))) if r.recent.exists(contentID) { return contentID, nil @@ -666,10 +672,6 @@ func (r *grpcRepositoryClient) Close(ctx context.Context) error { return nil } - if err := r.omgr.Close(); err != nil { - return errors.Wrap(err, "error closing object manager") - } - r.omgr = nil if atomic.AddInt32(r.connRefCount, -1) == 0 { diff --git a/repo/hashing/hashing.go b/repo/hashing/hashing.go index e74918050..d088ad85b 100644 --- a/repo/hashing/hashing.go +++ b/repo/hashing/hashing.go @@ -8,6 +8,8 @@ "sync" "github.com/pkg/errors" + + "github.com/kopia/kopia/internal/gather" ) // MaxHashSize is the maximum hash size supported in the system. @@ -20,7 +22,7 @@ type Parameters interface { } // HashFunc computes hash of content of data using a cryptographic hash function, possibly with HMAC and/or truncation. -type HashFunc func(output, data []byte) []byte +type HashFunc func(output []byte, data gather.Bytes) []byte // HashFuncFactory returns a hash function for given formatting options. type HashFuncFactory func(p Parameters) (HashFunc, error) @@ -57,13 +59,13 @@ func truncatedHMACHashFuncFactory(hf func() hash.Hash, truncate int) HashFuncFac }, } - return func(output, b []byte) []byte { + return func(output []byte, data gather.Bytes) []byte { // nolint:forcetypeassert h := pool.Get().(hash.Hash) defer pool.Put(h) h.Reset() - h.Write(b) + data.WriteTo(h) //nolint:errcheck return h.Sum(output)[0:truncate] }, nil @@ -86,13 +88,13 @@ func truncatedKeyedHashFuncFactory(hf func(key []byte) (hash.Hash, error), trunc }, } - return func(output, b []byte) []byte { + return func(output []byte, data gather.Bytes) []byte { // nolint:forcetypeassert h := pool.Get().(hash.Hash) defer pool.Put(h) h.Reset() - h.Write(b) + data.WriteTo(h) //nolint:errcheck return h.Sum(output)[0:truncate] }, nil diff --git a/repo/hashing/hashing_test.go b/repo/hashing/hashing_test.go index 63047be48..c8b9e370a 100644 --- a/repo/hashing/hashing_test.go +++ b/repo/hashing/hashing_test.go @@ -5,6 +5,7 @@ "crypto/rand" "testing" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/hashing" ) @@ -36,9 +37,9 @@ func TestRoundTrip(t *testing.T) { } outputBuffer := make([]byte, 0, 256) - hash1a := f(nil, data1) - hash1b := f(outputBuffer, data1) - hash2 := f(nil, data2) + hash1a := f(nil, gather.FromSlice(data1)) + hash1b := f(outputBuffer, gather.FromSlice(data1)) + hash2 := f(nil, gather.FromSlice(data2)) if !bytes.Equal(hash1a, hash1b) { t.Fatalf("hashing not stable: %x %x", hash1a, hash1b) diff --git a/repo/initialize.go b/repo/initialize.go index 24ac0966d..b8a255f68 100644 --- a/repo/initialize.go +++ b/repo/initialize.go @@ -7,6 +7,7 @@ "github.com/pkg/errors" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" "github.com/kopia/kopia/repo/content" "github.com/kopia/kopia/repo/encryption" @@ -47,7 +48,10 @@ func Initialize(ctx context.Context, st blob.Storage, opt *NewRepositoryOptions, } // get the blob - expect ErrNotFound - _, err := st.GetBlob(ctx, FormatBlobID, 0, -1) + var tmp gather.WriteBuffer + defer tmp.Close() + + err := st.GetBlob(ctx, FormatBlobID, 0, -1, &tmp) if err == nil { return ErrAlreadyInitialized } diff --git a/repo/maintenance/blob_gc_test.go b/repo/maintenance/blob_gc_test.go index 29864575a..e293a0e4f 100644 --- a/repo/maintenance/blob_gc_test.go +++ b/repo/maintenance/blob_gc_test.go @@ -207,10 +207,11 @@ func mustPutDummySessionBlob(t *testing.T, st blob.Storage, sessionIDSuffix blob require.NoError(t, err) - enc, err := e.Encrypt(nil, j, iv) - require.NoError(t, err) + var enc gather.WriteBuffer + defer enc.Close() - require.NoError(t, st.PutBlob(testlogging.Context(t), blobID, gather.FromSlice(enc))) + require.NoError(t, e.Encrypt(gather.FromSlice(j), iv, &enc)) + require.NoError(t, st.PutBlob(testlogging.Context(t), blobID, enc.Bytes())) return blobID } diff --git a/repo/maintenance/maintenance_schedule.go b/repo/maintenance/maintenance_schedule.go index a6e217162..214cc55c7 100644 --- a/repo/maintenance/maintenance_schedule.go +++ b/repo/maintenance/maintenance_schedule.go @@ -72,8 +72,11 @@ func getAES256GCM(rep repo.DirectRepository) (cipher.AEAD, error) { // GetSchedule gets the scheduled maintenance times. func GetSchedule(ctx context.Context, rep repo.DirectRepository) (*Schedule, error) { + var tmp gather.WriteBuffer + defer tmp.Close() + // read - v, err := rep.BlobReader().GetBlob(ctx, maintenanceScheduleBlobID, 0, -1) + err := rep.BlobReader().GetBlob(ctx, maintenanceScheduleBlobID, 0, -1, &tmp) if errors.Is(err, blob.ErrBlobNotFound) { return &Schedule{}, nil } @@ -88,6 +91,8 @@ func GetSchedule(ctx context.Context, rep repo.DirectRepository) (*Schedule, err return nil, errors.Wrap(err, "unable to get cipher") } + v := tmp.ToByteSlice() + if len(v) < c.NonceSize() { return nil, errors.Errorf("invalid schedule blob") } diff --git a/repo/object/object_manager.go b/repo/object/object_manager.go index 5d6793ed2..8b648ed6b 100644 --- a/repo/object/object_manager.go +++ b/repo/object/object_manager.go @@ -7,15 +7,11 @@ "github.com/pkg/errors" - "github.com/kopia/kopia/internal/buf" "github.com/kopia/kopia/repo/compression" "github.com/kopia/kopia/repo/content" "github.com/kopia/kopia/repo/splitter" ) -// maxCompressionOverheadPerSegment is maximum overhead that compression can incur. -const maxCompressionOverheadPerSegment = 16384 - // ErrObjectNotFound is returned when an object cannot be found. var ErrObjectNotFound = errors.New("object not found") @@ -49,7 +45,6 @@ type Manager struct { contentMgr contentManager newSplitter splitter.Factory - bufferPool *buf.Pool } // NewWriter creates an ObjectWriter for writing to the repository. @@ -70,8 +65,6 @@ func (om *Manager) NewWriter(ctx context.Context, opt WriterOptions) Writer { w.asyncWritesSemaphore = make(chan struct{}, opt.AsyncWrites) } - w.initBuffer() - return w } @@ -193,13 +186,5 @@ func NewObjectManager(ctx context.Context, bm contentManager, f Format) (*Manage om.newSplitter = splitter.Pooled(os) - om.bufferPool = buf.NewPool(ctx, om.newSplitter().MaxSegmentSize()+maxCompressionOverheadPerSegment, "object-manager") - return om, nil } - -// Close closes the object manager. -func (om *Manager) Close() error { - om.bufferPool.Close() - return nil -} diff --git a/repo/object/object_manager_test.go b/repo/object/object_manager_test.go index 9f7bfa4ed..6ccda6d8b 100644 --- a/repo/object/object_manager_test.go +++ b/repo/object/object_manager_test.go @@ -98,10 +98,6 @@ func setupTest(t *testing.T, compressionHeaderID map[content.ID]compression.Head t.Fatalf("can't create object manager: %v", err) } - t.Cleanup(func() { - r.Close() - }) - return data, r } @@ -272,7 +268,6 @@ func TestObjectWriterRaceBetweenCheckpointAndResult(t *testing.T) { if err != nil { t.Fatalf("can't create object manager: %v", err) } - defer om.Close() allZeroes := make([]byte, 1<<20-5) diff --git a/repo/object/object_reader.go b/repo/object/object_reader.go index 4ec173359..b7debe43d 100644 --- a/repo/object/object_reader.go +++ b/repo/object/object_reader.go @@ -289,7 +289,7 @@ func newRawReader(ctx context.Context, cr contentReader, objectID ID, assertLeng if compressed { var b bytes.Buffer - if err = decompress(&b, payload); err != nil { + if err = compression.DecompressByHeader(&b, bytes.NewReader(payload)); err != nil { return nil, errors.Wrap(err, "decompression error") } @@ -303,20 +303,6 @@ func newRawReader(ctx context.Context, cr contentReader, objectID ID, assertLeng return newObjectReaderWithData(payload), nil } -func decompress(output *bytes.Buffer, b []byte) error { - compressorID, err := compression.IDFromHeader(b) - if err != nil { - return errors.Wrap(err, "invalid compression header") - } - - compressor := compression.ByHeaderID[compressorID] - if compressor == nil { - return errors.Errorf("unsupported compressor %x", compressorID) - } - - return errors.Wrap(compressor.Decompress(output, b), "error decompressing") -} - type readerWithData struct { io.ReadSeeker length int64 diff --git a/repo/object/object_writer.go b/repo/object/object_writer.go index 192e1ea38..b6b09c11b 100644 --- a/repo/object/object_writer.go +++ b/repo/object/object_writer.go @@ -1,7 +1,6 @@ package object import ( - "bytes" "context" "encoding/json" "io" @@ -9,7 +8,7 @@ "github.com/pkg/errors" - "github.com/kopia/kopia/internal/buf" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/compression" "github.com/kopia/kopia/repo/content" "github.com/kopia/kopia/repo/logging" @@ -69,8 +68,7 @@ type objectWriter struct { compressor compression.Compressor prefix content.ID - buf buf.Buf - buffer *bytes.Buffer + buffer gather.WriteBuffer totalLength int64 currentPosition int64 @@ -93,17 +91,10 @@ type objectWriter struct { contentWriteError error // stores async write error, propagated in Result() } -func (w *objectWriter) initBuffer() { - w.buf = w.om.bufferPool.Allocate(w.splitter.MaxSegmentSize()) - w.buffer = bytes.NewBuffer(w.buf.Data[:0]) -} - func (w *objectWriter) Close() error { // wait for any async writes to complete w.asyncWritesWG.Wait() - w.buf.Release() - if w.splitter != nil { w.splitter.Close() } @@ -122,12 +113,17 @@ func (w *objectWriter) Write(data []byte) (n int, err error) { n := w.splitter.NextSplitPoint(data) if n < 0 { // no split points in the buffer - w.buffer.Write(data) + if _, err := w.buffer.Write(data); err != nil { + return 0, errors.Wrap(err, "error writing to buffer") + } + break } // found a split point after `n` bytes, write first n bytes then flush and repeat with the remainder. - w.buffer.Write(data[0:n]) + if _, err := w.buffer.Write(data[0:n]); err != nil { + return 0, errors.Wrap(err, "error writing to buffer") + } if err := w.flushBuffer(); err != nil { return 0, err @@ -140,7 +136,7 @@ func (w *objectWriter) Write(data []byte) (n int, err error) { } func (w *objectWriter) flushBuffer() error { - length := w.buffer.Len() + length := w.buffer.Length() // hold a lock as we may grow the index w.indirectIndexGrowMutex.Lock() @@ -161,20 +157,20 @@ func (w *objectWriter) flushBuffer() error { w.asyncWritesSemaphore <- struct{}{} w.asyncWritesWG.Add(1) - asyncBuf := w.om.bufferPool.Allocate(length) - - // nolint:gocritic - asyncBytes := append(asyncBuf.Data[:0], w.buffer.Bytes()...) + asyncBuf := gather.NewWriteBuffer() + if _, err := w.buffer.Bytes().WriteTo(asyncBuf); err != nil { + return errors.Wrap(err, "error copying buffer for async copy") + } go func() { defer func() { // release write semaphore and buffer <-w.asyncWritesSemaphore - asyncBuf.Release() + asyncBuf.Close() w.asyncWritesWG.Done() }() - if err := w.prepareAndWriteContentChunk(chunkID, asyncBytes); err != nil { + if err := w.prepareAndWriteContentChunk(chunkID, asyncBuf.Bytes()); err != nil { log(w.ctx).Errorf("async write error: %v", err) _ = w.saveError(err) @@ -184,11 +180,11 @@ func (w *objectWriter) flushBuffer() error { return nil } -func (w *objectWriter) prepareAndWriteContentChunk(chunkID int, data []byte) error { - // allocate buffer to hold either compressed bytes or the uncompressed - b := w.om.bufferPool.Allocate(len(data) + maxCompressionOverheadPerSegment) - defer b.Release() +func (w *objectWriter) prepareAndWriteContentChunk(chunkID int, data gather.Bytes) error { + var b gather.WriteBuffer + defer b.Close() + // allocate buffer to hold either compressed bytes or the uncompressed comp := content.NoCompression objectComp := w.compressor @@ -199,12 +195,12 @@ func (w *objectWriter) prepareAndWriteContentChunk(chunkID int, data []byte) err } // contentBytes is what we're going to write to the content manager, it potentially uses bytes from b - contentBytes, isCompressed, err := maybeCompressedContentBytes(objectComp, bytes.NewBuffer(b.Data[:0]), data) + contentBytes, isCompressed, err := maybeCompressedContentBytes(objectComp, data, &b) if err != nil { return errors.Wrap(err, "unable to prepare content bytes") } - contentID, err := w.om.contentMgr.WriteContent(w.ctx, contentBytes, w.prefix, comp) + contentID, err := w.om.contentMgr.WriteContent(w.ctx, contentBytes.ToByteSlice(), w.prefix, comp) if err != nil { return errors.Wrapf(err, "unable to write content chunk %v of %v: %v", chunkID, w.description, err) } @@ -238,13 +234,13 @@ func maybeCompressedObjectID(contentID content.ID, isCompressed bool) ID { return oid } -func maybeCompressedContentBytes(comp compression.Compressor, output *bytes.Buffer, input []byte) (data []byte, isCompressed bool, err error) { +func maybeCompressedContentBytes(comp compression.Compressor, input gather.Bytes, output *gather.WriteBuffer) (data gather.Bytes, isCompressed bool, err error) { if comp != nil { - if err := comp.Compress(output, input); err != nil { - return nil, false, errors.Wrap(err, "compression error") + if err := comp.Compress(output, input.Reader()); err != nil { + return gather.Bytes{}, false, errors.Wrap(err, "compression error") } - if output.Len() < len(input) { + if output.Length() < input.Length() { return output.Bytes(), true, nil } } @@ -258,7 +254,7 @@ func (w *objectWriter) Result() (ID, error) { // no need to hold a lock on w.indirectIndexGrowMutex, since growing index only happens synchronously // and never in parallel with calling Result() - if w.buffer.Len() > 0 || len(w.indirectIndex) == 0 { + if w.buffer.Length() > 0 || len(w.indirectIndex) == 0 { if err := w.flushBuffer(); err != nil { return "", err } @@ -306,8 +302,6 @@ func (w *objectWriter) checkpointLocked() (ID, error) { iw.prefix = indirectContentPrefix } - iw.initBuffer() - defer iw.Close() //nolint:errcheck if err := writeIndirectObject(iw, w.indirectIndex); err != nil { diff --git a/repo/open.go b/repo/open.go index 3b4966ba2..8929895e5 100644 --- a/repo/open.go +++ b/repo/open.go @@ -1,7 +1,6 @@ package repo import ( - "bytes" "context" "io/ioutil" "os" @@ -14,6 +13,7 @@ "github.com/kopia/kopia/internal/atomicfile" "github.com/kopia/kopia/internal/cache" "github.com/kopia/kopia/internal/clock" + "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/repo/blob" loggingwrapper "github.com/kopia/kopia/repo/blob/logging" "github.com/kopia/kopia/repo/blob/readonly" @@ -347,16 +347,18 @@ func readAndCacheFormatBlobBytes(ctx context.Context, st blob.Storage, cacheDire log(ctx).Debugf("kopia.repository cache not enabled") } - b, err := st.GetBlob(ctx, FormatBlobID, 0, -1) - if err != nil { + var b gather.WriteBuffer + defer b.Close() + + if err := st.GetBlob(ctx, FormatBlobID, 0, -1, &b); err != nil { return nil, errors.Wrap(err, "error getting format blob") } if cacheEnabled { - if err := atomicfile.Write(cachedFile, bytes.NewReader(b)); err != nil { + if err := atomicfile.Write(cachedFile, b.Bytes().Reader()); err != nil { log(ctx).Errorf("warning: unable to write cache: %v", err) } } - return b, nil + return b.ToByteSlice(), nil } diff --git a/repo/repository.go b/repo/repository.go index 1603bf7b2..8221de242 100644 --- a/repo/repository.go +++ b/repo/repository.go @@ -245,10 +245,6 @@ func (r *directRepository) Close(ctx context.Context) error { default: } - if err := r.omgr.Close(); err != nil { - return errors.Wrap(err, "error closing object manager") - } - // this will release shared manager and MAY release blob.Store (on last outstanding reference). if err := r.cmgr.Close(ctx); err != nil { return errors.Wrap(err, "error closing content-addressable storage manager") diff --git a/snapshot/snapshotfs/upload.go b/snapshot/snapshotfs/upload.go index 6b6f2a46e..19e285b42 100644 --- a/snapshot/snapshotfs/upload.go +++ b/snapshot/snapshotfs/upload.go @@ -21,6 +21,7 @@ "github.com/kopia/kopia/fs" "github.com/kopia/kopia/fs/ignorefs" "github.com/kopia/kopia/internal/clock" + "github.com/kopia/kopia/internal/iocopy" "github.com/kopia/kopia/repo" "github.com/kopia/kopia/repo/logging" "github.com/kopia/kopia/repo/object" @@ -31,8 +32,6 @@ // DefaultCheckpointInterval is the default frequency of mid-upload checkpointing. const DefaultCheckpointInterval = 45 * time.Minute -const copyBufferSize = 128 * 1024 - var log = logging.GetContextLoggerFunc("snapshotfs") var errCanceled = errors.New("canceled") @@ -77,8 +76,6 @@ type Uploader struct { stats *snapshot.Stats canceled int32 - uploadBufPool sync.Pool - getTicker func(time.Duration) <-chan time.Time // for testing only, when set will write to a given channel whenever checkpoint completes @@ -263,11 +260,8 @@ func (u *Uploader) uploadStreamingFileInternal(ctx context.Context, relativePath } func (u *Uploader) copyWithProgress(dst io.Writer, src io.Reader, completed, length int64) (int64, error) { - // nolint:forcetypeassert - uploadBufPtr := u.uploadBufPool.Get().(*[]byte) - defer u.uploadBufPool.Put(uploadBufPtr) - - uploadBuf := *uploadBufPtr + uploadBuf := iocopy.GetBuffer() + defer iocopy.ReleaseBuffer(uploadBuf) var written int64 @@ -1094,13 +1088,6 @@ func NewUploader(r repo.RepositoryWriter) *Uploader { EnableActions: r.ClientOptions().EnableActions, CheckpointInterval: DefaultCheckpointInterval, getTicker: time.Tick, - uploadBufPool: sync.Pool{ - New: func() interface{} { - p := make([]byte, copyBufferSize) - - return &p - }, - }, } } diff --git a/tests/testdirtree/testdirtree.go b/tests/testdirtree/testdirtree.go index 3bac96a23..b6a673990 100644 --- a/tests/testdirtree/testdirtree.go +++ b/tests/testdirtree/testdirtree.go @@ -199,8 +199,7 @@ func createRandomFile(filename string, options DirectoryTreeOptions, counters *D length = mfs } - _, err = iocopy.Copy(f, io.LimitReader(rand.New(rand.NewSource(clock.Now().UnixNano())), length)) - if err != nil { + if err := iocopy.JustCopy(f, io.LimitReader(rand.New(rand.NewSource(clock.Now().UnixNano())), length)); err != nil { return errors.Wrap(err, "file create error") } diff --git a/tests/testenv/cli_inproc_runner.go b/tests/testenv/cli_inproc_runner.go index 0d6086529..29747ca3b 100644 --- a/tests/testenv/cli_inproc_runner.go +++ b/tests/testenv/cli_inproc_runner.go @@ -6,7 +6,6 @@ "testing" "github.com/kopia/kopia/cli" - "github.com/kopia/kopia/internal/buf" "github.com/kopia/kopia/internal/testlogging" ) @@ -43,9 +42,3 @@ func NewInProcRunner(t *testing.T) *CLIInProcRunner { } var _ CLIRunner = (*CLIInProcRunner)(nil) - -func init() { - // disable buffer management in end-to-end tests as running too many of them in parallel causes too - // much memory usage on low-end platforms. - buf.DisableBufferManagement = true -}