From c57c03f8410d09bb4b84d9974282ff7ebf05a665 Mon Sep 17 00:00:00 2001 From: Josh Hawn Date: Wed, 17 Dec 2014 18:26:03 -0800 Subject: [PATCH] Refactor to optimize storage driver ApplyDiff() To avoid an expensive call to archive.ChangesDirs() which walks two directory trees and compares every entry, archive.ApplyLayer() has been extended to also return the size of the layer changes. Docker-DCO-1.1-Signed-off-by: Josh Hawn (github: jlhawn) --- archive/changes_test.go | 2 +- archive/diff.go | 40 +++++++++++++++++--------------- archive/utils_test.go | 3 ++- chrootarchive/archive_test.go | 2 +- chrootarchive/diff.go | 43 ++++++++++++++++++++++++++++------- 5 files changed, 61 insertions(+), 29 deletions(-) diff --git a/archive/changes_test.go b/archive/changes_test.go index 34c0f0d..6b8f235 100644 --- a/archive/changes_test.go +++ b/archive/changes_test.go @@ -286,7 +286,7 @@ func TestApplyLayer(t *testing.T) { t.Fatal(err) } - if err := ApplyLayer(src, layerCopy); err != nil { + if _, err := ApplyLayer(src, layerCopy); err != nil { t.Fatal(err) } diff --git a/archive/diff.go b/archive/diff.go index ba22c41..ca28207 100644 --- a/archive/diff.go +++ b/archive/diff.go @@ -15,7 +15,7 @@ import ( "github.com/docker/docker/pkg/system" ) -func UnpackLayer(dest string, layer ArchiveReader) error { +func UnpackLayer(dest string, layer ArchiveReader) (size int64, err error) { tr := tar.NewReader(layer) trBuf := pools.BufioReader32KPool.Get(tr) defer pools.BufioReader32KPool.Put(trBuf) @@ -33,9 +33,11 @@ func UnpackLayer(dest string, layer ArchiveReader) error { break } if err != nil { - return err + return 0, err } + size += hdr.Size + // Normalize name, for safety and for a simple is-root check hdr.Name = filepath.Clean(hdr.Name) @@ -48,7 +50,7 @@ func UnpackLayer(dest string, layer ArchiveReader) error { if _, err := os.Lstat(parentPath); err != nil && os.IsNotExist(err) { err = os.MkdirAll(parentPath, 0600) if err != nil { - return err + return 0, err } } } @@ -63,12 +65,12 @@ func UnpackLayer(dest string, layer ArchiveReader) error { aufsHardlinks[basename] = hdr if aufsTempdir == "" { if aufsTempdir, err = ioutil.TempDir("", "dockerplnk"); err != nil { - return err + return 0, err } defer os.RemoveAll(aufsTempdir) } if err := createTarFile(filepath.Join(aufsTempdir, basename), dest, hdr, tr, true); err != nil { - return err + return 0, err } } continue @@ -77,10 +79,10 @@ func UnpackLayer(dest string, layer ArchiveReader) error { path := filepath.Join(dest, hdr.Name) rel, err := filepath.Rel(dest, path) if err != nil { - return err + return 0, err } if strings.HasPrefix(rel, "..") { - return breakoutError(fmt.Errorf("%q is outside of %q", hdr.Name, dest)) + return 0, breakoutError(fmt.Errorf("%q is outside of %q", hdr.Name, dest)) } base := filepath.Base(path) @@ -88,7 +90,7 @@ func UnpackLayer(dest string, layer ArchiveReader) error { originalBase := base[len(".wh."):] originalPath := filepath.Join(filepath.Dir(path), originalBase) if err := os.RemoveAll(originalPath); err != nil { - return err + return 0, err } } else { // If path exits we almost always just want to remove and replace it. @@ -98,7 +100,7 @@ func UnpackLayer(dest string, layer ArchiveReader) error { if fi, err := os.Lstat(path); err == nil { if !(fi.IsDir() && hdr.Typeflag == tar.TypeDir) { if err := os.RemoveAll(path); err != nil { - return err + return 0, err } } } @@ -113,18 +115,18 @@ func UnpackLayer(dest string, layer ArchiveReader) error { linkBasename := filepath.Base(hdr.Linkname) srcHdr = aufsHardlinks[linkBasename] if srcHdr == nil { - return fmt.Errorf("Invalid aufs hardlink") + return 0, fmt.Errorf("Invalid aufs hardlink") } tmpFile, err := os.Open(filepath.Join(aufsTempdir, linkBasename)) if err != nil { - return err + return 0, err } defer tmpFile.Close() srcData = tmpFile } if err := createTarFile(path, dest, srcHdr, srcData, true); err != nil { - return err + return 0, err } // Directory mtimes must be handled at the end to avoid further @@ -139,27 +141,29 @@ func UnpackLayer(dest string, layer ArchiveReader) error { path := filepath.Join(dest, hdr.Name) ts := []syscall.Timespec{timeToTimespec(hdr.AccessTime), timeToTimespec(hdr.ModTime)} if err := syscall.UtimesNano(path, ts); err != nil { - return err + return 0, err } } - return nil + + return size, nil } // ApplyLayer parses a diff in the standard layer format from `layer`, and -// applies it to the directory `dest`. -func ApplyLayer(dest string, layer ArchiveReader) error { +// applies it to the directory `dest`. Returns the size in bytes of the +// contents of the layer. +func ApplyLayer(dest string, layer ArchiveReader) (int64, error) { dest = filepath.Clean(dest) // We need to be able to set any perms oldmask, err := system.Umask(0) if err != nil { - return err + return 0, err } defer system.Umask(oldmask) // ignore err, ErrNotSupportedPlatform layer, err = DecompressStream(layer) if err != nil { - return err + return 0, err } return UnpackLayer(dest, layer) } diff --git a/archive/utils_test.go b/archive/utils_test.go index 3624fe5..9048027 100644 --- a/archive/utils_test.go +++ b/archive/utils_test.go @@ -17,7 +17,8 @@ var testUntarFns = map[string]func(string, io.Reader) error{ return Untar(r, dest, nil) }, "applylayer": func(dest string, r io.Reader) error { - return ApplyLayer(dest, ArchiveReader(r)) + _, err := ApplyLayer(dest, ArchiveReader(r)) + return err }, } diff --git a/chrootarchive/archive_test.go b/chrootarchive/archive_test.go index 0fe3d64..bb8a22d 100644 --- a/chrootarchive/archive_test.go +++ b/chrootarchive/archive_test.go @@ -95,7 +95,7 @@ func TestChrootApplyEmptyArchiveFromSlowReader(t *testing.T) { t.Fatal(err) } stream := &slowEmptyTarReader{size: 10240, chunkSize: 1024} - if err := ApplyLayer(dest, stream); err != nil { + if _, err := ApplyLayer(dest, stream); err != nil { t.Fatal(err) } } diff --git a/chrootarchive/diff.go b/chrootarchive/diff.go index d4e9529..ac1cbf9 100644 --- a/chrootarchive/diff.go +++ b/chrootarchive/diff.go @@ -1,6 +1,8 @@ package chrootarchive import ( + "bytes" + "encoding/json" "flag" "fmt" "io" @@ -14,6 +16,10 @@ import ( "github.com/docker/docker/pkg/reexec" ) +type applyLayerResponse struct { + LayerSize int64 `json:"layerSize"` +} + func applyLayer() { runtime.LockOSThread() flag.Parse() @@ -21,6 +27,7 @@ func applyLayer() { if err := chroot(flag.Arg(0)); err != nil { fatal(err) } + // We need to be able to set any perms oldmask := syscall.Umask(0) defer syscall.Umask(oldmask) @@ -28,33 +35,53 @@ func applyLayer() { if err != nil { fatal(err) } + os.Setenv("TMPDIR", tmpDir) - err = archive.UnpackLayer("/", os.Stdin) + size, err := archive.UnpackLayer("/", os.Stdin) os.RemoveAll(tmpDir) if err != nil { fatal(err) } - os.RemoveAll(tmpDir) + + encoder := json.NewEncoder(os.Stdout) + if err := encoder.Encode(applyLayerResponse{size}); err != nil { + fatal(fmt.Errorf("unable to encode layerSize JSON: %s", err)) + } + + flush(os.Stdout) flush(os.Stdin) os.Exit(0) } -func ApplyLayer(dest string, layer archive.ArchiveReader) error { +func ApplyLayer(dest string, layer archive.ArchiveReader) (size int64, err error) { dest = filepath.Clean(dest) decompressed, err := archive.DecompressStream(layer) if err != nil { - return err + return 0, err } + defer func() { if c, ok := decompressed.(io.Closer); ok { c.Close() } }() + cmd := reexec.Command("docker-applyLayer", dest) cmd.Stdin = decompressed - out, err := cmd.CombinedOutput() - if err != nil { - return fmt.Errorf("ApplyLayer %s %s", err, out) + + outBuf, errBuf := new(bytes.Buffer), new(bytes.Buffer) + cmd.Stdout, cmd.Stderr = outBuf, errBuf + + if err = cmd.Run(); err != nil { + return 0, fmt.Errorf("ApplyLayer %s stdout: %s stderr: %s", err, outBuf, errBuf) } - return nil + + // Stdout should be a valid JSON struct representing an applyLayerResponse. + response := applyLayerResponse{} + decoder := json.NewDecoder(outBuf) + if err = decoder.Decode(&response); err != nil { + return 0, fmt.Errorf("unable to decode ApplyLayer JSON response: %s", err) + } + + return response.LayerSize, nil }