Skip to content

Implement btrfs usage#1871

Merged
stevvooe merged 1 commit intocontainerd:masterfrom
dmcgowan:btrfs-usage
Dec 5, 2017
Merged

Implement btrfs usage#1871
stevvooe merged 1 commit intocontainerd:masterfrom
dmcgowan:btrfs-usage

Conversation

@dmcgowan
Copy link
Copy Markdown
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 <[email protected]>
@crosbymichael
Copy link
Copy Markdown
Member

LGTM

@codecov-io
Copy link
Copy Markdown

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
Copy Markdown
Member

stevvooe commented Dec 5, 2017

LGTM

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

Doesnt it work for reflinked files?

@stevvooe
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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