8000 Implement btrfs usage by dmcgowan · Pull Request #1871 · containerd/containerd · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Implement btrfs usage #1871

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.

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

Merged
merged 1 commit into from
Dec 5, 2017
Merged

Implement btrfs usage #1871

merged 1 commit into from
Dec 5, 2017

Conversation

dmcgowan
Copy link
Member
@dmcgowan dmcgowan commented Dec 4, 2017

Implements btrfs usage using a double walking diff and counting the result. Walking gives the most accurate count and includes inode usage.

This is an alternative implementation to #1836 using our naive diff implementation instead of relying on btrfs quotas. The efficacy of btrfs quotas for filling in this usage has not been proven. There is still a use for quotas in implementing snapshot quotas in the future, but trusting the value is accurate and consistent on commit would require much more testing and research. The naive diff is slower but will always provide a consistent result.

Closes #1836

Implements btrfs usage using a double walking diff and
counting the result. Walking gives the most accurate
count and includes inode usage.

Signed-off-by: Derek McGowan <derek@mcgstyle.net>
@crosbymichael
Copy link
Member

LGTM

@codecov-io
Copy link

Codecov Report

Merging #1871 into master will decrease coverage by 0.2%.
The diff coverage is 24.65%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1871      +/-   ##
==========================================
- Coverage   49.07%   48.87%   -0.21%     
==========================================
  Files          86       86              
  Lines        8263     8332      +69     
==========================================
+ Hits         4055     4072      +17     
- Misses       3538     3586      +48     
- Partials      670      674       +4
Flag Coverage Δ
#linux 52.34% <35.29%> (-0.12%) ⬇️
#windows 44.1% <0%> (-0.15%) ⬇️
Impacted Files Coverage Δ
fs/du.go 0% <0%> (ø) ⬆️
fs/du_unix.go 0% <0%> (ø) ⬆️
fs/du_windows.go 0% <0%> (ø) ⬆️
snapshots/btrfs/btrfs.go 57.39% <66.66%> (+1.33%) ⬆️

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 617c63d...3bc4e69. Read the comment docs.

@stevvooe
Copy link
Member
stevvooe commented Dec 5, 2017

LGTM

@stevvooe stevvooe added this to the 1.0.0 milestone Dec 5, 2017
@AkihiroSuda
Copy link
Member

Doesnt it work for reflinked files?

@stevvooe
Copy link
Member
stevvooe commented Dec 5, 2017

Doesnt it work for reflinked files?

No, this is going to overshoot the total usage.

This change provides the upper limit of usage for snapshot, rather than trying to minimize, which could change on btrfs depending on construction order.

Copy link
Member
@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

LGTM

cc @tonistiigi (for buildctl du)

@stevvooe
Copy link
Member
stevvooe commented Dec 5, 2017

@AkihiroSuda Spoke with tonis and he is okay with this.

@stevvooe stevvooe merged commit 9570174 into containerd:master Dec 5, 2017
@stevvooe stevvooe mentioned this pull request Dec 5, 2017
@dmcgowan dmcgowan deleted the btrfs-usage branch September 10, 2019 17:46
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.

5 participants
0