Skip to content

content: update copy to discard instead of truncate#2065

Merged
estesp merged 2 commits intocontainerd:masterfrom
dmcgowan:content-discard-over-truncate
Jan 29, 2018
Merged

content: update copy to discard instead of truncate#2065
estesp merged 2 commits intocontainerd:masterfrom
dmcgowan:content-discard-over-truncate

Conversation

@dmcgowan
Copy link
Copy Markdown
Member

Prevents the copy method from calling discard on the writer when the reader is not seekable. Instead, the copy method will discard up to the offset. Truncate is a more expensive operation since any bytes that are truncated already have their hash calculated and are stored on disk in the backend. Re-writing bytes which were truncated requires transfering the data over GRPC again and re-computing the hash up to the point of truncation.

@dmcgowan dmcgowan force-pushed the content-discard-over-truncate branch from b5393c9 to 913c221 Compare January 26, 2018 01:20
@dmcgowan dmcgowan mentioned this pull request Jan 26, 2018
Comment thread content/testsuite/testsuite.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fatalf here.

@dmcgowan dmcgowan force-pushed the content-discard-over-truncate branch from 913c221 to d0b2de1 Compare January 26, 2018 19:44
Signed-off-by: Derek McGowan <[email protected]>
Prevents the copy method from calling discard on the
writer when the reader is not seekable. Instead,
the copy method will discard up to the offset.
Truncate is a more expensive operation since any
bytes that are truncated already have their hash calculated
and are stored on disk in the backend. Re-writing bytes
which were truncated requires transfering the data over
GRPC again and re-computing the hash up to the point of
truncation.

Signed-off-by: Derek McGowan <[email protected]>
@dmcgowan dmcgowan force-pushed the content-discard-over-truncate branch from d0b2de1 to a12f493 Compare January 26, 2018 19:44
@stevvooe
Copy link
Copy Markdown
Member

LGTM

Make sure to backport this to 1.0 branch.

@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #2065 into master will decrease coverage by <.01%.
The diff coverage is 41.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2065      +/-   ##
==========================================
- Coverage   45.44%   45.43%   -0.01%     
==========================================
  Files          96       96              
  Lines        9432     9436       +4     
==========================================
+ Hits         4286     4287       +1     
- Misses       4436     4438       +2     
- Partials      710      711       +1
Flag Coverage Δ
#linux 50.35% <44.44%> (-0.02%) ⬇️
#windows 40.29% <41.66%> (-0.01%) ⬇️
Impacted Files Coverage Δ
content/helpers.go 32.46% <41.66%> (-0.41%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc63a6c...a12f493. Read the comment docs.

@stevvooe stevvooe added this to the 1.0.2 milestone Jan 26, 2018
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit c6a7d10 into containerd:master Jan 29, 2018
@dmcgowan dmcgowan deleted the content-discard-over-truncate branch June 5, 2018 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants