8000 LCOW: ApplyDiff() use tar2ext4, not SVM by lowenna · Pull Request #37999 · moby/moby · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

LCOW: ApplyDiff() use tar2ext4, not SVM #37999

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Oct 24, 2018
Merged

LCOW: ApplyDiff() use tar2ext4, not SVM #37999

merged 2 commits into from
Oct 24, 2018

Conversation

lowenna
Copy link
Member
@lowenna lowenna commented Oct 9, 2018

Signed-off-by: John Howard jhoward@microsoft.com

Fixes #36353

This removes the need for an SVM in the LCOW driver to ApplyDiff. This has a significant performance improvement in docker pull (comparable to native Linux now), and also in build steps as no SVM is required for the ApplyDiff side, although it is still required on the Diff side.

This change relates to a fix for #36353

However, it found another issue, tracked by #37955.

The matching containerd PR submitted by @jterry75 is at containerd/containerd#2704

@johnstep PTAL

@lowenna lowenna requested a review from johnstep as a code owner October 9, 2018 22:31
@lowenna
Copy link
Member Author
lowenna commented Oct 9, 2018

Performance change on a test VM:

After

PS C:\control> docker rmi ubuntu; $(measure-command {docker pull ubuntu}).TotalSeconds
Untagged: ubuntu:latest
Untagged: ubuntu@sha256:de774a3145f7ca4f0bd144c7d4ffb2931e06634f11529653b23eba85aef8e378
Deleted: sha256:cd6d8154f1e16e38493c3c2798977c5e142be5e5d41403ca89883840c6d51762
Deleted: sha256:2416e906f135eea2d08b4a8a8ae539328482eacb6cf39100f7c8f99e98a78d84
Deleted: sha256:7f8291c73f3ecc4dc9317076ad01a567dd44510e789242368cd061c709e0e36d
Deleted: sha256:4b3d88bd6e729deea28b2390d1ddfdbfa3db603160a1129f06f85f26e7bcf4a2
Deleted: sha256:f51700a4e396a235cee37249ffc260cdbeb33268225eb8f7345970f5ae309312
Deleted: sha256:a30b835850bfd4c7e9495edf7085cedfad918219227c7157ff71e8afe2661f63
5.3532088

Before

PS C:\control> # Master as at 10/9/2018...
PS C:\control> docker rmi ubuntu; $(measure-command {docker pull ubuntu}).TotalSeconds
Untagged: ubuntu:latest
Untagged: ubuntu@sha256:de774a3145f7ca4f0bd144c7d4ffb2931e06634f11529653b23eba85aef8e378
Deleted: sha256:cd6d8154f1e16e38493c3c2798977c5e142be5e5d41403ca89883840c6d51762
Deleted: sha256:2416e906f135eea2d08b4a8a8ae539328482eacb6cf39100f7c8f99e98a78d84
Deleted: sha256:7f8291c73f3ecc4dc9317076ad01a567dd44510e789242368cd061c709e0e36d
Deleted: sha256:4b3d88bd6e729deea28b2390d1ddfdbfa3db603160a1129f06f85f26e7bcf4a2
Deleted: sha256:f51700a4e396a235cee37249ffc260cdbeb33268225eb8f7345970f5ae309312
Deleted: sha256:a30b835850bfd4c7e9495edf7085cedfad918219227c7157ff71e8afe2661f63
18.3834906

@lowenna
Copy link
Member Author
lowenna commented Oct 9, 2018

@dmcgowan @crosbymichael If one of you guys could take a look too, would be appreciated 😄

John Howard added 2 commits October 9, 2018 16:10
LCOW: ApplyDiff() use tar2ext4, not SVM
bde9996 
Signed-off-by: John Howard <jhoward@microsoft.com>

This removes the need for an SVM in the LCOW driver to ApplyDiff.

This change relates to a fix for moby#36353

However, it found another issue, tracked by moby#37955
Vendor Microsoft/hcsshim @ v0.7.9
d03ab10 
Signed-off-by: John Howard <jhoward@microsoft.com>
@codecov
Copy link
codecov bot commented Oct 10, 2018 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@82a4797). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #37999   +/-   ##
=========================================
  Coverage          ?    36.1%           
=========================================
  Files             ?      610           
  Lines             ?    45159           
  Branches          ?        0           
=========================================
  Hits              ?    16305           
  Misses            ?    26615           
  Partials          ?     2239

@lowenna
Copy link
Member Author
lowenna commented Oct 10, 2018

Vendor check was failing as there were line-ending issues in HCSShim v0.7.8. Bumped to v0.7.9, hopefully that should now work.

thaJeztah
thaJeztah approved these changes Oct 10, 2018
View reviewed changes
Copy link
Member
@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

@thaJeztah
Copy link
Member

ping @simonferquel @tonistiigi PTAL

@thaJeztah
Copy link
Member

oh, and @dmcgowan

@lowenna
Copy link
Member Author
lowenna commented Oct 24, 2018

@johnstep @dmcgowan Either of you got time to review this? Only a change to the LCOW graphdriver and vendoring. The same change is already in the containerd LCOW snapshotter

@dmcgowan
Copy link
Member

LGTM

@thaJeztah
Copy link
Member

Thanks for reviewing @dmcgowan - I'll take your LGTM and merge 👍

@thaJeztah thaJeztah merged commit 1527a67 into moby:master Oct 24, 2018
@lowenna lowenna deleted the jjh/tar2vhd branch October 25, 2018 02:59
@thaJeztah thaJeztah added the area/lcow Issues and PR's related to the experimental LCOW feature label Oct 30, 2018
@thaJeztah thaJeztah mentioned this pull request Nov 6, 2018
Merged
@thaJeztah thaJeztah mentioned this pull request Nov 21, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

@johnstep johnstep Awaiting requested review from johnstep

Assignees

@coolljt0725 coolljt0725

Labels
area/builder area/lcow Issues and PR's related to the experimental LCOW feature platform/windows status/2-code-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

COPY fails with 'The file or directory is corrupted and unreadable'
5 participants
@lowenna @thaJeztah @dmcgowan @coolljt0725 @GordonTheTurtle
0