Skip to content

Commit

Permalink
fix: gc works unexpeted in high concurrent
Browse files Browse the repository at this point in the history
When acceld converts multiple images with large size differences, for example : alpine and WordPress.
Obviously alpine will finish the task first, and WordPress is still in the pulling phase.
If alpine triggers gc(decide by configuration), GC will clear the blobs of WordPress,
because the lease of WordPress blob had cached in lease and only used once(in commit).
To solve this problem, now gc will wait all converting tasks completed.

fix #174

Signed-off-by: Yadong Ding <ding_yadong@foxmail.com>
  • Loading branch information
Desiki-high committed Aug 24, 2023
1 parent 50eaaa3 commit f889840
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 0 deletions.
3 changes: 3 additions & 0 deletions pkg/adapter/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,12 @@ func (adp *LocalAdapter) Convert(ctx context.Context, source string) error {
if err = adp.content.NewRemoteCache(cacheRef); err != nil {
return err
}
adp.content.GcMutex.RLock()
if _, err = adp.cvt.Convert(ctx, source, target, cacheRef); err != nil {
adp.content.GcMutex.RUnlock()
return err
}
adp.content.GcMutex.RUnlock()
if err := adp.content.GC(ctx); err != nil {
return err
}
Expand Down
11 changes: 11 additions & 0 deletions pkg/content/content.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
"path/filepath"
"strconv"
"sync"
"time"

ctrcontent "github.com/containerd/containerd/content"
Expand Down Expand Up @@ -47,6 +48,8 @@ type Content struct {
lm leases.Manager
// gcSingleflight help to resolve concurrent gc
gcSingleflight *singleflight.Group
// GcMutex works between gc and convert
GcMutex *sync.RWMutex
// lc cache the used count and reference order of lease
lc *leaseCache
// store is the local content store wrapped inner db
Expand Down Expand Up @@ -95,6 +98,7 @@ func NewContent(contentDir string, databaseDir string, threshold string, useRemo
db: db,
lm: metadata.NewLeaseManager(db),
gcSingleflight: &singleflight.Group{},
GcMutex: &sync.RWMutex{},
lc: lc,
store: db.ContentStore(),
threshold: int64(t),
Expand Down Expand Up @@ -137,6 +141,13 @@ func (content *Content) GC(ctx context.Context) error {
// if the local content size over eighty percent of threshold, gc start
if size > (content.threshold*int64(80))/100 {
if _, err, _ := content.gcSingleflight.Do(accelerationServiceNamespace, func() (interface{}, error) {
content.GcMutex.Lock()
defer content.GcMutex.Unlock()
// recalculate the local cache size
size, err := content.Size()
if err != nil {
return nil, err
}
return nil, content.garbageCollect(ctx, size-(content.threshold*int64(80))/100)
}); err != nil {
return err
Expand Down

0 comments on commit f889840

Please sign in to comment.