Skip to content
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

add prune support and fix containerd gc #249

Merged
merged 8 commits into from
Jan 5, 2018
Merged

Conversation

tonistiigi
Copy link
Member

@tonistiigi tonistiigi commented Dec 28, 2017

fixes #25
fixes #159

Contrary to the original plan, I don't think anymore that (at least the current implementation) of leases are a good fit to BuildKit. Problem with leases is that they are hidden from the api and interfaces, hard to use in oci worker and because they only apply on creation they make it impossible for two users to share resources. Eg. one blob can't have 2 leases and I think this can cause broken pulls even on concurrent pulls with ctr.

Instead, I've created wrappers that make the create-delete model work again with the GC backed implementation so callers don't need to know the implementation differences of gc and labels. Managing the labels, instead of removing object also makes it easier to share data between build cache and exported resources.

Missing:

  • Content blobs don't get cleaned properly. The root label gets removed but blobs are kept by the references in the manifest. I plan to fix this by adding backreference from layer blob to manifest instead and removing manifests on pull so they will get cleaned up when all associated blobs have been removed.
  • Set up proper gc label on exported images same way as it is done by the puller.
  • integration tests for client and buildctl

@tonistiigi tonistiigi changed the title wip: add prune support and fix containerd gc add prune support and fix containerd gc Dec 28, 2017
@tonistiigi
Copy link
Member Author

This is ready for review now.

cache/manager.go Outdated
@@ -69,42 +70,49 @@ func NewManager(opt ManagerOpt) (Manager, error) {
return cm, nil
}

// init loads all snapshots from metadata state and tries to load the from the
Copy link
Member

Choose a reason for hiding this comment

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

load the ...?

cache/manager.go Outdated
}

if cr.isDead() {
continue
Copy link
Member

Choose a reason for hiding this comment

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

no unlock?

cache/refs.go Outdated
func (cr *cacheRecord) mref() *mutableRef {
ref := &mutableRef{cacheRecord: cr}
cr.refs[ref] = struct{}{}
return ref
}

// hold ref locak before calling
Copy link
Member

Choose a reason for hiding this comment

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

locak -> lock

@tonistiigi
Copy link
Member Author

@AkihiroSuda updated

@AkihiroSuda AkihiroSuda merged commit ae43689 into moby:master Jan 5, 2018
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.

containerd: use lease instead of gc.root label Add prune command to cache manager
2 participants