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

feat: GC on oci.Store.Delete #653

Merged
merged 26 commits into from
Dec 29, 2023
Merged
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
refined lock
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
  • Loading branch information
wangxiaoxuan273 committed Dec 29, 2023
commit ced6b530f7624ad713318febc2e532c6eba45a81
53 changes: 30 additions & 23 deletions content/oci/oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,14 @@

// AutoGC controls if the OCI store will automatically clean newly produced
// dangling (unreferenced) blobs during Delete() operation. For example the
// blobs whose manifests have been deleted. Manifests in the index will not
// blobs whose manifests have been deleted. Manifests in index.json will not
// be deleted.
// - Default value: true.
AutoGC bool

// AutoDeleteReferrers controls if the OCI store will automatically delete its
// referrers when a manifest is deleted. When set to true, the referrers will
// be deleted even if they exist in the index.
// be deleted even if they exist in index.json.
// - Default value: true.
AutoDeleteReferrers bool

Expand All @@ -82,6 +82,19 @@
indexLock sync.Mutex
}

// unsafeStore is used to bypass lock restrictions in Delete.
type unsafeStore struct {
*Store
}

func (s *unsafeStore) Fetch(ctx context.Context, target ocispec.Descriptor) (io.ReadCloser, error) {
return s.storage.Fetch(ctx, target)
}

func (s *unsafeStore) Predecessors(ctx context.Context, node ocispec.Descriptor) ([]ocispec.Descriptor, error) {
return s.graph.Predecessors(ctx, node)
}
wangxiaoxuan273 marked this conversation as resolved.
Show resolved Hide resolved

// New creates a new OCI store with context.Background().
func New(root string) (*Store, error) {
return NewWithContext(context.Background(), root)
Expand Down Expand Up @@ -161,46 +174,40 @@
// Delete deletes the content matching the descriptor from the store. Delete may
// fail on certain systems (i.e. NTFS), if there is a process (i.e. an unclosed
// Reader) using target. If s.AutoGC is set to true, Delete will recursively
// remove the dangling nodes caused by the current delete. If s.AutoRemoveReferrers
// remove the dangling blobs caused by the current delete. If s.AutoDeleteReferrers
// is set to true, Delete will recursively remove the referrers of the manifests
// being deleted.
func (s *Store) Delete(ctx context.Context, target ocispec.Descriptor) error {
deleteQueue := []ocispec.Descriptor{target}
s.sync.Lock()
defer s.sync.Unlock()

deleteQueue := []ocispec.Descriptor{target}
for len(deleteQueue) > 0 {
head := deleteQueue[0]
deleteQueue = deleteQueue[1:]

// get referrers if applicable
if s.AutoDeleteReferrers && descriptor.IsManifest(head) {
referrers, err := registry.Referrers(ctx, s, head, "")
referrers, err := registry.Referrers(ctx, &unsafeStore{s}, head, "")
if err != nil {
return err
}

Check warning on line 194 in content/oci/oci.go

View check run for this annotation

Codecov / codecov/patch

content/oci/oci.go#L193-L194

Added lines #L193 - L194 were not covered by tests
deleteQueue = append(deleteQueue, referrers...)
}

// delete the head of queue
if err := func() error {
s.sync.Lock()
defer s.sync.Unlock()

danglings, err := s.delete(ctx, head)
if err != nil {
return err
}
if s.AutoGC {
for _, d := range danglings {
// do not delete existing manifests in tagResolver
_, err = s.tagResolver.Resolve(ctx, string(d.Digest))
if errors.Is(err, errdef.ErrNotFound) {
deleteQueue = append(deleteQueue, d)
}
danglings, err := s.delete(ctx, head)
if err != nil {
return err
wangxiaoxuan273 marked this conversation as resolved.
Show resolved Hide resolved
}

Check warning on line 202 in content/oci/oci.go

View check run for this annotation

Codecov / codecov/patch

content/oci/oci.go#L201-L202

Added lines #L201 - L202 were not covered by tests
if s.AutoGC {
for _, d := range danglings {
// do not delete existing manifests in tagResolver
_, err = s.tagResolver.Resolve(ctx, string(d.Digest))
if errors.Is(err, errdef.ErrNotFound) {
deleteQueue = append(deleteQueue, d)
}
}
return nil
}(); err != nil {
return err
}
}

Expand All @@ -221,12 +228,12 @@
if untagged && s.AutoSaveIndex {
err := s.saveIndex()
if err != nil {
return nil, err

Check warning on line 231 in content/oci/oci.go

View check run for this annotation

Codecov / codecov/patch

content/oci/oci.go#L231

Added line #L231 was not covered by tests
}
}
if err := s.storage.Delete(ctx, target); err != nil {
return nil, err
}

Check warning on line 236 in content/oci/oci.go

View check run for this annotation

Codecov / codecov/patch

content/oci/oci.go#L235-L236

Added lines #L235 - L236 were not covered by tests
return danglings, nil
}

Expand Down
Loading