Skip to content

Add flat GC label for leases#3415

Merged
crosbymichael merged 1 commit intocontainerd:masterfrom
dmcgowan:gc-flat-lease
Jul 19, 2019
Merged

Add flat GC label for leases#3415
crosbymichael merged 1 commit intocontainerd:masterfrom
dmcgowan:gc-flat-lease

Conversation

@dmcgowan
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan commented Jul 15, 2019

Provide a flag which configures a lease to only hold reference to its given references and ignore label references during garbage collection rooted from the lease.

This covers a use case to enable using longer lived leases which need to treat the references it is holding as immutable. Since labels are mutable, holding onto an object may cause the number of objects being referenced to grow even when the lease is only intending on keeping the original object. For example, a lease wants to keep a manifest, but not the blobs referred to within the manifest. When another process pulls in those blobs that lease now additionally holds those blobs even if the original process removed its reference to the manifest.

Also added higher level lease tests to test this case and normal leases.

cc @tonistiigi

@dmcgowan dmcgowan force-pushed the gc-flat-lease branch 2 times, most recently from 2469e76 to ed7329d Compare July 15, 2019 18:59
@containerd containerd deleted a comment from theopenlab-ci Bot Jul 15, 2019
@containerd containerd deleted a comment from theopenlab-ci Bot Jul 15, 2019
@crosbymichael
Copy link
Copy Markdown
Member

LGTM

Comment thread metadata/gc.go Outdated
Comment thread metadata/gc.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.

Can be another PR, but we should have a documentation about these labels

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was thinking the same thing

@AkihiroSuda
Copy link
Copy Markdown
Member

needs rebase

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Jul 17, 2019

Build succeeded.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 17, 2019

Codecov Report

Merging #3415 into master will increase coverage by 4.78%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3415      +/-   ##
==========================================
+ Coverage   44.28%   49.06%   +4.78%     
==========================================
  Files         123      102      -21     
  Lines       13665     9916    -3749     
==========================================
- Hits         6051     4865    -1186     
+ Misses       6683     4206    -2477     
+ Partials      931      845      -86
Flag Coverage Δ
#linux 49.06% <100%> (+0.96%) ⬆️
#windows ?
Impacted Files Coverage Δ
gc/gc.go 69.11% <100%> (+1.26%) ⬆️
metadata/gc.go 70.92% <100%> (+5.96%) ⬆️
remotes/docker/auth.go 63.82% <0%> (-3.97%) ⬇️
remotes/docker/status.go 21.42% <0%> (-3.58%) ⬇️
platforms/cpuinfo.go 3.77% <0%> (-0.85%) ⬇️
log/context.go 36.84% <0%> (-0.66%) ⬇️
remotes/docker/handler.go 22.38% <0%> (-0.09%) ⬇️
errdefs/grpc.go 77.46% <0%> (-0.04%) ⬇️
remotes/docker/pusher.go 0% <0%> (ø) ⬆️
content/local/locks.go 100% <0%> (ø) ⬆️
... and 75 more

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 283d5d9...dd0a45d. Read the comment docs.

Copy link
Copy Markdown
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread metadata/gc_test.go Outdated
Provide a flag which configures a lease to only hold
reference to its given references and ignore label references
during garbage collection rooted from the lease.

Signed-off-by: Derek McGowan <[email protected]>
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Jul 18, 2019

Build succeeded.

Copy link
Copy Markdown
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@crosbymichael
Copy link
Copy Markdown
Member

LGTM

@crosbymichael crosbymichael merged commit 4005979 into containerd:master Jul 19, 2019
@dmcgowan dmcgowan deleted the gc-flat-lease branch September 10, 2019 17:43
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